From e5cb4d6388bd67193d88b12b4b84f7b0a11d3e95 Mon Sep 17 00:00:00 2001 From: Chandra Kiran G <121796315+kiran-4444@users.noreply.github.com> Date: Fri, 13 Dec 2024 01:14:32 +0530 Subject: [PATCH] [`flake8-pyi`]: More autofixes for redundant-none-literal (`PYI061`) (#14872) Co-authored-by: Alex Waygood --- .../rules/redundant_none_literal.rs | 107 ++++++++++++++---- ...__flake8_pyi__tests__PYI061_PYI061.py.snap | 89 +++++++++++++-- ..._flake8_pyi__tests__PYI061_PYI061.pyi.snap | 41 ++++++- 3 files changed, 203 insertions(+), 34 deletions(-) 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 f9e37c7840afc..db0f6f5abf0cd 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 @@ -1,15 +1,14 @@ use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{Expr, ExprBinOp, ExprNoneLiteral, ExprSubscript, Operator}; -use ruff_python_semantic::{ - analyze::typing::{traverse_literal, traverse_union}, - SemanticModel, +use ruff_python_ast::{ + self as ast, Expr, ExprBinOp, ExprContext, ExprNoneLiteral, ExprSubscript, Operator, }; -use ruff_text_size::Ranged; +use ruff_python_semantic::analyze::typing::{traverse_literal, traverse_union}; +use ruff_text_size::{Ranged, TextRange}; use smallvec::SmallVec; -use crate::checkers::ast::Checker; +use crate::{checkers::ast::Checker, settings::types::PythonVersion}; /// ## What it does /// Checks for redundant `Literal[None]` annotations. @@ -34,9 +33,14 @@ use crate::checkers::ast::Checker; /// Literal[1, 2, 3, "foo", 5] | None /// ``` /// -/// ## Fix safety +/// ## Fix safety and availability /// This rule's fix is marked as safe unless the literal contains comments. /// +/// There is currently no fix available if there are other elements in the `Literal` slice aside +/// from `None` and [`target-version`] is set to Python 3.9 or lower, as the fix always uses the +/// `|` syntax to create unions rather than `typing.Union`, and the `|` syntax for unions was added +/// in Python 3.10. +/// /// ## References /// - [Typing documentation: Legal parameters for `Literal` at type check time](https://typing.readthedocs.io/en/latest/spec/literal.html#legal-parameters-for-literal-at-type-check-time) #[derive(ViolationMetadata)] @@ -65,35 +69,44 @@ impl Violation for RedundantNoneLiteral { } } -/// RUF037 +/// PYI061 pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) { - if !checker.semantic().seen_typing() { + let semantic = checker.semantic(); + + if !semantic.seen_typing() { return; } + let Expr::Subscript(ast::ExprSubscript { + value: literal_subscript, + .. + }) = literal_expr + else { + return; + }; + let mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new(); - let mut other_literal_elements_seen = false; + let mut literal_elements = vec![]; - let mut find_none = |expr: &'a Expr, _parent: &'a Expr| { + let mut partition_literal_elements = |expr: &'a Expr, _parent: &'a Expr| { if let Expr::NoneLiteral(none_expr) = expr { none_exprs.push(none_expr); } else { - other_literal_elements_seen = true; + literal_elements.push(expr); } }; - traverse_literal(&mut find_none, checker.semantic(), literal_expr); + traverse_literal(&mut partition_literal_elements, semantic, literal_expr); if none_exprs.is_empty() { return; } - // Provide a [`Fix`] when the complete `Literal` can be replaced. Applying the fix - // can leave an unused import to be fixed by the `unused-import` rule. - let fix = if other_literal_elements_seen { - None - } else { - create_fix_edit(checker.semantic(), literal_expr).map(|edit| { + let other_literal_elements_seen = !literal_elements.is_empty(); + + // N.B. Applying the fix can leave an unused import to be fixed by the `unused-import` rule. + let fix = + create_fix_edit(checker, literal_expr, literal_subscript, literal_elements).map(|edit| { Fix::applicable_edit( edit, if checker.comment_ranges().intersects(literal_expr.range()) { @@ -102,8 +115,7 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &' Applicability::Safe }, ) - }) - }; + }); for none_expr in none_exprs { let mut diagnostic = Diagnostic::new( @@ -128,7 +140,14 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &' /// See . /// /// [`typing.Union`]: https://docs.python.org/3/library/typing.html#typing.Union -fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option { +fn create_fix_edit( + checker: &Checker, + literal_expr: &Expr, + literal_subscript: &Expr, + literal_elements: Vec<&Expr>, +) -> Option { + let semantic = checker.semantic(); + let enclosing_pep604_union = semantic .current_expressions() .skip(1) @@ -143,8 +162,9 @@ fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option Option 1 { + Expr::Tuple(ast::ExprTuple { + elts: literal_elements.into_iter().cloned().collect(), + range: TextRange::default(), + ctx: ExprContext::Load, + parenthesized: true, + }) + } else { + literal_elements[0].clone() + }), + })), + op: ruff_python_ast::Operator::BitOr, + right: Box::new(Expr::NoneLiteral(ExprNoneLiteral { + range: TextRange::default(), + })), + }); + + let content = checker.generator().expr(&bin_or); + Some(Edit::range_replacement(content, literal_expr.range())) } 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 380b81d37b060..06bec134febe7 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 @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +snapshot_kind: text --- PYI061.py:4:25: PYI061 [*] `Literal[None]` can be replaced with `None` | @@ -55,7 +56,7 @@ PYI061.py:12:24: PYI061 [*] `Literal[None]` can be replaced with `None` 14 14 | 15 15 | -PYI061.py:16:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.py:16:30: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 16 | def func4(arg1: Literal[int, None, float]): | ^^^^ PYI061 @@ -63,6 +64,16 @@ PYI061.py:16:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` +ℹ Safe fix +13 13 | ... +14 14 | +15 15 | +16 |-def func4(arg1: Literal[int, None, float]): + 16 |+def func4(arg1: Literal[int, float] | None): +17 17 | ... +18 18 | +19 19 | + PYI061.py:20:25: PYI061 [*] `Literal[None]` can be replaced with `None` | 20 | def func5(arg1: Literal[None, None]): @@ -99,7 +110,7 @@ PYI061.py:20:31: PYI061 [*] `Literal[None]` can be replaced with `None` 22 22 | 23 23 | -PYI061.py:26:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.py:26:5: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 24 | def func6(arg1: Literal[ 25 | "hello", @@ -110,6 +121,20 @@ PYI061.py:26:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | | = help: Replace with `Literal[...] | None` +ℹ Unsafe fix +21 21 | ... +22 22 | +23 23 | +24 |-def func6(arg1: Literal[ +25 |- "hello", +26 |- None # Comment 1 +27 |- , "world" +28 |- ]): + 24 |+def func6(arg1: Literal["hello", "world"] | None): +29 25 | ... +30 26 | +31 27 | + PYI061.py:33:5: PYI061 [*] `Literal[None]` can be replaced with `None` | 32 | def func7(arg1: Literal[ @@ -177,7 +202,7 @@ PYI061.py:52:9: PYI061 [*] `Literal[None]` can be replaced with `None` 54 54 | 55 55 | ### -PYI061.py:53:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.py:53:15: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 51 | # From flake8-pyi 52 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None" @@ -188,6 +213,16 @@ PYI061.py:53:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` +ℹ Safe fix +50 50 | +51 51 | # From flake8-pyi +52 52 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None" +53 |-Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None" + 53 |+Literal[True] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None" +54 54 | +55 55 | ### +56 56 | # The following rules here are slightly subtle, + PYI061.py:62:9: PYI061 [*] `Literal[None]` can be replaced with `None` | 60 | # If Y061 and Y062 both apply, but all the duplicate members are None, @@ -228,7 +263,7 @@ PYI061.py:62:15: PYI061 [*] `Literal[None]` can be replaced with `None` 64 64 | 65 65 | # ... but if Y061 and Y062 both apply -PYI061.py:63:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.py:63:12: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 61 | # only emit Y061... 62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None" @@ -239,7 +274,17 @@ PYI061.py:63:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` -PYI061.py:63:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +ℹ Safe fix +60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None, +61 61 | # only emit Y061... +62 62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None" +63 |-Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None" + 63 |+Literal[1, "foo"] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None" +64 64 | +65 65 | # ... but if Y061 and Y062 both apply +66 66 | # and there are no None members in the Literal[] slice, + +PYI061.py:63:25: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 61 | # only emit Y061... 62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None" @@ -250,7 +295,17 @@ PYI061.py:63:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` -PYI061.py:68:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +ℹ Safe fix +60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None, +61 61 | # only emit Y061... +62 62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None" +63 |-Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None" + 63 |+Literal[1, "foo"] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None" +64 64 | +65 65 | # ... but if Y061 and Y062 both apply +66 66 | # and there are no None members in the Literal[] slice, + +PYI061.py:68:9: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 66 | # and there are no None members in the Literal[] slice, 67 | # only emit Y062: @@ -259,7 +314,17 @@ PYI061.py:68:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | | = help: Replace with `Literal[...] | None` -PYI061.py:68:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +ℹ Safe fix +65 65 | # ... but if Y061 and Y062 both apply +66 66 | # and there are no None members in the Literal[] slice, +67 67 | # only emit Y062: +68 |-Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True" + 68 |+Literal[True, True] | None # Y062 Duplicate "Literal[]" member "True" +69 69 | +70 70 | +71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567 + +PYI061.py:68:21: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 66 | # and there are no None members in the Literal[] slice, 67 | # only emit Y062: @@ -268,6 +333,16 @@ PYI061.py:68:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` +ℹ Safe fix +65 65 | # ... but if Y061 and Y062 both apply +66 66 | # and there are no None members in the Literal[] slice, +67 67 | # only emit Y062: +68 |-Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True" + 68 |+Literal[True, True] | None # Y062 Duplicate "Literal[]" member "True" +69 69 | +70 70 | +71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567 + PYI061.py:72:12: PYI061 `Literal[None]` can be replaced with `None` | 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567 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 d1b695f73dbe1..1352d24f3935b 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 @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +snapshot_kind: text --- PYI061.pyi:4:25: PYI061 [*] `Literal[None]` can be replaced with `None` | @@ -52,13 +53,23 @@ PYI061.pyi:10:24: PYI061 [*] `Literal[None]` can be replaced with `None` 12 12 | 13 13 | def func4(arg1: Literal[int, None, float]): ... -PYI061.pyi:13:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.pyi:13:30: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 13 | def func4(arg1: Literal[int, None, float]): ... | ^^^^ PYI061 | = help: Replace with `Literal[...] | None` +ℹ Safe fix +10 10 | def func3() -> Literal[None]: ... +11 11 | +12 12 | +13 |-def func4(arg1: Literal[int, None, float]): ... + 13 |+def func4(arg1: Literal[int, float] | None): ... +14 14 | +15 15 | +16 16 | def func5(arg1: Literal[None, None]): ... + PYI061.pyi:16:25: PYI061 [*] `Literal[None]` can be replaced with `None` | 16 | def func5(arg1: Literal[None, None]): ... @@ -93,7 +104,7 @@ PYI061.pyi:16:31: PYI061 [*] `Literal[None]` can be replaced with `None` 18 18 | 19 19 | def func6(arg1: Literal[ -PYI061.pyi:21:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.pyi:21:5: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 19 | def func6(arg1: Literal[ 20 | "hello", @@ -104,6 +115,20 @@ PYI061.pyi:21:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` +ℹ Unsafe fix +16 16 | def func5(arg1: Literal[None, None]): ... +17 17 | +18 18 | +19 |-def func6(arg1: Literal[ +20 |- "hello", +21 |- None # Comment 1 +22 |- , "world" +23 |-]): ... + 19 |+def func6(arg1: Literal["hello", "world"] | None): ... +24 20 | +25 21 | +26 22 | def func7(arg1: Literal[ + PYI061.pyi:27:5: PYI061 [*] `Literal[None]` can be replaced with `None` | 26 | def func7(arg1: Literal[ @@ -168,7 +193,7 @@ PYI061.pyi:42:9: PYI061 [*] `Literal[None]` can be replaced with `None` 44 44 | 45 45 | -PYI061.pyi:43:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None` +PYI061.pyi:43:15: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None` | 41 | # From flake8-pyi 42 | Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None" @@ -177,6 +202,16 @@ PYI061.pyi:43:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | = help: Replace with `Literal[...] | None` +ℹ Safe fix +40 40 | +41 41 | # From flake8-pyi +42 42 | Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None" +43 |-Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None" + 43 |+Literal[True] | None # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None" +44 44 | +45 45 | +46 46 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567 + PYI061.pyi:47:12: PYI061 `Literal[None]` can be replaced with `None` | 46 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567