[pylint] Extend self-assigning-variable to multi-target assignmen…

…ts (#8839) Closes #8667.
astral-sh · Nov 25, 2023 · 9b17724 · 9b17724
1 parent 0d4af9d
commit 9b17724
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 33 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/self_assigning_variable.py b/crates/ruff_linter/resources/test/fixtures/pylint/self_assigning_variable.py
@@ -23,6 +23,9 @@
 (foo, (bar, baz)) = (foo, (bar, 1))
 foo: int = foo
 bar: int = bar
+foo = foo = bar
+(foo, bar) = (foo, bar) = baz
+(foo, bar) = baz = (foo, bar) = 1
 
 # Non-errors.
 foo = bar

diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -1355,7 +1355,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             }
         }
         Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => {
-            checker.enabled(Rule::NonAsciiName);
             if checker.enabled(Rule::LambdaAssignment) {
                 if let [target] = &targets[..] {
                     pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt);
@@ -1407,9 +1406,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 }
             }
             if checker.settings.rules.enabled(Rule::SelfAssigningVariable) {
-                if let [target] = targets.as_slice() {
-                    pylint::rules::self_assigning_variable(checker, target, value);
-                }
+                pylint::rules::self_assignment(checker, assign);
             }
             if checker.settings.rules.enabled(Rule::TypeParamNameMismatch) {
                 pylint::rules::type_param_name_mismatch(checker, value, targets);
@@ -1479,9 +1476,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                         stmt,
                     );
                 }
