[ruff] Implement none-not-at-end-of-union (RUF036) (#14314)

crates/ruff_linter/resources/test/fixtures/ruff/RUF036.py

@@ -0,0 +1,47 @@
+from typing import Union as U
+
+
+def func1(arg: None | int): 
+    ...
+
+
+def func2() -> None | int: 
+    ...
+
+
+def func3(arg: None | None | int): 
+    ...
+
+
+def func4(arg: U[None, int]): 
+    ...
+
+
+def func5() -> U[None, int]: 
+    ...
+
+
+def func6(arg: U[None, None, int]): 
+    ...
+
+
+# Ok
+def good_func1(arg: int | None): 
+    ...
+
+
+def good_func2() -> int | None: 
+    ...
+
+
+def good_func3(arg: None): 
+    ...
+
+
+def good_func4(arg: U[None]): 
+    ...
+
+
+def good_func5(arg: U[int]): 
+    ...
+

crates/ruff_linter/resources/test/fixtures/ruff/RUF036.pyi

@@ -0,0 +1,25 @@
+from typing import Union as U
+
+
+def func1(arg: None | int): ...
+
+def func2() -> None | int: ...
+
+def func3(arg: None | None | int): ...
+
+def func4(arg: U[None, int]): ...
+
+def func5() -> U[None, int]: ...
+
+def func6(arg: U[None, None, int]): ...
+
+# Ok
+def good_func1(arg: int | None): ...
+
+def good_func2() -> int | None: ...
+
+def good_func3(arg: None): ...
+
+def good_func4(arg: U[None]): ...
+
+def good_func5(arg: U[int]): ...

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

@@ -80,6 +80,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 Rule::DuplicateUnionMember,
                 Rule::RedundantLiteralUnion,
                 Rule::UnnecessaryTypeUnion,
+                Rule::NoneNotAtEndOfUnion,
             ]) {
                 // Avoid duplicate checks if the parent is a union, since these rules already
                 // traverse nested unions.
@@ -96,6 +97,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                     if checker.enabled(Rule::UnnecessaryTypeUnion) {
                         flake8_pyi::rules::unnecessary_type_union(checker, expr);
                     }
+                    if checker.enabled(Rule::NoneNotAtEndOfUnion) {
+                        ruff::rules::none_not_at_end_of_union(checker, expr);
+                    }
                 }
             }
 
@@ -1282,6 +1286,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 if checker.enabled(Rule::RuntimeStringUnion) {
                     flake8_type_checking::rules::runtime_string_union(checker, expr);
                 }
+                if checker.enabled(Rule::NoneNotAtEndOfUnion) {
+                    ruff::rules::none_not_at_end_of_union(checker, expr);
+                }
             }
         }
         Expr::UnaryOp(

crates/ruff_linter/src/codes.rs

@@ -968,6 +968,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault),
         (Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse),
         (Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
+        (Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
 

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

@@ -62,6 +62,8 @@ mod tests {
     #[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))]
     #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
     #[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
+    #[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.py"))]
+    #[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

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

@@ -17,6 +17,7 @@ pub(crate) use mutable_class_default::*;
 pub(crate) use mutable_dataclass_default::*;
 pub(crate) use mutable_fromkeys_value::*;
 pub(crate) use never_union::*;
+pub(crate) use none_not_at_end_of_union::*;
 pub(crate) use parenthesize_logical_operators::*;
 pub(crate) use post_init_default::*;
 pub(crate) use quadratic_list_summation::*;
@@ -55,6 +56,7 @@ mod mutable_class_default;
 mod mutable_dataclass_default;
 mod mutable_fromkeys_value;
 mod never_union;
+mod none_not_at_end_of_union;
 mod parenthesize_logical_operators;
 mod post_init_default;
 mod quadratic_list_summation;

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

@@ -0,0 +1,77 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::Expr;
+use ruff_python_semantic::analyze::typing::traverse_union;
+use ruff_text_size::Ranged;
+use smallvec::SmallVec;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for type annotations where `None` is not at the end of an union.
+///
+/// ## Why is this bad?
+/// Type annotation unions are associative, meaning that the order of the elements
+/// does not matter. The `None` literal represents the absence of a value. For
+/// readability, it's preferred to write the more informative type expressions first.
+///
+/// ## Example
+/// ```python
+/// def func(arg: None | int): ...
+/// ```
+///
+/// Use instead:
+/// ```python
+/// def func(arg: int | None): ...
+/// ```
+///
+/// ## References
+/// - [Python documentation: Union type](https://docs.python.org/3/library/stdtypes.html#types-union)
+/// - [Python documentation: `typing.Optional`](https://docs.python.org/3/library/typing.html#typing.Optional)
+/// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None)
+#[violation]
+pub struct NoneNotAtEndOfUnion;
+
+impl Violation for NoneNotAtEndOfUnion {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "`None` not at the end of the type annotation.".to_string()
+    }
+}
+
+/// RUF036
+pub(crate) fn none_not_at_end_of_union<'a>(checker: &mut Checker, union: &'a Expr) {
+    let semantic = checker.semantic();
+    let mut none_exprs: SmallVec<[&Expr; 1]> = SmallVec::new();
+
+    let mut last_expr: Option<&Expr> = None;
+    let mut find_none = |expr: &'a Expr, _parent: &Expr| {
+        if matches!(expr, Expr::NoneLiteral(_)) {
+            none_exprs.push(expr);
+        }
+        last_expr = Some(expr);
+    };
+
+    // Walk through all type expressions in the union and keep track of `None` literals.
+    traverse_union(&mut find_none, semantic, union);
+
+    let Some(last_expr) = last_expr else {
+        return;
+    };
+
+    // The must be at least one `None` expression.
+    let Some(last_none) = none_exprs.last() else {
+        return;
+    };
+
+    // If any of the `None` literals is last we do not emit.
+    if *last_none == last_expr {
+        return;
+    }
+
+    for none_expr in none_exprs {
+        checker
+            .diagnostics
+            .push(Diagnostic::new(NoneNotAtEndOfUnion, none_expr.range()));
+    }
+}

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF036_RUF036.py.snap

