Add a fix for never-union

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

@@ -5,3 +5,4 @@
 Never | int
 NoReturn | int
 Union[Union[Never, int], Union[NoReturn, int]]
+Union[NoReturn, int, float]

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

@@ -98,12 +98,13 @@ 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::NeverUnion) {
-                        ruff::rules::never_union(checker, expr);
-                    }
                 }
             }
 
+            if checker.enabled(Rule::NeverUnion) {
+                ruff::rules::never_union(checker, expr);
+            }
+
             if checker.any_enabled(&[
                 Rule::SysVersionSlice3,
                 Rule::SysVersion2,
@@ -1158,6 +1159,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 }
             }
 
+            if checker.enabled(Rule::NeverUnion) {
+                ruff::rules::never_union(checker, expr);
+            }
+
             // Avoid duplicate checks if the parent is a union, since these rules already
             // traverse nested unions.
             if !checker.semantic.in_nested_union() {
@@ -1178,9 +1183,6 @@ 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::NeverUnion) {
-                    ruff::rules::never_union(checker, expr);
-                }
             }
         }
         Expr::UnaryOp(

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

@@ -1,8 +1,7 @@
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 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 ruff_python_ast::{self as ast, Expr, Operator};
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
 
@@ -40,7 +39,7 @@ pub struct NeverUnion {
     union_like: UnionLike,
 }
 
-impl Violation for NeverUnion {
+impl AlwaysFixableViolation for NeverUnion {
     #[derive_message_formats]
     fn message(&self) -> String {
         let Self {
@@ -56,52 +55,121 @@ impl Violation for NeverUnion {
             }
         }
     }
+
+    fn fix_title(&self) -> String {
+        let Self { never_like, .. } = self;
+        format!("Remove `{never_like}`")
+    }
 }
 
 /// RUF020
-pub(crate) fn never_union<'a>(checker: &mut Checker, expr: &'a Expr) {
-    let mut expressions: Vec<(NeverLike, UnionLike, &'a Expr)> = Vec::new();
-    let mut rest: Vec<&'a Expr> = Vec::new();
+pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
+    match expr {
+        // Ex) `typing.NoReturn | int`
+        Expr::BinOp(ast::ExprBinOp {
+            op: Operator::BitOr,
+            left,
+            right,
+            range: _,
+        }) => {
+            // Analyze the left-hand side of the `|` operator.
+            if let Some(never_like) = NeverLike::from_expr(left, checker.semantic()) {
+                let mut diagnostic = Diagnostic::new(
+                    NeverUnion {
+                        never_like,
+                        union_like: UnionLike::BinOp,
+                    },
+                    left.range(),
+                );
+                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                    checker.locator().slice(right.as_ref()).to_string(),
+                    expr.range(),
+                )));
+                checker.diagnostics.push(diagnostic);
+            }
 
-    // Find all `typing.Never` and `typing.NoReturn` expressions.
-    let semantic = checker.semantic();
-    let mut collect_never = |expr: &'a Expr, parent: Option<&'a Expr>| {
-        if let Some(call_path) = semantic.resolve_call_path(expr) {
-            let union_like = if parent.is_some_and(Expr::is_bin_op_expr) {
-                UnionLike::BinOp
-            } else {
-                UnionLike::TypingUnion
-            };
+            // Analyze the right-hand side of the `|` operator.
+            if let Some(never_like) = NeverLike::from_expr(right, checker.semantic()) {
+                let mut diagnostic = Diagnostic::new(
+                    NeverUnion {
+                        never_like,
+                        union_like: UnionLike::BinOp,
+                    },
+                    right.range(),
+                );
+                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                    checker.locator().slice(left.as_ref()).to_string(),
+                    expr.range(),
+                )));
+                checker.diagnostics.push(diagnostic);
+            }
+        }
 
