From ceb2bf116841fab63c02229e24d1bd5f42738fea Mon Sep 17 00:00:00 2001 From: Victor Hugo Gomes Date: Mon, 28 Apr 2025 08:23:29 -0300 Subject: [PATCH] [`flake8-pyi`] Ensure `Literal[None,] | Literal[None,]` is not autofixed to `None | None` (`PYI061`) (#17659) Co-authored-by: Alex Waygood --- crates/ruff/tests/lint.rs | 49 ++++++++++++++++++- .../test/fixtures/flake8_pyi/PYI061.py | 1 - .../test/fixtures/flake8_pyi/PYI061.pyi | 1 - .../rules/redundant_none_literal.rs | 19 +++---- ...__flake8_pyi__tests__PYI061_PYI061.py.snap | 20 -------- ..._flake8_pyi__tests__PYI061_PYI061.pyi.snap | 20 -------- ...ke8_pyi__tests__py38_PYI061_PYI061.py.snap | 20 -------- ...e8_pyi__tests__py38_PYI061_PYI061.pyi.snap | 20 -------- 8 files changed, 56 insertions(+), 94 deletions(-) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index d9b18fcfb7..d58e85a468 100644 --- a/crates/ruff/tests/lint.rs +++ b/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) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py index 51baabe892..4bdc3d9879 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py @@ -78,4 +78,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] diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi index ed879dd646..404ac1157e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi +++ b/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] diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs index 16a48bc5f5..07ca4a96c2 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap index 33bf166e8b..32cfb3f84c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap +++ b/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` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.pyi.snap index a2c845c064..6a48501c8d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.pyi.snap +++ b/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` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.py.snap index 99f4969598..bbee6e9da5 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.py.snap +++ b/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` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.pyi.snap index a2c845c064..6a48501c8d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.pyi.snap +++ b/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`