@@ -0,0 +1,58 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+---
+RUF036.py:4:16: RUF036 `None` not at the end of the type annotation.
+  |
+4 | def func1(arg: None | int): 
+  |                ^^^^ RUF036
+5 |     ...
+  |
+
+RUF036.py:8:16: RUF036 `None` not at the end of the type annotation.
+  |
+8 | def func2() -> None | int: 
+  |                ^^^^ RUF036
+9 |     ...
+  |
+
+RUF036.py:12:16: RUF036 `None` not at the end of the type annotation.
+   |
+12 | def func3(arg: None | None | int): 
+   |                ^^^^ RUF036
+13 |     ...
+   |
+
+RUF036.py:12:23: RUF036 `None` not at the end of the type annotation.
+   |
+12 | def func3(arg: None | None | int): 
+   |                       ^^^^ RUF036
+13 |     ...
+   |
+
+RUF036.py:16:18: RUF036 `None` not at the end of the type annotation.
+   |
+16 | def func4(arg: U[None, int]): 
+   |                  ^^^^ RUF036
+17 |     ...
+   |
+
+RUF036.py:20:18: RUF036 `None` not at the end of the type annotation.
+   |
+20 | def func5() -> U[None, int]: 
+   |                  ^^^^ RUF036
+21 |     ...
+   |
+
+RUF036.py:24:18: RUF036 `None` not at the end of the type annotation.
+   |
+24 | def func6(arg: U[None, None, int]): 
+   |                  ^^^^ RUF036
+25 |     ...
+   |
+
+RUF036.py:24:24: RUF036 `None` not at the end of the type annotation.
+   |
+24 | def func6(arg: U[None, None, int]): 
+   |                        ^^^^ RUF036
+25 |     ...
+   |

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF036_RUF036.pyi.snap

@@ -0,0 +1,80 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+---
+RUF036.pyi:4:16: RUF036 `None` not at the end of the type annotation.
+  |
+4 | def func1(arg: None | int): ...
+  |                ^^^^ RUF036
+5 | 
+6 | def func2() -> None | int: ...
+  |
+
+RUF036.pyi:6:16: RUF036 `None` not at the end of the type annotation.
+  |
+4 | def func1(arg: None | int): ...
+5 | 
+6 | def func2() -> None | int: ...
+  |                ^^^^ RUF036
+7 | 
+8 | def func3(arg: None | None | int): ...
+  |
+
+RUF036.pyi:8:16: RUF036 `None` not at the end of the type annotation.
+   |
+ 6 | def func2() -> None | int: ...
+ 7 | 
+ 8 | def func3(arg: None | None | int): ...
+   |                ^^^^ RUF036
+ 9 | 
+10 | def func4(arg: U[None, int]): ...
+   |
+
+RUF036.pyi:8:23: RUF036 `None` not at the end of the type annotation.
+   |
+ 6 | def func2() -> None | int: ...
+ 7 | 
+ 8 | def func3(arg: None | None | int): ...
+   |                       ^^^^ RUF036
+ 9 | 
+10 | def func4(arg: U[None, int]): ...
+   |
+
+RUF036.pyi:10:18: RUF036 `None` not at the end of the type annotation.
+   |
+ 8 | def func3(arg: None | None | int): ...
+ 9 | 
+10 | def func4(arg: U[None, int]): ...
+   |                  ^^^^ RUF036
+11 | 
+12 | def func5() -> U[None, int]: ...
+   |
+
+RUF036.pyi:12:18: RUF036 `None` not at the end of the type annotation.
+   |
+10 | def func4(arg: U[None, int]): ...
+11 | 
+12 | def func5() -> U[None, int]: ...
+   |                  ^^^^ RUF036
+13 | 
+14 | def func6(arg: U[None, None, int]): ...
+   |
+
+RUF036.pyi:14:18: RUF036 `None` not at the end of the type annotation.
+   |
+12 | def func5() -> U[None, int]: ...
+13 | 
+14 | def func6(arg: U[None, None, int]): ...
+   |                  ^^^^ RUF036
+15 | 
+16 | # Ok
+   |
+
+RUF036.pyi:14:24: RUF036 `None` not at the end of the type annotation.
+   |
+12 | def func5() -> U[None, int]: ...
+13 | 
+14 | def func6(arg: U[None, None, int]): ...
+   |                        ^^^^ RUF036
+15 | 
+16 | # Ok
+   |