Skip to content

Commit a40bc6a

Browse files
authored
[ruff] Implement none-not-at-end-of-union (RUF036) (#14314)
1 parent 577de6c commit a40bc6a

File tree

10 files changed

+300
-0
lines changed

10 files changed

+300
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
from typing import Union as U
2+
3+
4+
def func1(arg: None | int):
5+
...
6+
7+
8+
def func2() -> None | int:
9+
...
10+
11+
12+
def func3(arg: None | None | int):
13+
...
14+
15+
16+
def func4(arg: U[None, int]):
17+
...
18+
19+
20+
def func5() -> U[None, int]:
21+
...
22+
23+
24+
def func6(arg: U[None, None, int]):
25+
...
26+
27+
28+
# Ok
29+
def good_func1(arg: int | None):
30+
...
31+
32+
33+
def good_func2() -> int | None:
34+
...
35+
36+
37+
def good_func3(arg: None):
38+
...
39+
40+
41+
def good_func4(arg: U[None]):
42+
...
43+
44+
45+
def good_func5(arg: U[int]):
46+
...
47+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
from typing import Union as U
2+
3+
4+
def func1(arg: None | int): ...
5+
6+
def func2() -> None | int: ...
7+
8+
def func3(arg: None | None | int): ...
9+
10+
def func4(arg: U[None, int]): ...
11+
12+
def func5() -> U[None, int]: ...
13+
14+
def func6(arg: U[None, None, int]): ...
15+
16+
# Ok
17+
def good_func1(arg: int | None): ...
18+
19+
def good_func2() -> int | None: ...
20+
21+
def good_func3(arg: None): ...
22+
23+
def good_func4(arg: U[None]): ...
24+
25+
def good_func5(arg: U[int]): ...

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

+7
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
8080
Rule::DuplicateUnionMember,
8181
Rule::RedundantLiteralUnion,
8282
Rule::UnnecessaryTypeUnion,
83+
Rule::NoneNotAtEndOfUnion,
8384
]) {
8485
// Avoid duplicate checks if the parent is a union, since these rules already
8586
// traverse nested unions.
@@ -96,6 +97,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
9697
if checker.enabled(Rule::UnnecessaryTypeUnion) {
9798
flake8_pyi::rules::unnecessary_type_union(checker, expr);
9899
}
100+
if checker.enabled(Rule::NoneNotAtEndOfUnion) {
101+
ruff::rules::none_not_at_end_of_union(checker, expr);
102+
}
99103
}
100104
}
101105

