[flake8-pyi] Ensure Literal[None,] | Literal[None,] is not autofixed to None | None (PYI061) (#17659)

Co-authored-by: Alex Waygood <[email protected]>

Author: LaBatata101

crates/ruff/tests/lint.rs

@@ -3475,7 +3475,7 @@ requires-python = ">= 3.11"
         &inner_pyproject,
         r#"
 [tool.ruff]
-target-version = "py310"        
+target-version = "py310"
 "#,
     )?;
 
@@ -4980,6 +4980,53 @@ fn flake8_import_convention_unused_aliased_import_no_conflict() {
         .pass_stdin("1"));
 }
 
+// See: https://github.com/astral-sh/ruff/issues/16177
+#[test]
+fn flake8_pyi_redundant_none_literal() {
+    let snippet = r#"
+from typing import Literal
+
+# For each of these expressions, Ruff provides a fix for one of the `Literal[None]` elements
+# but not both, as if both were autofixed it would result in `None | None`,
+# which leads to a `TypeError` at runtime.
+a: Literal[None,] | Literal[None,]
+b: Literal[None] | Literal[None]
+c: Literal[None] | Literal[None,]
+d: Literal[None,] | Literal[None]
+"#;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .args(STDIN_BASE_OPTIONS)
+        .args(["--select", "PYI061"])
+        .args(["--stdin-filename", "test.py"])
+        .arg("--preview")
+        .arg("--diff")
+        .arg("-")
+        .pass_stdin(snippet), @r"
+    success: false
+    exit_code: 1
+    ----- stdout -----
+    --- test.py
+    +++ test.py
+    @@ -4,7 +4,7 @@
+     # For each of these expressions, Ruff provides a fix for one of the `Literal[None]` elements
+     # but not both, as if both were autofixed it would result in `None | None`,
+     # which leads to a `TypeError` at runtime.
+    -a: Literal[None,] | Literal[None,]
+    -b: Literal[None] | Literal[None]
+    -c: Literal[None] | Literal[None,]
+    -d: Literal[None,] | Literal[None]
+    +a: None | Literal[None,]
+    +b: None | Literal[None]
+    +c: None | Literal[None,]
+    +d: None | Literal[None]
+
+
+    ----- stderr -----
+    Would fix 4 errors.
+    ");
+}
+
 /// Test that private, old-style `TypeVar` generics
 /// 1. Get replaced with PEP 695 type parameters (UP046, UP047)
 /// 2. Get renamed to remove leading underscores (UP049)

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py

@@ -78,4 +78,3 @@ def good_func(arg1: Literal[int] | None):
 c: (None | Literal[None]) | None
 d: None | (Literal[None] | None)
 e: None | ((None | Literal[None]) | None) | None
-f: Literal[None] | Literal[None]

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi

@@ -53,4 +53,3 @@ b: None | Literal[None] | None
 c: (None | Literal[None]) | None
 d: None | (Literal[None] | None)
 e: None | ((None | Literal[None]) | None) | None
-f: Literal[None] | Literal[None]

crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs

@@ -5,7 +5,7 @@ use ruff_python_ast::{
     self as ast,
     helpers::{pep_604_union, typing_optional},
     name::Name,
-    Expr, ExprBinOp, ExprContext, ExprNoneLiteral, ExprSubscript, Operator, PythonVersion,
+    Expr, ExprBinOp, ExprContext, ExprNoneLiteral, Operator, PythonVersion,
 };
 use ruff_python_semantic::analyze::typing::{traverse_literal, traverse_union};
 use ruff_text_size::{Ranged, TextRange};
@@ -130,6 +130,12 @@ pub(crate) fn redundant_none_literal<'a>(checker: &Checker, literal_expr: &'a Ex
                 literal_elements.clone(),
                 union_kind,
             )
+            // Isolate the fix to ensure multiple fixes on the same expression (like
+            // `Literal[None,] | Literal[None,]` -> `None | None`) happen across separate passes,
+            // preventing the production of invalid code.
+            .map(|fix| {
+                fix.map(|fix| fix.isolate(Checker::isolation(semantic.current_statement_id())))
+            })
         });
         checker.report_diagnostic(diagnostic);
     }
