From 740c08b033d835f071e4887cea2f608d6cc662c6 Mon Sep 17 00:00:00 2001 From: Steve C Date: Fri, 15 Mar 2024 10:43:55 -0400 Subject: [PATCH] [`pylint`] - implement `redeclared-assigned-name` (`W0128`) (#9268) ## Summary Implements [`W0128`/`redeclared-assigned-name`](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/redeclared-assigned-name.html) See: #970 ## Test Plan `cargo test` --- .../pylint/redeclared_assigned_name.py | 6 ++ .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + .../pylint/rules/redeclared_assigned_name.rs | 76 +++++++++++++++++++ ...__PLW0128_redeclared_assigned_name.py.snap | 59 ++++++++++++++ ruff.schema.json | 1 + 8 files changed, 149 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py new file mode 100644 index 0000000000000..50b3a27925c36 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py @@ -0,0 +1,6 @@ +FIRST, FIRST = (1, 2) # PLW0128 +FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + +FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 191456bf0766b..d4f6287bf95e9 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1389,6 +1389,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { + if checker.enabled(Rule::RedeclaredAssignedName) { + pylint::rules::redeclared_assigned_name(checker, targets); + } if checker.enabled(Rule::LambdaAssignment) { if let [target] = &targets[..] { pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c471c890a0f1a..e6900d54d24c5 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -294,6 +294,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0108") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryLambda), (Pylint, "W0120") => (RuleGroup::Stable, rules::pylint::rules::UselessElseOnLoop), (Pylint, "W0127") => (RuleGroup::Stable, rules::pylint::rules::SelfAssigningVariable), + (Pylint, "W0128") => (RuleGroup::Preview, rules::pylint::rules::RedeclaredAssignedName), (Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral), (Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext), (Pylint, "W0133") => (RuleGroup::Preview, rules::pylint::rules::UselessExceptionStatement), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 0cbd284ddbc8f..ebd20c35ff90c 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -95,6 +95,7 @@ mod tests { #[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))] #[test_case(Rule::NonSlotAssignment, Path::new("non_slot_assignment.py"))] #[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))] + #[test_case(Rule::RedeclaredAssignedName, Path::new("redeclared_assigned_name.py"))] #[test_case( Rule::RedefinedArgumentFromLocal, Path::new("redefined_argument_from_local.py") diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 8f6d40554d2ad..0f7c0e9e983ea 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -47,6 +47,7 @@ pub(crate) use non_slot_assignment::*; pub(crate) use nonlocal_without_binding::*; pub(crate) use potential_index_error::*; pub(crate) use property_with_parameters::*; +pub(crate) use redeclared_assigned_name::*; pub(crate) use redefined_argument_from_local::*; pub(crate) use redefined_loop_name::*; pub(crate) use repeated_equality_comparison::*; @@ -136,6 +137,7 @@ mod non_slot_assignment; mod nonlocal_without_binding; mod potential_index_error; mod property_with_parameters; +mod redeclared_assigned_name; mod redefined_argument_from_local; mod redefined_loop_name; mod repeated_equality_comparison; diff --git a/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs b/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs new file mode 100644 index 0000000000000..cb0dbc5175f2b --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs @@ -0,0 +1,76 @@ +use ruff_python_ast::{self as ast, Expr}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for declared assignments to the same variable multiple times +/// in the same assignment. +/// +/// ## Why is this bad? +/// Assigning a variable multiple times in the same assignment is redundant, +/// as the final assignment to the variable is what the value will be. +/// +/// ## Example +/// ```python +/// a, b, a = (1, 2, 3) +/// print(a) # 3 +/// ``` +/// +/// Use instead: +/// ```python +/// # this is assuming you want to assign 3 to `a` +/// _, b, a = (1, 2, 3) +/// print(a) # 3 +/// ``` +/// +#[violation] +pub struct RedeclaredAssignedName { + name: String, +} + +impl Violation for RedeclaredAssignedName { + #[derive_message_formats] + fn message(&self) -> String { + let RedeclaredAssignedName { name } = self; + format!("Redeclared variable `{name}` in assignment") + } +} + +/// PLW0128 +pub(crate) fn redeclared_assigned_name(checker: &mut Checker, targets: &Vec) { + let mut names: Vec = Vec::new(); + + for target in targets { + check_expr(checker, target, &mut names); + } +} + +fn check_expr(checker: &mut Checker, expr: &Expr, names: &mut Vec) { + match expr { + Expr::Tuple(ast::ExprTuple { elts, .. }) => { + for target in elts { + check_expr(checker, target, names); + } + } + Expr::Name(ast::ExprName { id, .. }) => { + if checker.settings.dummy_variable_rgx.is_match(id) { + // Ignore dummy variable assignments + return; + } + if names.contains(id) { + checker.diagnostics.push(Diagnostic::new( + RedeclaredAssignedName { + name: id.to_string(), + }, + expr.range(), + )); + } + names.push(id.to_string()); + } + _ => {} + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap new file mode 100644 index 0000000000000..eda5b8ea9f353 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap @@ -0,0 +1,59 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +redeclared_assigned_name.py:1:8: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 + | ^^^^^ PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | + +redeclared_assigned_name.py:2:9: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:3:9: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:3:32: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:4:23: PLW0128 Redeclared variable `FIRST` in assignment + | +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | ^^^^^ PLW0128 +5 | +6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK + | + +redeclared_assigned_name.py:4:30: PLW0128 Redeclared variable `SECOND` in assignment + | +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | ^^^^^^ PLW0128 +5 | +6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 7b65b8f158596..9f06af7bc68b0 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3330,6 +3330,7 @@ "PLW012", "PLW0120", "PLW0127", + "PLW0128", "PLW0129", "PLW013", "PLW0131",