From ebbd9f10bc505f092b75ffcaed48a047f6d0f0c3 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Wed, 13 Nov 2024 17:17:12 +0100 Subject: [PATCH 1/4] [`ruff`] Implement `unnecessary-nested-literal` (`RUF039`) --- .../resources/test/fixtures/ruff/RUF039.py | 31 ++ .../resources/test/fixtures/ruff/RUF039.pyi | 31 ++ .../src/checkers/ast/analyze/expression.rs | 10 +- crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 2 + .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../ruff/rules/unnecessary_nested_literal.rs | 129 ++++++++ ..._rules__ruff__tests__RUF039_RUF039.py.snap | 278 ++++++++++++++++++ ...rules__ruff__tests__RUF039_RUF039.pyi.snap | 278 ++++++++++++++++++ ruff.schema.json | 1 + 10 files changed, 761 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF039.py create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF039.pyi create mode 100644 crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.py.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.pyi.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.py new file mode 100644 index 0000000000000..44adc4e364864 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.py @@ -0,0 +1,31 @@ +from typing import Literal +import typing as t +import typing_extensions + + +y: Literal[1, print("hello"), 3, Literal[4, 1]] +Literal[1, Literal[1]] +Literal[1, 2, Literal[1, 2]] +Literal[1, Literal[1], Literal[1]] +Literal[1, Literal[2], Literal[2]] +t.Literal[1, t.Literal[2, t.Literal[1]]] +Literal[ + 1, # comment 1 + Literal[ # another comment + 1 # yet annother comment + ] +] # once + +# Ensure issue is only raised once, even on nested literals +MyType = Literal["foo", Literal[True, False, True], "bar"] + +# nested literals, all equivalent to `Literal[1]` +Literal[Literal[1]] +Literal[Literal[Literal[1], Literal[1]]] +Literal[Literal[1], Literal[Literal[Literal[1]]]] + +# OK +x: Literal[True, False, True, False] +z: Literal[{1, 3, 5}, "foobar", {1,3,5}] +typing_extensions.Literal[1, 1, 1] +n: Literal["No", "duplicates", "here", 1, "1"] diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.pyi b/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.pyi new file mode 100644 index 0000000000000..44adc4e364864 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF039.pyi @@ -0,0 +1,31 @@ +from typing import Literal +import typing as t +import typing_extensions + + +y: Literal[1, print("hello"), 3, Literal[4, 1]] +Literal[1, Literal[1]] +Literal[1, 2, Literal[1, 2]] +Literal[1, Literal[1], Literal[1]] +Literal[1, Literal[2], Literal[2]] +t.Literal[1, t.Literal[2, t.Literal[1]]] +Literal[ + 1, # comment 1 + Literal[ # another comment + 1 # yet annother comment + ] +] # once + +# Ensure issue is only raised once, even on nested literals +MyType = Literal["foo", Literal[True, False, True], "bar"] + +# nested literals, all equivalent to `Literal[1]` +Literal[Literal[1]] +Literal[Literal[Literal[1], Literal[1]]] +Literal[Literal[1], Literal[Literal[Literal[1]]]] + +# OK +x: Literal[True, False, True, False] +z: Literal[{1, 3, 5}, "foobar", {1,3,5}] +typing_extensions.Literal[1, 1, 1] +n: Literal["No", "duplicates", "here", 1, "1"] diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 74f860d42998c..db73573d05a98 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -100,9 +100,15 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } // Ex) Literal[...] - if checker.enabled(Rule::DuplicateLiteralMember) { + if checker.any_enabled(&[Rule::DuplicateLiteralMember, Rule::UnnecessaryNestedLiteral]) + { if !checker.semantic.in_nested_literal() { - flake8_pyi::rules::duplicate_literal_member(checker, expr); + if checker.enabled(Rule::DuplicateLiteralMember) { + flake8_pyi::rules::duplicate_literal_member(checker, expr); + } + if checker.enabled(Rule::UnnecessaryNestedLiteral) { + ruff::rules::unnecessary_nested_literal(checker, expr); + } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5d676033ad186..666e67521a01d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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, "039") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryNestedLiteral), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 19917696691b0..b21a498628baf 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/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::UnnecessaryNestedLiteral, Path::new("RUF039.py"))] + #[test_case(Rule::UnnecessaryNestedLiteral, Path::new("RUF039.pyi"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 102174f8e32d2..bd532f676910b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -28,6 +28,7 @@ pub(crate) use static_key_dict_comprehension::*; pub(crate) use test_rules::*; pub(crate) use unnecessary_iterable_allocation_for_first_element::*; pub(crate) use unnecessary_key_check::*; +pub(crate) use unnecessary_nested_literal::*; pub(crate) use unsafe_markup_use::*; pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; @@ -68,6 +69,7 @@ mod suppression_comment_visitor; pub(crate) mod test_rules; mod unnecessary_iterable_allocation_for_first_element; mod unnecessary_key_check; +mod unnecessary_nested_literal; mod unsafe_markup_use; mod unused_async; mod unused_noqa; diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs new file mode 100644 index 0000000000000..68ef313c0a01f --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs @@ -0,0 +1,129 @@ +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, ExprContext, ExprSubscript, ExprTuple}; +use ruff_python_semantic::analyze::typing::traverse_literal; +use ruff_text_size::{Ranged, TextRange}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for unnecessary nested `Literal`. +/// +/// ## Why is this bad? +/// Parameterization of literals by other literals is supported as an ergonomic +/// feature as proposed in [PEP 586], to enable patterns such as: +/// ```python +/// ReadOnlyMode = Literal["r", "r+"] +/// WriteAndTruncateMode = Literal["w", "w+", "wt", "w+t"] +/// WriteNoTruncateMode = Literal["r+", "r+t"] +/// AppendMode = Literal["a", "a+", "at", "a+t"] +/// +/// AllModes = Literal[ReadOnlyMode, WriteAndTruncateMode, +/// WriteNoTruncateMode, AppendMode] +/// ``` +/// +/// As a consequence, type checkers also support nesting of literals +/// which is less readable than a flat `Literal`: +/// ```python +/// AllModes = Literal[Literal["r", "r+"], Literal["w", "w+", "wt", "w+t"], +/// Literal["r+", "r+t"], Literal["a", "a+", "at", "a+t"]] +/// ``` +/// +/// ## Example +/// ```python +/// AllModes = Literal[ +/// Literal["r", "r+"], +/// Literal["w", "w+", "wt", "w+t"], +/// Literal["r+", "r+t"], +/// Literal["a", "a+", "at", "a+t"], +/// ] +/// ``` +/// +/// Use instead: +/// ```python +/// AllModes = Literal[ +/// "r", "r+", "w", "w+", "wt", "w+t", "r+", "r+t", "a", "a+", "at", "a+t" +/// ] +/// ``` +/// +/// or assign the literal to a variable as in the first example. +/// +/// ## Fix safety +/// The fix for this rule is marked as unsafe when the `Literal` contains comments. +/// +/// ## 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) +/// +/// [PEP 586](https://peps.python.org/pep-0586/) +#[violation] +pub struct UnnecessaryNestedLiteral; + +impl Violation for UnnecessaryNestedLiteral { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "Unnecessary nested `Literal`".to_string() + } + + fn fix_title(&self) -> Option { + Some("Replace with flattened `Literal`".to_string()) + } +} + +/// RUF039 +pub(crate) fn unnecessary_nested_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) { + let mut nodes: Vec<&Expr> = Vec::new(); + let mut is_nested = false; + + let mut check_for_duplicate_members = |expr: &'a Expr, parent: &'a Expr| { + // If the parent is not equal to the `literal_expr` then we know we are traversing recursively. + if parent != literal_expr { + is_nested = true; + }; + nodes.push(expr); + }; + + // Traverse the type expressions in the `Literal`. + traverse_literal( + &mut check_for_duplicate_members, + checker.semantic(), + literal_expr, + ); + + if !is_nested { + return; + } + + let mut diagnostic = Diagnostic::new(UnnecessaryNestedLiteral {}, literal_expr.range()); + + // Create a [`Fix`] that flattens all nodes. + if let Expr::Subscript(subscript) = literal_expr { + let subscript = Expr::Subscript(ExprSubscript { + slice: Box::new(if let [elt] = nodes.as_slice() { + (*elt).clone() + } else { + Expr::Tuple(ExprTuple { + elts: nodes.into_iter().cloned().collect(), + range: TextRange::default(), + ctx: ExprContext::Load, + parenthesized: false, + }) + }), + value: subscript.value.clone(), + range: TextRange::default(), + ctx: ExprContext::Load, + }); + let fix = Fix::applicable_edit( + Edit::range_replacement(checker.generator().expr(&subscript), literal_expr.range()), + if checker.comment_ranges().intersects(literal_expr.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }, + ); + diagnostic.set_fix(fix); + }; + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.py.snap new file mode 100644 index 0000000000000..324f629e99ba0 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.py.snap @@ -0,0 +1,278 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF039.py:6:4: RUF039 [*] Unnecessary nested `Literal` + | +6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +7 | Literal[1, Literal[1]] +8 | Literal[1, 2, Literal[1, 2]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +3 3 | import typing_extensions +4 4 | +5 5 | +6 |-y: Literal[1, print("hello"), 3, Literal[4, 1]] + 6 |+y: Literal[1, print("hello"), 3, 4, 1] +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] + +RUF039.py:7:1: RUF039 [*] Unnecessary nested `Literal` + | +6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 | Literal[1, Literal[1]] + | ^^^^^^^^^^^^^^^^^^^^^^ RUF039 +8 | Literal[1, 2, Literal[1, 2]] +9 | Literal[1, Literal[1], Literal[1]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +4 4 | +5 5 | +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 |-Literal[1, Literal[1]] + 7 |+Literal[1, 1] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] + +RUF039.py:8:1: RUF039 [*] Unnecessary nested `Literal` + | + 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] + 7 | Literal[1, Literal[1]] + 8 | Literal[1, 2, Literal[1, 2]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +5 5 | +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 7 | Literal[1, Literal[1]] +8 |-Literal[1, 2, Literal[1, 2]] + 8 |+Literal[1, 2, 1, 2] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + +RUF039.py:9:1: RUF039 [*] Unnecessary nested `Literal` + | + 7 | Literal[1, Literal[1]] + 8 | Literal[1, 2, Literal[1, 2]] + 9 | Literal[1, Literal[1], Literal[1]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 |-Literal[1, Literal[1], Literal[1]] + 9 |+Literal[1, 1, 1] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 12 | Literal[ + +RUF039.py:10:1: RUF039 [*] Unnecessary nested `Literal` + | + 8 | Literal[1, 2, Literal[1, 2]] + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 | Literal[ + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 |-Literal[1, Literal[2], Literal[2]] + 10 |+Literal[1, 2, 2] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 12 | Literal[ +13 13 | 1, # comment 1 + +RUF039.py:11:1: RUF039 [*] Unnecessary nested `Literal` + | + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +12 | Literal[ +13 | 1, # comment 1 + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 |-t.Literal[1, t.Literal[2, t.Literal[1]]] + 11 |+t.Literal[1, 2, 1] +12 12 | Literal[ +13 13 | 1, # comment 1 +14 14 | Literal[ # another comment + +RUF039.py:12:1: RUF039 [*] Unnecessary nested `Literal` + | +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 | / Literal[ +13 | | 1, # comment 1 +14 | | Literal[ # another comment +15 | | 1 # yet annother comment +16 | | ] +17 | | ] # once + | |_^ RUF039 +18 | +19 | # Ensure issue is only raised once, even on nested literals + | + = help: Replace with flattened `Literal` + +ℹ Unsafe fix +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 |-Literal[ +13 |- 1, # comment 1 +14 |- Literal[ # another comment +15 |- 1 # yet annother comment +16 |- ] +17 |-] # once + 12 |+Literal[1, 1] # once +18 13 | +19 14 | # Ensure issue is only raised once, even on nested literals +20 15 | MyType = Literal["foo", Literal[True, False, True], "bar"] + +RUF039.py:20:10: RUF039 [*] Unnecessary nested `Literal` + | +19 | # Ensure issue is only raised once, even on nested literals +20 | MyType = Literal["foo", Literal[True, False, True], "bar"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +21 | +22 | # nested literals, all equivalent to `Literal[1]` + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +17 17 | ] # once +18 18 | +19 19 | # Ensure issue is only raised once, even on nested literals +20 |-MyType = Literal["foo", Literal[True, False, True], "bar"] + 20 |+MyType = Literal["foo", True, False, True, "bar"] +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] + +RUF039.py:23:1: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] + | ^^^^^^^^^^^^^^^^^^^ RUF039 +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +20 20 | MyType = Literal["foo", Literal[True, False, True], "bar"] +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 |-Literal[Literal[1]] + 23 |+Literal[1] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | + +RUF039.py:24:1: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 |-Literal[Literal[Literal[1], Literal[1]]] + 24 |+Literal[1, 1] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | +27 27 | # OK + +RUF039.py:24:9: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 |-Literal[Literal[Literal[1], Literal[1]]] + 24 |+Literal[Literal[1, 1]] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | +27 27 | # OK + +RUF039.py:25:1: RUF039 [*] Unnecessary nested `Literal` + | +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +26 | +27 | # OK + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 |-Literal[Literal[1], Literal[Literal[Literal[1]]]] + 25 |+Literal[1, 1] +26 26 | +27 27 | # OK +28 28 | x: Literal[True, False, True, False] + +RUF039.py:25:29: RUF039 [*] Unnecessary nested `Literal` + | +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | ^^^^^^^^^^^^^^^^^^^ RUF039 +26 | +27 | # OK + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 |-Literal[Literal[1], Literal[Literal[Literal[1]]]] + 25 |+Literal[Literal[1], Literal[Literal[1]]] +26 26 | +27 27 | # OK +28 28 | x: Literal[True, False, True, False] diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.pyi.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.pyi.snap new file mode 100644 index 0000000000000..add8b63119332 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF039_RUF039.pyi.snap @@ -0,0 +1,278 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF039.pyi:6:4: RUF039 [*] Unnecessary nested `Literal` + | +6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +7 | Literal[1, Literal[1]] +8 | Literal[1, 2, Literal[1, 2]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +3 3 | import typing_extensions +4 4 | +5 5 | +6 |-y: Literal[1, print("hello"), 3, Literal[4, 1]] + 6 |+y: Literal[1, print("hello"), 3, 4, 1] +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] + +RUF039.pyi:7:1: RUF039 [*] Unnecessary nested `Literal` + | +6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 | Literal[1, Literal[1]] + | ^^^^^^^^^^^^^^^^^^^^^^ RUF039 +8 | Literal[1, 2, Literal[1, 2]] +9 | Literal[1, Literal[1], Literal[1]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +4 4 | +5 5 | +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 |-Literal[1, Literal[1]] + 7 |+Literal[1, 1] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] + +RUF039.pyi:8:1: RUF039 [*] Unnecessary nested `Literal` + | + 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] + 7 | Literal[1, Literal[1]] + 8 | Literal[1, 2, Literal[1, 2]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +5 5 | +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 7 | Literal[1, Literal[1]] +8 |-Literal[1, 2, Literal[1, 2]] + 8 |+Literal[1, 2, 1, 2] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + +RUF039.pyi:9:1: RUF039 [*] Unnecessary nested `Literal` + | + 7 | Literal[1, Literal[1]] + 8 | Literal[1, 2, Literal[1, 2]] + 9 | Literal[1, Literal[1], Literal[1]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +6 6 | y: Literal[1, print("hello"), 3, Literal[4, 1]] +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 |-Literal[1, Literal[1], Literal[1]] + 9 |+Literal[1, 1, 1] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 12 | Literal[ + +RUF039.pyi:10:1: RUF039 [*] Unnecessary nested `Literal` + | + 8 | Literal[1, 2, Literal[1, 2]] + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 | Literal[ + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +7 7 | Literal[1, Literal[1]] +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 |-Literal[1, Literal[2], Literal[2]] + 10 |+Literal[1, 2, 2] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 12 | Literal[ +13 13 | 1, # comment 1 + +RUF039.pyi:11:1: RUF039 [*] Unnecessary nested `Literal` + | + 9 | Literal[1, Literal[1], Literal[1]] +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +12 | Literal[ +13 | 1, # comment 1 + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +8 8 | Literal[1, 2, Literal[1, 2]] +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 |-t.Literal[1, t.Literal[2, t.Literal[1]]] + 11 |+t.Literal[1, 2, 1] +12 12 | Literal[ +13 13 | 1, # comment 1 +14 14 | Literal[ # another comment + +RUF039.pyi:12:1: RUF039 [*] Unnecessary nested `Literal` + | +10 | Literal[1, Literal[2], Literal[2]] +11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 | / Literal[ +13 | | 1, # comment 1 +14 | | Literal[ # another comment +15 | | 1 # yet annother comment +16 | | ] +17 | | ] # once + | |_^ RUF039 +18 | +19 | # Ensure issue is only raised once, even on nested literals + | + = help: Replace with flattened `Literal` + +ℹ Unsafe fix +9 9 | Literal[1, Literal[1], Literal[1]] +10 10 | Literal[1, Literal[2], Literal[2]] +11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]] +12 |-Literal[ +13 |- 1, # comment 1 +14 |- Literal[ # another comment +15 |- 1 # yet annother comment +16 |- ] +17 |-] # once + 12 |+Literal[1, 1] # once +18 13 | +19 14 | # Ensure issue is only raised once, even on nested literals +20 15 | MyType = Literal["foo", Literal[True, False, True], "bar"] + +RUF039.pyi:20:10: RUF039 [*] Unnecessary nested `Literal` + | +19 | # Ensure issue is only raised once, even on nested literals +20 | MyType = Literal["foo", Literal[True, False, True], "bar"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +21 | +22 | # nested literals, all equivalent to `Literal[1]` + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +17 17 | ] # once +18 18 | +19 19 | # Ensure issue is only raised once, even on nested literals +20 |-MyType = Literal["foo", Literal[True, False, True], "bar"] + 20 |+MyType = Literal["foo", True, False, True, "bar"] +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] + +RUF039.pyi:23:1: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] + | ^^^^^^^^^^^^^^^^^^^ RUF039 +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +20 20 | MyType = Literal["foo", Literal[True, False, True], "bar"] +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 |-Literal[Literal[1]] + 23 |+Literal[1] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | + +RUF039.pyi:24:1: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 |-Literal[Literal[Literal[1], Literal[1]]] + 24 |+Literal[1, 1] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | +27 27 | # OK + +RUF039.pyi:24:9: RUF039 [*] Unnecessary nested `Literal` + | +22 | # nested literals, all equivalent to `Literal[1]` +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +21 21 | +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 |-Literal[Literal[Literal[1], Literal[1]]] + 24 |+Literal[Literal[1, 1]] +25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] +26 26 | +27 27 | # OK + +RUF039.pyi:25:1: RUF039 [*] Unnecessary nested `Literal` + | +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039 +26 | +27 | # OK + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 |-Literal[Literal[1], Literal[Literal[Literal[1]]]] + 25 |+Literal[1, 1] +26 26 | +27 27 | # OK +28 28 | x: Literal[True, False, True, False] + +RUF039.pyi:25:29: RUF039 [*] Unnecessary nested `Literal` + | +23 | Literal[Literal[1]] +24 | Literal[Literal[Literal[1], Literal[1]]] +25 | Literal[Literal[1], Literal[Literal[Literal[1]]]] + | ^^^^^^^^^^^^^^^^^^^ RUF039 +26 | +27 | # OK + | + = help: Replace with flattened `Literal` + +ℹ Safe fix +22 22 | # nested literals, all equivalent to `Literal[1]` +23 23 | Literal[Literal[1]] +24 24 | Literal[Literal[Literal[1], Literal[1]]] +25 |-Literal[Literal[1], Literal[Literal[Literal[1]]]] + 25 |+Literal[Literal[1], Literal[Literal[1]]] +26 26 | +27 27 | # OK +28 28 | x: Literal[True, False, True, False] diff --git a/ruff.schema.json b/ruff.schema.json index e816780e4f495..a48e8ae2afcf1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3833,6 +3833,7 @@ "RUF033", "RUF034", "RUF035", + "RUF039", "RUF1", "RUF10", "RUF100", From 1bee19abf9a6d64461d6f56a44f108ceb1e7f6a2 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Thu, 14 Nov 2024 08:55:14 +0100 Subject: [PATCH 2/4] Update crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs Co-authored-by: Micha Reiser --- .../src/rules/ruff/rules/unnecessary_nested_literal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs index 68ef313c0a01f..2301998694f8f 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs @@ -95,7 +95,7 @@ pub(crate) fn unnecessary_nested_literal<'a>(checker: &mut Checker, literal_expr return; } - let mut diagnostic = Diagnostic::new(UnnecessaryNestedLiteral {}, literal_expr.range()); + let mut diagnostic = Diagnostic::new(UnnecessaryNestedLiteral, literal_expr.range()); // Create a [`Fix`] that flattens all nodes. if let Expr::Subscript(subscript) = literal_expr { From 069327de0121fbfeb428a7fbe9ad01e88fd3f2d5 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Thu, 14 Nov 2024 16:28:30 +0100 Subject: [PATCH 3/4] Update crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs Co-authored-by: Alex Waygood --- .../src/rules/ruff/rules/unnecessary_nested_literal.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs index 2301998694f8f..8ec3cedcf7785 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs @@ -49,7 +49,8 @@ use crate::checkers::ast::Checker; /// or assign the literal to a variable as in the first example. /// /// ## Fix safety -/// The fix for this rule is marked as unsafe when the `Literal` contains comments. +/// The fix for this rule is marked as unsafe when the `Literal` slice is split +/// across multiple lines and some of the lines have trailing comments. /// /// ## 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) From f5c223e27189eebd60a4704a7a1ab3b8ce2b1e19 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Thu, 14 Nov 2024 16:36:46 +0100 Subject: [PATCH 4/4] Tweaks. --- .../src/rules/ruff/rules/unnecessary_nested_literal.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs index 8ec3cedcf7785..d0e094b27e193 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{Expr, ExprContext, ExprSubscript, ExprTuple}; +use ruff_python_ast::{AnyNodeRef, Expr, ExprContext, ExprSubscript, ExprTuple}; use ruff_python_semantic::analyze::typing::traverse_literal; use ruff_text_size::{Ranged, TextRange}; @@ -10,6 +10,8 @@ use crate::checkers::ast::Checker; /// Checks for unnecessary nested `Literal`. /// /// ## Why is this bad? +/// Prefer using a single `Literal`, which is equivalent and more concise. +/// /// Parameterization of literals by other literals is supported as an ergonomic /// feature as proposed in [PEP 586], to enable patterns such as: /// ```python @@ -79,7 +81,7 @@ pub(crate) fn unnecessary_nested_literal<'a>(checker: &mut Checker, literal_expr let mut check_for_duplicate_members = |expr: &'a Expr, parent: &'a Expr| { // If the parent is not equal to the `literal_expr` then we know we are traversing recursively. - if parent != literal_expr { + if !AnyNodeRef::ptr_eq(parent.into(), literal_expr.into()) { is_nested = true; }; nodes.push(expr);