Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
KaufmanDmitriy marked this conversation as resolved.
Outdated
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
53 changes: 43 additions & 10 deletions crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ enum InnerBindingKind {
For,
With,
Assignment,
AugAssignment,
}

impl fmt::Display for InnerBindingKind {
Expand All @@ -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"),
}
}
}
Expand All @@ -142,6 +144,7 @@ struct ExprWithOuterBindingKind<'a> {

struct ExprWithInnerBindingKind<'a> {
expr: &'a Expr,
value: Option<&'a Expr>,
binding_kind: InnerBindingKind,
}

Expand All @@ -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,
}
}),
Expand All @@ -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,
},
),
Expand All @@ -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, .. }) => {
Comment thread
KaufmanDmitriy marked this conversation as resolved.
Outdated
self.assignment_targets.extend(
assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| {
ExprWithInnerBindingKind {
expr,
binding_kind: InnerBindingKind::Assignment,
value: Some(value),
Comment thread
KaufmanDmitriy marked this conversation as resolved.
Outdated
binding_kind: InnerBindingKind::AugAssignment,
}
}),
);
Expand All @@ -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,
}
}),
Expand Down Expand Up @@ -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 {
Comment thread
KaufmanDmitriy marked this conversation as resolved.
Outdated
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 {
Expand Down Expand Up @@ -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(),
);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
| ^
|
Loading