@@ -1282,6 +1286,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
12821286
if checker.enabled(Rule::RuntimeStringUnion) {
12831287
flake8_type_checking::rules::runtime_string_union(checker, expr);
12841288
}
1289+
if checker.enabled(Rule::NoneNotAtEndOfUnion) {
1290+
ruff::rules::none_not_at_end_of_union(checker, expr);
1291+
}
12851292
}
12861293
}
12871294
Expr::UnaryOp(

crates/ruff_linter/src/codes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
968968
(Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault),
969969
(Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse),
970970
(Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
971+
(Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
971972
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
972973
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
973974

crates/ruff_linter/src/rules/ruff/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ mod tests {
6262
#[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))]
6363
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
6464
#[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
65+
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.py"))]
66+
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))]
6567
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
6668
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
6769
let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ pub(crate) use mutable_class_default::*;
1717
pub(crate) use mutable_dataclass_default::*;
1818
pub(crate) use mutable_fromkeys_value::*;
1919
pub(crate) use never_union::*;
20+
pub(crate) use none_not_at_end_of_union::*;
2021
pub(crate) use parenthesize_logical_operators::*;
2122
pub(crate) use post_init_default::*;
2223
pub(crate) use quadratic_list_summation::*;
@@ -55,6 +56,7 @@ mod mutable_class_default;
5556
mod mutable_dataclass_default;
5657
mod mutable_fromkeys_value;
5758
mod never_union;
59+
mod none_not_at_end_of_union;
5860
mod parenthesize_logical_operators;
5961
mod post_init_default;
6062
mod quadratic_list_summation;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
use ruff_diagnostics::{Diagnostic, Violation};
2+
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast::Expr;
4+
use ruff_python_semantic::analyze::typing::traverse_union;
5+
use ruff_text_size::Ranged;
6+
use smallvec::SmallVec;
7+
8+
use crate::checkers::ast::Checker;
9+
10+
/// ## What it does
11+
/// Checks for type annotations where `None` is not at the end of an union.
12+
///
13+
/// ## Why is this bad?
14+
/// Type annotation unions are associative, meaning that the order of the elements
15+
/// does not matter. The `None` literal represents the absence of a value. For
16+
/// readability, it's preferred to write the more informative type expressions first.
17+
///
18+
/// ## Example
19+
/// ```python
20+
/// def func(arg: None | int): ...
21+
/// ```
22+
///
23+
/// Use instead:
24+
/// ```python
25+
/// def func(arg: int | None): ...
26+
/// ```
27+
///
28+
/// ## References
29+
/// - [Python documentation: Union type](https://docs.python.org/3/library/stdtypes.html#types-union)
30+
/// - [Python documentation: `typing.Optional`](https://docs.python.org/3/library/typing.html#typing.Optional)
31+
/// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None)
32+
#[violation]
33+
pub struct NoneNotAtEndOfUnion;
34+
35+
impl Violation for NoneNotAtEndOfUnion {
36+
#[derive_message_formats]
37+
fn message(&self) -> String {
38+
"`None` not at the end of the type annotation.".to_string()
39+
}
40+
}
41+
42+
/// RUF036
43+
pub(crate) fn none_not_at_end_of_union<'a>(checker: &mut Checker, union: &'a Expr) {
44+
let semantic = checker.semantic();
45+
let mut none_exprs: SmallVec<[&Expr; 1]> = SmallVec::new();
46+
47+
let mut last_expr: Option<&Expr> = None;
48+
let mut find_none = |expr: &'a Expr, _parent: &Expr| {
49+
if matches!(expr, Expr::NoneLiteral(_)) {
50+
none_exprs.push(expr);
51+
}
52+
last_expr = Some(expr);
53+
};
54+
55+
// Walk through all type expressions in the union and keep track of `None` literals.
56+
traverse_union(&mut find_none, semantic, union);
57+
58+
let Some(last_expr) = last_expr else {
59+
return;
60+
};
61+
62+
// The must be at least one `None` expression.
63+
let Some(last_none) = none_exprs.last() else {
64+
return;
65+
};
66+
67+
// If any of the `None` literals is last we do not emit.
68+
if *last_none == last_expr {
69+
return;
70+
}
71+
72+
for none_expr in none_exprs {
73+
checker
74+
.diagnostics
75+
.push(Diagnostic::new(NoneNotAtEndOfUnion, none_expr.range()));
76+
}
77+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
source: crates/ruff_linter/src/rules/ruff/mod.rs
3+
---
4+
RUF036.py:4:16: RUF036 `None` not at the end of the type annotation.
5+
|
6+
4 | def func1(arg: None | int):
7+
| ^^^^ RUF036
8+
5 | ...
9+
|
10+
11+
RUF036.py:8:16: RUF036 `None` not at the end of the type annotation.
12+
|
13+
8 | def func2() -> None | int:
14+
| ^^^^ RUF036
15+
9 | ...
16+
|
17+
18+
RUF036.py:12:16: RUF036 `None` not at the end of the type annotation.
19+
|
20+
12 | def func3(arg: None | None | int):
21+
| ^^^^ RUF036
22+
13 | ...
23+
|
24+
25+
RUF036.py:12:23: RUF036 `None` not at the end of the type annotation.
26+
|
27+
12 | def func3(arg: None | None | int):
28+
| ^^^^ RUF036
29+
13 | ...
30+
|
31+
32+
RUF036.py:16:18: RUF036 `None` not at the end of the type annotation.
33+
|
34+
16 | def func4(arg: U[None, int]):
35+
| ^^^^ RUF036
36+
17 | ...
37+
|
38+
39+
RUF036.py:20:18: RUF036 `None` not at the end of the type annotation.
40+
|
41+
20 | def func5() -> U[None, int]:
42+
| ^^^^ RUF036
43+
21 | ...
44+
|
45+
46+
RUF036.py:24:18: RUF036 `None` not at the end of the type annotation.
47+
|
48+
24 | def func6(arg: U[None, None, int]):
49+
| ^^^^ RUF036
50+
25 | ...
51+
|
52+
53+
RUF036.py:24:24: RUF036 `None` not at the end of the type annotation.
54+
|
55+
24 | def func6(arg: U[None, None, int]):
56+
| ^^^^ RUF036
57+
25 | ...
58+
|
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
source: crates/ruff_linter/src/rules/ruff/mod.rs
3+
---
4+
RUF036.pyi:4:16: RUF036 `None` not at the end of the type annotation.
5+
|
6+
4 | def func1(arg: None | int): ...
7+
| ^^^^ RUF036
8+
5 |
9+
6 | def func2() -> None | int: ...
10+
|
11+
12+
RUF036.pyi:6:16: RUF036 `None` not at the end of the type annotation.
13+
|
14+
4 | def func1(arg: None | int): ...
15+
5 |
16+
6 | def func2() -> None | int: ...
17+
| ^^^^ RUF036
18+
7 |
19+
8 | def func3(arg: None | None | int): ...
20+
|
21+
22+
RUF036.pyi:8:16: RUF036 `None` not at the end of the type annotation.
23+
|
24+
6 | def func2() -> None | int: ...
25+
7 |
26+
8 | def func3(arg: None | None | int): ...
27+
| ^^^^ RUF036
28+
9 |
29+
10 | def func4(arg: U[None, int]): ...
30+
|
31+
32+
RUF036.pyi:8:23: RUF036 `None` not at the end of the type annotation.
33+
|
34+
6 | def func2() -> None | int: ...
35+
7 |
36+
8 | def func3(arg: None | None | int): ...
37+
| ^^^^ RUF036
38+
9 |
39+
10 | def func4(arg: U[None, int]): ...
40+
|
41+
42+
RUF036.pyi:10:18: RUF036 `None` not at the end of the type annotation.
43+
|
44+
8 | def func3(arg: None | None | int): ...
45+
9 |
46+
10 | def func4(arg: U[None, int]): ...
47+
| ^^^^ RUF036
48+
11 |
49+
12 | def func5() -> U[None, int]: ...
50+
|
51+
52+
RUF036.pyi:12:18: RUF036 `None` not at the end of the type annotation.
53+
|
54+
10 | def func4(arg: U[None, int]): ...
55+
11 |
56+
12 | def func5() -> U[None, int]: ...
57+
| ^^^^ RUF036
58+
13 |
59+
14 | def func6(arg: U[None, None, int]): ...
60+
|
61+
62+
RUF036.pyi:14:18: RUF036 `None` not at the end of the type annotation.
63+
|
64+
12 | def func5() -> U[None, int]: ...
65+
13 |
66+
14 | def func6(arg: U[None, None, int]): ...
67+
| ^^^^ RUF036
68+
15 |
69+
16 | # Ok
70+
|
71+
72+
RUF036.pyi:14:24: RUF036 `None` not at the end of the type annotation.
73+
|
74+
12 | def func5() -> U[None, int]: ...
75+
13 |
76+
14 | def func6(arg: U[None, None, int]): ...
77+
| ^^^^ RUF036
78+
15 |
79+
16 | # Ok
80+
|

ruff.schema.json

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)