-                if checker.enabled(Rule::SelfAssigningVariable) {
-                    pylint::rules::self_assigning_variable(checker, target, value);
-                }
+            }
+            if checker.enabled(Rule::SelfAssigningVariable) {
+                pylint::rules::self_annotated_assignment(checker, assign_stmt);
             }
             if checker.enabled(Rule::UnintentionalTypeAnnotation) {
                 flake8_bugbear::rules::unintentional_type_annotation(

diff --git a/crates/ruff_linter/src/rules/pylint/rules/self_assigning_variable.rs b/crates/ruff_linter/src/rules/pylint/rules/self_assigning_variable.rs
@@ -1,3 +1,4 @@
+use itertools::Itertools;
 use ruff_python_ast::{self as ast, Expr};
 
 use ruff_diagnostics::{Diagnostic, Violation};
@@ -36,36 +37,58 @@ impl Violation for SelfAssigningVariable {
 }
 
 /// PLW0127
-pub(crate) fn self_assigning_variable(checker: &mut Checker, target: &Expr, value: &Expr) {
-    fn inner(left: &Expr, right: &Expr, diagnostics: &mut Vec<Diagnostic>) {
-        match (left, right) {
-            (
-                Expr::Tuple(ast::ExprTuple { elts: lhs_elts, .. }),
-                Expr::Tuple(ast::ExprTuple { elts: rhs_elts, .. }),
-            ) if lhs_elts.len() == rhs_elts.len() => lhs_elts
-                .iter()
-                .zip(rhs_elts.iter())
-                .for_each(|(lhs, rhs)| inner(lhs, rhs, diagnostics)),
-            (
-                Expr::Name(ast::ExprName { id: lhs_name, .. }),
-                Expr::Name(ast::ExprName { id: rhs_name, .. }),
-            ) if lhs_name == rhs_name => {
-                diagnostics.push(Diagnostic::new(
-                    SelfAssigningVariable {
-                        name: lhs_name.to_string(),
-                    },
-                    left.range(),
-                ));
-            }
-            _ => {}
-        }
+pub(crate) fn self_assignment(checker: &mut Checker, assign: &ast::StmtAssign) {
+    // Assignments in class bodies are attributes (e.g., `x = x` assigns `x` to `self.x`, and thus
+    // is not a self-assignment).
+    if checker.semantic().current_scope().kind.is_class() {
+        return;
+    }
+
+    for (left, right) in assign
+        .targets
+        .iter()
+        .chain(std::iter::once(assign.value.as_ref()))
+        .tuple_combinations()
+    {
+        visit_assignments(left, right, &mut checker.diagnostics);
     }
+}
+
+/// PLW0127
+pub(crate) fn self_annotated_assignment(checker: &mut Checker, assign: &ast::StmtAnnAssign) {
+    let Some(value) = assign.value.as_ref() else {
+        return;
+    };
 
     // Assignments in class bodies are attributes (e.g., `x = x` assigns `x` to `self.x`, and thus
     // is not a self-assignment).
     if checker.semantic().current_scope().kind.is_class() {
         return;
     }
 
-    inner(target, value, &mut checker.diagnostics);
+    visit_assignments(&assign.target, value, &mut checker.diagnostics);
+}
+
+fn visit_assignments(left: &Expr, right: &Expr, diagnostics: &mut Vec<Diagnostic>) {
+    match (left, right) {
+        (
+            Expr::Tuple(ast::ExprTuple { elts: lhs_elts, .. }),
+            Expr::Tuple(ast::ExprTuple { elts: rhs_elts, .. }),
+        ) if lhs_elts.len() == rhs_elts.len() => lhs_elts
+            .iter()
+            .zip(rhs_elts.iter())
+            .for_each(|(lhs, rhs)| visit_assignments(lhs, rhs, diagnostics)),
+        (
+            Expr::Name(ast::ExprName { id: lhs_name, .. }),
+            Expr::Name(ast::ExprName { id: rhs_name, .. }),
+        ) if lhs_name == rhs_name => {
+            diagnostics.push(Diagnostic::new(
+                SelfAssigningVariable {
+                    name: lhs_name.to_string(),
+                },
+                left.range(),
+            ));
+        }
+        _ => {}
+    }
 }
diff --git a/...lint/snapshots/ruff_linter__rules__pylint__tests__PLW0127_self_assigning_variable.py.snap b/...lint/snapshots/ruff_linter__rules__pylint__tests__PLW0127_self_assigning_variable.py.snap
@@ -347,6 +347,7 @@ self_assigning_variable.py:24:1: PLW0127 Self-assignment of variable `foo`
 24 | foo: int = foo
    | ^^^ PLW0127
 25 | bar: int = bar
+26 | foo = foo = bar
    |
 
 self_assigning_variable.py:25:1: PLW0127 Self-assignment of variable `bar`
@@ -355,8 +356,56 @@ self_assigning_variable.py:25:1: PLW0127 Self-assignment of variable `bar`
 24 | foo: int = foo
 25 | bar: int = bar
    | ^^^ PLW0127
-26 | 
-27 | # Non-errors.
+26 | foo = foo = bar
+27 | (foo, bar) = (foo, bar) = baz
+   |
+
+self_assigning_variable.py:26:1: PLW0127 Self-assignment of variable `foo`
+   |
+24 | foo: int = foo
+25 | bar: int = bar
+26 | foo = foo = bar
+   | ^^^ PLW0127
+27 | (foo, bar) = (foo, bar) = baz
+28 | (foo, bar) = baz = (foo, bar) = 1
+   |
+
+self_assigning_variable.py:27:2: PLW0127 Self-assignment of variable `foo`
+   |
+25 | bar: int = bar
+26 | foo = foo = bar
+27 | (foo, bar) = (foo, bar) = baz
+   |  ^^^ PLW0127
+28 | (foo, bar) = baz = (foo, bar) = 1
+   |
+
+self_assigning_variable.py:27:7: PLW0127 Self-assignment of variable `bar`
+   |
+25 | bar: int = bar
+26 | foo = foo = bar
+27 | (foo, bar) = (foo, bar) = baz
+   |       ^^^ PLW0127
+28 | (foo, bar) = baz = (foo, bar) = 1
+   |
+
+self_assigning_variable.py:28:2: PLW0127 Self-assignment of variable `foo`
+   |
+26 | foo = foo = bar
+27 | (foo, bar) = (foo, bar) = baz
+28 | (foo, bar) = baz = (foo, bar) = 1
+   |  ^^^ PLW0127
+29 | 
+30 | # Non-errors.
+   |
+
+self_assigning_variable.py:28:7: PLW0127 Self-assignment of variable `bar`
+   |
+26 | foo = foo = bar
+27 | (foo, bar) = (foo, bar) = baz
+28 | (foo, bar) = baz = (foo, bar) = 1
+   |       ^^^ PLW0127
+29 | 
+30 | # Non-errors.
    |