-            let never_like = if semantic.match_typing_call_path(&call_path, "NoReturn") {
-                Some(NeverLike::NoReturn)
-            } else if semantic.match_typing_call_path(&call_path, "Never") {
-                Some(NeverLike::Never)
-            } else {
-                None
+        // Ex) `typing.Union[typing.NoReturn, int]`
+        Expr::Subscript(ast::ExprSubscript {
+            value,
+            slice,
+            ctx: _,
+            range: _,
+        }) if checker.semantic().match_typing_expr(value, "Union") => {
+            let Expr::Tuple(ast::ExprTuple {
+                elts,
+                ctx: _,
+                range: _,
+            }) = slice.as_ref()
+            else {
+                return;
             };
 
-            if let Some(never_like) = never_like {
-                expressions.push((never_like, union_like, expr));
-                return;
-            }
-        }
+            // Analyze each element of the `Union`.
+            for elt in elts {
+                if let Some(never_like) = NeverLike::from_expr(elt, checker.semantic()) {
+                    // Collect the other elements of the `Union`.
+                    let rest = elts
+                        .iter()
+                        .filter(|other| *other != elt)
+                        .cloned()
+                        .collect::<Vec<_>>();
 
-        rest.push(expr);
-    };
+                    // Ignore, e.g., `typing.Union[typing.NoReturn]`.
+                    if rest.is_empty() {
+                        return;
+                    }
 
-    traverse_union(&mut collect_never, checker.semantic(), expr, None);
+                    let mut diagnostic = Diagnostic::new(
+                        NeverUnion {
+                            never_like,
+                            union_like: UnionLike::TypingUnion,
+                        },
+                        elt.range(),
+                    );
+                    diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                        if let [only] = rest.as_slice() {
+                            // Ex) `typing.Union[typing.NoReturn, int]` -> `int`
+                            checker.locator().slice(only).to_string()
+                        } else {
+                            // Ex) `typing.Union[typing.NoReturn, int, str]` -> `typing.Union[int, str]`
+                            checker
+                                .generator()
+                                .expr(&Expr::Subscript(ast::ExprSubscript {
+                                    value: value.clone(),
+                                    slice: Box::new(Expr::Tuple(ast::ExprTuple {
+                                        elts: rest,
+                                        ctx: ast::ExprContext::Load,
+                                        range: TextRange::default(),
+                                    })),
+                                    ctx: ast::ExprContext::Load,
+                                    range: TextRange::default(),
+                                }))
+                        },
+                        expr.range(),
+                    )));
+                    checker.diagnostics.push(diagnostic);
+                }
+            }
+        }
 
-    // Create a diagnostic for each `typing.Never` and `typing.NoReturn` expression.
-    for (never_like, union_like, child) in expressions {
-        let diagnostic = Diagnostic::new(
-            NeverUnion {
-                never_like,
-                union_like,
-            },
-            child.range(),
-        );
-        checker.diagnostics.push(diagnostic);
+        _ => {}
     }
 }
 
@@ -121,6 +189,19 @@ enum NeverLike {
     Never,
 }
 