@@ -172,18 +178,9 @@ fn create_fix(
 
         traverse_union(
             &mut |expr, _| {
-                if matches!(expr, Expr::NoneLiteral(_)) {
+                if expr.is_none_literal_expr() {
                     is_fixable = false;
                 }
-                if expr != literal_expr {
-                    if let Expr::Subscript(ExprSubscript { value, slice, .. }) = expr {
-                        if semantic.match_typing_expr(value, "Literal")
-                            && matches!(**slice, Expr::NoneLiteral(_))
-                        {
-                            is_fixable = false;
-                        }
-                    }
-                }
             },
             semantic,
             enclosing_pep604_union,

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap

@@ -422,7 +422,6 @@ PYI061.py:79:20: PYI061 Use `None` rather than `Literal[None]`
 79 | d: None | (Literal[None] | None)
    |                    ^^^^ PYI061
 80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
    |
    = help: Replace with `None`
 
@@ -432,24 +431,5 @@ PYI061.py:80:28: PYI061 Use `None` rather than `Literal[None]`
 79 | d: None | (Literal[None] | None)
 80 | e: None | ((None | Literal[None]) | None) | None
    |                            ^^^^ PYI061
-81 | f: Literal[None] | Literal[None]
-   |
-   = help: Replace with `None`
-
-PYI061.py:81:12: PYI061 Use `None` rather than `Literal[None]`
-   |
-79 | d: None | (Literal[None] | None)
-80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
-   |            ^^^^ PYI061
-   |
-   = help: Replace with `None`
-
-PYI061.py:81:28: PYI061 Use `None` rather than `Literal[None]`
-   |
-79 | d: None | (Literal[None] | None)
-80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
-   |                            ^^^^ PYI061
    |
    = help: Replace with `None`

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.pyi.snap

@@ -291,7 +291,6 @@ PYI061.pyi:54:20: PYI061 Use `None` rather than `Literal[None]`
 54 | d: None | (Literal[None] | None)
    |                    ^^^^ PYI061
 55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
    |
    = help: Replace with `None`
 
@@ -301,24 +300,5 @@ PYI061.pyi:55:28: PYI061 Use `None` rather than `Literal[None]`
 54 | d: None | (Literal[None] | None)
 55 | e: None | ((None | Literal[None]) | None) | None
    |                            ^^^^ PYI061
-56 | f: Literal[None] | Literal[None]
-   |
-   = help: Replace with `None`
-
-PYI061.pyi:56:12: PYI061 Use `None` rather than `Literal[None]`
-   |
-54 | d: None | (Literal[None] | None)
-55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
-   |            ^^^^ PYI061
-   |
-   = help: Replace with `None`
-
-PYI061.pyi:56:28: PYI061 Use `None` rather than `Literal[None]`
-   |
-54 | d: None | (Literal[None] | None)
-55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
-   |                            ^^^^ PYI061
    |
    = help: Replace with `None`

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.py.snap

@@ -464,7 +464,6 @@ PYI061.py:79:20: PYI061 Use `None` rather than `Literal[None]`
 79 | d: None | (Literal[None] | None)
    |                    ^^^^ PYI061
 80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
    |
    = help: Replace with `None`
 
@@ -474,24 +473,5 @@ PYI061.py:80:28: PYI061 Use `None` rather than `Literal[None]`
 79 | d: None | (Literal[None] | None)
 80 | e: None | ((None | Literal[None]) | None) | None
    |                            ^^^^ PYI061
-81 | f: Literal[None] | Literal[None]
-   |
-   = help: Replace with `None`
-
-PYI061.py:81:12: PYI061 Use `None` rather than `Literal[None]`
-   |
-79 | d: None | (Literal[None] | None)
-80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
-   |            ^^^^ PYI061
-   |
-   = help: Replace with `None`
-
-PYI061.py:81:28: PYI061 Use `None` rather than `Literal[None]`
-   |
-79 | d: None | (Literal[None] | None)
-80 | e: None | ((None | Literal[None]) | None) | None
-81 | f: Literal[None] | Literal[None]
-   |                            ^^^^ PYI061
    |
    = help: Replace with `None`

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.pyi.snap

@@ -291,7 +291,6 @@ PYI061.pyi:54:20: PYI061 Use `None` rather than `Literal[None]`
 54 | d: None | (Literal[None] | None)
    |                    ^^^^ PYI061
 55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
    |
    = help: Replace with `None`
 
@@ -301,24 +300,5 @@ PYI061.pyi:55:28: PYI061 Use `None` rather than `Literal[None]`
 54 | d: None | (Literal[None] | None)
 55 | e: None | ((None | Literal[None]) | None) | None
    |                            ^^^^ PYI061
-56 | f: Literal[None] | Literal[None]
-   |
-   = help: Replace with `None`
-
-PYI061.pyi:56:12: PYI061 Use `None` rather than `Literal[None]`
-   |
-54 | d: None | (Literal[None] | None)
-55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
-   |            ^^^^ PYI061
-   |
-   = help: Replace with `None`
-
-PYI061.pyi:56:28: PYI061 Use `None` rather than `Literal[None]`
-   |
-54 | d: None | (Literal[None] | None)
-55 | e: None | ((None | Literal[None]) | None) | None
-56 | f: Literal[None] | Literal[None]
-   |                            ^^^^ PYI061
    |
    = help: Replace with `None`