From 9646991674110aee19ee794aab1fc75e94e7d280 Mon Sep 17 00:00:00 2001 From: Kaufman Dmitriy Date: Sat, 6 Jun 2026 18:59:17 +0300 Subject: [PATCH 1/2] [pylint] Ignore mutable type updates in redefined-loop-name (PLW2901) --- .../fixtures/pylint/redefined_loop_name.py | 40 ++++++++++++++ .../rules/pylint/rules/redefined_loop_name.rs | 53 +++++++++++++++---- ...tests__PLW2901_redefined_loop_name.py.snap | 44 +++++++++++++++ 3 files changed, 127 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py index 6b9b499714a405..7bace087d34c62 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py @@ -178,3 +178,43 @@ class A: a. i = 2 # error for a. i in []: a.i = 2 # error + +# For -> augmented assignment with list (in-place update) +for i in []: + i += [1] # no error + +# For -> normal assignment with list (not an in-place update) +for i in []: + i = [1] # error + +# For -> augmented assignment with dict (in-place update) +for i in []: + i |= {"a": 1} # no error + +# For -> augmented assignment with set (in-place update) +for i in []: + i |= {1} # no error + +# For -> augmented assignment with list comprehension (in-place update) +for i in []: + i += [x for x in ()] # no error + +# For -> augmented assignment with dict comprehension (in-place update) +for i in []: + i |= {x: x for x in ()} # no error + +# For -> augmented assignment with set comprehension (in-place update) +for i in []: + i |= {x for x in ()} # no error + +# For -> normal assignment with set comprehension (not an in-place update) +for i in []: + i = {x for x in ()} # error + +# For -> augmented assignment with immutable type (tuple) +for i in []: + i += (1,) # error + +# For -> augmented assignment with immutable type (string) +for i in []: + i += "a" # error diff --git a/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs index b77f07f2a22c2d..e5fb8ada643cea 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs @@ -113,6 +113,7 @@ enum InnerBindingKind { For, With, Assignment, + AugAssignment, } impl fmt::Display for InnerBindingKind { @@ -121,6 +122,7 @@ impl fmt::Display for InnerBindingKind { InnerBindingKind::For => fmt.write_str("`for` loop"), InnerBindingKind::With => fmt.write_str("`with` statement"), InnerBindingKind::Assignment => fmt.write_str("assignment"), + InnerBindingKind::AugAssignment => fmt.write_str("assignment"), } } } @@ -142,6 +144,7 @@ struct ExprWithOuterBindingKind<'a> { struct ExprWithInnerBindingKind<'a> { expr: &'a Expr, + value: Option<&'a Expr>, binding_kind: InnerBindingKind, } @@ -160,6 +163,7 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, + value: None, binding_kind: InnerBindingKind::For, } }), @@ -170,6 +174,7 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_with_items(items, self.dummy_variable_rgx).map( |expr| ExprWithInnerBindingKind { expr, + value: None, binding_kind: InnerBindingKind::With, }, ), @@ -188,17 +193,19 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_assign_targets(targets, self.dummy_variable_rgx).map( |expr| ExprWithInnerBindingKind { expr, + value: Some(value), binding_kind: InnerBindingKind::Assignment, }, ), ); } - Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => { + Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { self.assignment_targets.extend( assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, - binding_kind: InnerBindingKind::Assignment, + value: Some(value), + binding_kind: InnerBindingKind::AugAssignment, } }), ); @@ -211,6 +218,7 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, + value: value.as_deref(), binding_kind: InnerBindingKind::Assignment, } }), @@ -348,6 +356,26 @@ fn assignment_targets_from_assign_targets<'a>( .flat_map(|target| assignment_targets_from_expr(target, dummy_variable_rgx)) } +/// Returns `true` if the expression appears to be an in-place mutation (e.g., `x += [1]`). +/// +/// Since we lack full type inference, this uses a heuristic: if it is an augmented +/// assignment (`+=`, `|=`) and the right side is a mutable type (list, set, dict), +/// we assume the loop variable is being mutated in-place rather than overwritten. +fn is_mutable_type_update(value: Option<&Expr>, assignment: InnerBindingKind) -> bool { + let is_mutable = matches!( + value, + Some( + Expr::Dict(_) + | Expr::List(_) + | Expr::Set(_) + | Expr::DictComp(_) + | Expr::ListComp(_) + | Expr::SetComp(_) + ) + ); + is_mutable && assignment == InnerBindingKind::AugAssignment +} + /// PLW2901 pub(crate) fn redefined_loop_name(checker: &Checker, stmt: &Stmt) { let (outer_assignment_targets, inner_assignment_targets) = match stmt { @@ -396,14 +424,19 @@ pub(crate) fn redefined_loop_name(checker: &Checker, stmt: &Stmt) { if ComparableExpr::from(outer_assignment_target.expr) .eq(&(ComparableExpr::from(inner_assignment_target.expr))) { - checker.report_diagnostic( - RedefinedLoopName { - name: checker.generator().expr(outer_assignment_target.expr), - outer_kind: outer_assignment_target.binding_kind, - inner_kind: inner_assignment_target.binding_kind, - }, - inner_assignment_target.expr.range(), - ); + if !is_mutable_type_update( + inner_assignment_target.value, + inner_assignment_target.binding_kind, + ) { + checker.report_diagnostic( + RedefinedLoopName { + name: checker.generator().expr(outer_assignment_target.expr), + outer_kind: outer_assignment_target.binding_kind, + inner_kind: inner_assignment_target.binding_kind, + }, + inner_assignment_target.expr.range(), + ); + } } } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap index 4a37f5facd13e8..838a35f5670143 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap @@ -274,4 +274,48 @@ PLW2901 `for` loop variable `a.i` overwritten by assignment target 179 | for a. i in []: 180 | a.i = 2 # error | ^^^ +181 | +182 | # For -> augmented assignment with list (in-place update) + | + +PLW2901 `for` loop variable `i` overwritten by assignment target + --> redefined_loop_name.py:188:5 + | +186 | # For -> normal assignment with list (not an in-place update) +187 | for i in []: +188 | i = [1] # error + | ^ +189 | +190 | # For -> augmented assignment with dict (in-place update) + | + +PLW2901 `for` loop variable `i` overwritten by assignment target + --> redefined_loop_name.py:212:5 + | +210 | # For -> normal assignment with set comprehension (not an in-place update) +211 | for i in []: +212 | i = {x for x in ()} # error + | ^ +213 | +214 | # For -> augmented assignment with immutable type (tuple) + | + +PLW2901 `for` loop variable `i` overwritten by assignment target + --> redefined_loop_name.py:216:5 + | +214 | # For -> augmented assignment with immutable type (tuple) +215 | for i in []: +216 | i += (1,) # error + | ^ +217 | +218 | # For -> augmented assignment with immutable type (string) + | + +PLW2901 `for` loop variable `i` overwritten by assignment target + --> redefined_loop_name.py:220:5 + | +218 | # For -> augmented assignment with immutable type (string) +219 | for i in []: +220 | i += "a" # error + | ^ | From befcb10811bbb4a7c2be3949323c9cdea22bafa1 Mon Sep 17 00:00:00 2001 From: Kaufman Dmitriy Date: Wed, 10 Jun 2026 22:05:01 +0300 Subject: [PATCH 2/2] Reused existing function, created mdtests --- .../mdtest/pylint/redefined-loop-name.md | 86 +++++++++++++++++++ .../fixtures/pylint/redefined_loop_name.py | 40 --------- .../rules/pylint/rules/redefined_loop_name.rs | 70 ++++++--------- ...tests__PLW2901_redefined_loop_name.py.snap | 44 ---------- 4 files changed, 113 insertions(+), 127 deletions(-) create mode 100644 crates/ruff_linter/resources/mdtest/pylint/redefined-loop-name.md diff --git a/crates/ruff_linter/resources/mdtest/pylint/redefined-loop-name.md b/crates/ruff_linter/resources/mdtest/pylint/redefined-loop-name.md new file mode 100644 index 00000000000000..6b360389cee554 --- /dev/null +++ b/crates/ruff_linter/resources/mdtest/pylint/redefined-loop-name.md @@ -0,0 +1,86 @@ +# `redefined-loop-name` (`PLW2901`) + +```toml +[lint] +select = ["PLW2901"] +``` + +## Augmented assignment + +Ignore in-place update of a mutable type. + +```py +for i in []: + i += [1] + +for i in []: + i = [1] # snapshot: redefined-loop-name + +for i in []: + i |= {"a": 1} + +for i in []: + i = {"b": 2} # snapshot: redefined-loop-name + +for i in []: + i |= {1} + +for i in []: + i &= {1} + +for i in []: + i ^= {1} + +for i in []: + i -= {1} + +for i in []: + i = {1} # snapshot: redefined-loop-name + +for i in []: + i += (1,) # snapshot: redefined-loop-name + +for i in []: + i += "a" # snapshot: redefined-loop-name +``` + +```snapshot +error[PLW2901]: `for` loop variable `i` overwritten by assignment target + --> src/mdtest_snippet.py:5:5 + | +5 | i = [1] # snapshot: redefined-loop-name + | ^ + | + + +error[PLW2901]: `for` loop variable `i` overwritten by assignment target + --> src/mdtest_snippet.py:11:5 + | +11 | i = {"b": 2} # snapshot: redefined-loop-name + | ^ + | + + +error[PLW2901]: `for` loop variable `i` overwritten by assignment target + --> src/mdtest_snippet.py:26:5 + | +26 | i = {1} # snapshot: redefined-loop-name + | ^ + | + + +error[PLW2901]: `for` loop variable `i` overwritten by assignment target + --> src/mdtest_snippet.py:29:5 + | +29 | i += (1,) # snapshot: redefined-loop-name + | ^ + | + + +error[PLW2901]: `for` loop variable `i` overwritten by assignment target + --> src/mdtest_snippet.py:32:5 + | +32 | i += "a" # snapshot: redefined-loop-name + | ^ + | +``` diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py index 7bace087d34c62..6b9b499714a405 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/redefined_loop_name.py @@ -178,43 +178,3 @@ class A: a. i = 2 # error for a. i in []: a.i = 2 # error - -# For -> augmented assignment with list (in-place update) -for i in []: - i += [1] # no error - -# For -> normal assignment with list (not an in-place update) -for i in []: - i = [1] # error - -# For -> augmented assignment with dict (in-place update) -for i in []: - i |= {"a": 1} # no error - -# For -> augmented assignment with set (in-place update) -for i in []: - i |= {1} # no error - -# For -> augmented assignment with list comprehension (in-place update) -for i in []: - i += [x for x in ()] # no error - -# For -> augmented assignment with dict comprehension (in-place update) -for i in []: - i |= {x: x for x in ()} # no error - -# For -> augmented assignment with set comprehension (in-place update) -for i in []: - i |= {x for x in ()} # no error - -# For -> normal assignment with set comprehension (not an in-place update) -for i in []: - i = {x for x in ()} # error - -# For -> augmented assignment with immutable type (tuple) -for i in []: - i += (1,) # error - -# For -> augmented assignment with immutable type (string) -for i in []: - i += "a" # error diff --git a/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs index e5fb8ada643cea..316412b733804d 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs @@ -7,6 +7,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt}; use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::analyze::typing::is_mutable_expr; use ruff_text_size::Ranged; use crate::Violation; @@ -113,7 +114,6 @@ enum InnerBindingKind { For, With, Assignment, - AugAssignment, } impl fmt::Display for InnerBindingKind { @@ -122,7 +122,6 @@ impl fmt::Display for InnerBindingKind { InnerBindingKind::For => fmt.write_str("`for` loop"), InnerBindingKind::With => fmt.write_str("`with` statement"), InnerBindingKind::Assignment => fmt.write_str("assignment"), - InnerBindingKind::AugAssignment => fmt.write_str("assignment"), } } } @@ -144,7 +143,6 @@ struct ExprWithOuterBindingKind<'a> { struct ExprWithInnerBindingKind<'a> { expr: &'a Expr, - value: Option<&'a Expr>, binding_kind: InnerBindingKind, } @@ -163,7 +161,6 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, - value: None, binding_kind: InnerBindingKind::For, } }), @@ -174,7 +171,6 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_with_items(items, self.dummy_variable_rgx).map( |expr| ExprWithInnerBindingKind { expr, - value: None, binding_kind: InnerBindingKind::With, }, ), @@ -193,19 +189,33 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_assign_targets(targets, self.dummy_variable_rgx).map( |expr| ExprWithInnerBindingKind { expr, - value: Some(value), binding_kind: InnerBindingKind::Assignment, }, ), ); } - Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { + Stmt::AugAssign(ast::StmtAugAssign { + target, value, op, .. + }) => { + // Check for in-place update of mutable type + if is_mutable_expr(value, self.context) + && matches!( + op, + ast::Operator::Add + | ast::Operator::Sub + | ast::Operator::BitOr + | ast::Operator::BitAnd + | ast::Operator::BitXor + ) + { + return; + } + self.assignment_targets.extend( assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, - value: Some(value), - binding_kind: InnerBindingKind::AugAssignment, + binding_kind: InnerBindingKind::Assignment, } }), ); @@ -218,7 +228,6 @@ impl<'b> StatementVisitor<'b> for InnerForWithAssignTargetsVisitor<'_, 'b> { assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { expr, - value: value.as_deref(), binding_kind: InnerBindingKind::Assignment, } }), @@ -356,26 +365,6 @@ fn assignment_targets_from_assign_targets<'a>( .flat_map(|target| assignment_targets_from_expr(target, dummy_variable_rgx)) } -/// Returns `true` if the expression appears to be an in-place mutation (e.g., `x += [1]`). -/// -/// Since we lack full type inference, this uses a heuristic: if it is an augmented -/// assignment (`+=`, `|=`) and the right side is a mutable type (list, set, dict), -/// we assume the loop variable is being mutated in-place rather than overwritten. -fn is_mutable_type_update(value: Option<&Expr>, assignment: InnerBindingKind) -> bool { - let is_mutable = matches!( - value, - Some( - Expr::Dict(_) - | Expr::List(_) - | Expr::Set(_) - | Expr::DictComp(_) - | Expr::ListComp(_) - | Expr::SetComp(_) - ) - ); - is_mutable && assignment == InnerBindingKind::AugAssignment -} - /// PLW2901 pub(crate) fn redefined_loop_name(checker: &Checker, stmt: &Stmt) { let (outer_assignment_targets, inner_assignment_targets) = match stmt { @@ -424,19 +413,14 @@ pub(crate) fn redefined_loop_name(checker: &Checker, stmt: &Stmt) { if ComparableExpr::from(outer_assignment_target.expr) .eq(&(ComparableExpr::from(inner_assignment_target.expr))) { - if !is_mutable_type_update( - inner_assignment_target.value, - inner_assignment_target.binding_kind, - ) { - checker.report_diagnostic( - RedefinedLoopName { - name: checker.generator().expr(outer_assignment_target.expr), - outer_kind: outer_assignment_target.binding_kind, - inner_kind: inner_assignment_target.binding_kind, - }, - inner_assignment_target.expr.range(), - ); - } + checker.report_diagnostic( + RedefinedLoopName { + name: checker.generator().expr(outer_assignment_target.expr), + outer_kind: outer_assignment_target.binding_kind, + inner_kind: inner_assignment_target.binding_kind, + }, + inner_assignment_target.expr.range(), + ); } } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap index 838a35f5670143..4a37f5facd13e8 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap @@ -274,48 +274,4 @@ PLW2901 `for` loop variable `a.i` overwritten by assignment target 179 | for a. i in []: 180 | a.i = 2 # error | ^^^ -181 | -182 | # For -> augmented assignment with list (in-place update) - | - -PLW2901 `for` loop variable `i` overwritten by assignment target - --> redefined_loop_name.py:188:5 - | -186 | # For -> normal assignment with list (not an in-place update) -187 | for i in []: -188 | i = [1] # error - | ^ -189 | -190 | # For -> augmented assignment with dict (in-place update) - | - -PLW2901 `for` loop variable `i` overwritten by assignment target - --> redefined_loop_name.py:212:5 - | -210 | # For -> normal assignment with set comprehension (not an in-place update) -211 | for i in []: -212 | i = {x for x in ()} # error - | ^ -213 | -214 | # For -> augmented assignment with immutable type (tuple) - | - -PLW2901 `for` loop variable `i` overwritten by assignment target - --> redefined_loop_name.py:216:5 - | -214 | # For -> augmented assignment with immutable type (tuple) -215 | for i in []: -216 | i += (1,) # error - | ^ -217 | -218 | # For -> augmented assignment with immutable type (string) - | - -PLW2901 `for` loop variable `i` overwritten by assignment target - --> redefined_loop_name.py:220:5 - | -218 | # For -> augmented assignment with immutable type (string) -219 | for i in []: -220 | i += "a" # error - | ^ |