+impl NeverLike {
+    fn from_expr(expr: &Expr, semantic: &ruff_python_semantic::SemanticModel) -> Option<Self> {
+        let call_path = semantic.resolve_call_path(expr)?;
+        if semantic.match_typing_call_path(&call_path, "NoReturn") {
+            Some(NeverLike::NoReturn)
+        } else if semantic.match_typing_call_path(&call_path, "Never") {
+            Some(NeverLike::Never)
+        } else {
+            None
+        }
+    }
+}
+
 impl std::fmt::Display for NeverLike {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {

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

@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/ruff/mod.rs
 ---
-RUF020.py:3:7: RUF020 `Union[Never, T]` is equivalent to `T`
+RUF020.py:3:7: RUF020 [*] `Union[Never, T]` is equivalent to `T`
   |
 1 | from typing import Never, NoReturn, Union
 2 | 
@@ -10,17 +10,38 @@ RUF020.py:3:7: RUF020 `Union[Never, T]` is equivalent to `T`
 4 | Union[NoReturn, int]
 5 | Never | int
   |
+  = help: Remove `Never`
 
-RUF020.py:4:7: RUF020 `Union[NoReturn, T]` is equivalent to `T`
+ℹ Safe fix
+1 1 | from typing import Never, NoReturn, Union
+2 2 | 
+3   |-Union[Never, int]
+  3 |+int
+4 4 | Union[NoReturn, int]
+5 5 | Never | int
+6 6 | NoReturn | int
+
+RUF020.py:4:7: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
   |
 3 | Union[Never, int]
 4 | Union[NoReturn, int]
   |       ^^^^^^^^ RUF020
 5 | Never | int
 6 | NoReturn | int
   |
+  = help: Remove `NoReturn`
+
+ℹ Safe fix
+1 1 | from typing import Never, NoReturn, Union
+2 2 | 
+3 3 | Union[Never, int]
+4   |-Union[NoReturn, int]
+  4 |+int
+5 5 | Never | int
+6 6 | NoReturn | int
+7 7 | Union[Union[Never, int], Union[NoReturn, int]]
 
-RUF020.py:5:1: RUF020 `Never | T` is equivalent to `T`
+RUF020.py:5:1: RUF020 [*] `Never | T` is equivalent to `T`
   |
 3 | Union[Never, int]
 4 | Union[NoReturn, int]
@@ -29,30 +50,88 @@ RUF020.py:5:1: RUF020 `Never | T` is equivalent to `T`
 6 | NoReturn | int
 7 | Union[Union[Never, int], Union[NoReturn, int]]
   |
+  = help: Remove `Never`
 
-RUF020.py:6:1: RUF020 `NoReturn | T` is equivalent to `T`
+ℹ Safe fix
+2 2 | 
+3 3 | Union[Never, int]
+4 4 | Union[NoReturn, int]
+5   |-Never | int
+  5 |+int
+6 6 | NoReturn | int
+7 7 | Union[Union[Never, int], Union[NoReturn, int]]
+8 8 | Union[NoReturn, int, float]
+
+RUF020.py:6:1: RUF020 [*] `NoReturn | T` is equivalent to `T`
   |
 4 | Union[NoReturn, int]
 5 | Never | int
 6 | NoReturn | int
   | ^^^^^^^^ RUF020
 7 | Union[Union[Never, int], Union[NoReturn, int]]
+8 | Union[NoReturn, int, float]
   |
+  = help: Remove `NoReturn`
+
+ℹ Safe fix
+3 3 | Union[Never, int]
+4 4 | Union[NoReturn, int]
+5 5 | Never | int
+6   |-NoReturn | int
+  6 |+int
+7 7 | Union[Union[Never, int], Union[NoReturn, int]]
+8 8 | Union[NoReturn, int, float]
 
-RUF020.py:7:13: RUF020 `Union[Never, T]` is equivalent to `T`
+RUF020.py:7:13: RUF020 [*] `Union[Never, T]` is equivalent to `T`
   |
 5 | Never | int
 6 | NoReturn | int
 7 | Union[Union[Never, int], Union[NoReturn, int]]
   |             ^^^^^ RUF020
+8 | Union[NoReturn, int, float]
   |
+  = help: Remove `Never`
 
-RUF020.py:7:32: RUF020 `Union[NoReturn, T]` is equivalent to `T`
+ℹ Safe fix
+4 4 | Union[NoReturn, int]
+5 5 | Never | int
+6 6 | NoReturn | int
+7   |-Union[Union[Never, int], Union[NoReturn, int]]
+  7 |+Union[int, Union[NoReturn, int]]
+8 8 | Union[NoReturn, int, float]
+
+RUF020.py:7:32: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
   |
 5 | Never | int
 6 | NoReturn | int
 7 | Union[Union[Never, int], Union[NoReturn, int]]
   |                                ^^^^^^^^ RUF020
+8 | Union[NoReturn, int, float]
   |
+  = help: Remove `NoReturn`
+
+ℹ Safe fix
+4 4 | Union[NoReturn, int]
+5 5 | Never | int
+6 6 | NoReturn | int
+7   |-Union[Union[Never, int], Union[NoReturn, int]]
+  7 |+Union[Union[Never, int], int]
+8 8 | Union[NoReturn, int, float]
+
+RUF020.py:8:7: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
+  |
+6 | NoReturn | int
+7 | Union[Union[Never, int], Union[NoReturn, int]]
+8 | Union[NoReturn, int, float]
+  |       ^^^^^^^^ RUF020
+  |
+  = help: Remove `NoReturn`
+
+ℹ Safe fix
+5 5 | Never | int
+6 6 | NoReturn | int
+7 7 | Union[Union[Never, int], Union[NoReturn, int]]
+8   |-Union[NoReturn, int, float]
+  8 |+Union[int, float]