diff --git a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py index b99bfe9323..6b9b499714 100644 --- a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py +++ b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py @@ -31,6 +31,21 @@ with None as i: with None as j: # ok pass +# Async with -> with, variable reused +async with None as i: + with None as i: # error + pass + +# Async with -> with, different variable +async with None as i: + with None as j: # ok + pass + +# Async for -> for, variable reused +async for i in []: + for i in []: # error + pass + # For -> for -> for, doubly nested variable reuse for i in []: for j in []: diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 6dd4cc885c..11c69d01af 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1483,7 +1483,7 @@ where ); } if self.enabled(Rule::RedefinedLoopName) { - pylint::rules::redefined_loop_name(self, &Node::Stmt(stmt)); + pylint::rules::redefined_loop_name(self, stmt); } } Stmt::While(ast::StmtWhile { body, orelse, .. }) => { @@ -1524,7 +1524,7 @@ where pylint::rules::useless_else_on_loop(self, stmt, body, orelse); } if self.enabled(Rule::RedefinedLoopName) { - pylint::rules::redefined_loop_name(self, &Node::Stmt(stmt)); + pylint::rules::redefined_loop_name(self, stmt); } if self.enabled(Rule::IterationOverSet) { pylint::rules::iteration_over_set(self, iter); diff --git a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs index 5a3573952d..0bdcb3ce9e 100644 --- a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs +++ b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs @@ -7,53 +7,10 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor}; -use ruff_python_ast::types::Node; use ruff_python_semantic::SemanticModel; use crate::checkers::ast::Checker; -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub(crate) enum OuterBindingKind { - For, - With, -} - -impl fmt::Display for OuterBindingKind { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self { - OuterBindingKind::For => fmt.write_str("`for` loop"), - OuterBindingKind::With => fmt.write_str("`with` statement"), - } - } -} - -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub(crate) enum InnerBindingKind { - For, - With, - Assignment, -} - -impl fmt::Display for InnerBindingKind { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self { - InnerBindingKind::For => fmt.write_str("`for` loop"), - InnerBindingKind::With => fmt.write_str("`with` statement"), - InnerBindingKind::Assignment => fmt.write_str("assignment"), - } - } -} - -impl PartialEq for OuterBindingKind { - fn eq(&self, other: &InnerBindingKind) -> bool { - matches!( - (self, other), - (OuterBindingKind::For, InnerBindingKind::For) - | (OuterBindingKind::With, InnerBindingKind::With) - ) - } -} - /// ## What it does /// Checks for variables defined in `for` loops and `with` statements that /// get overwritten within the body, for example by another `for` loop or @@ -128,6 +85,48 @@ impl Violation for RedefinedLoopName { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +enum OuterBindingKind { + For, + With, +} + +impl fmt::Display for OuterBindingKind { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + OuterBindingKind::For => fmt.write_str("`for` loop"), + OuterBindingKind::With => fmt.write_str("`with` statement"), + } + } +} + +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +enum InnerBindingKind { + For, + With, + Assignment, +} + +impl fmt::Display for InnerBindingKind { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + InnerBindingKind::For => fmt.write_str("`for` loop"), + InnerBindingKind::With => fmt.write_str("`with` statement"), + InnerBindingKind::Assignment => fmt.write_str("assignment"), + } + } +} + +impl PartialEq for OuterBindingKind { + fn eq(&self, other: &InnerBindingKind) -> bool { + matches!( + (self, other), + (OuterBindingKind::For, InnerBindingKind::For) + | (OuterBindingKind::With, InnerBindingKind::With) + ) + } +} + struct ExprWithOuterBindingKind<'a> { expr: &'a Expr, binding_kind: OuterBindingKind, @@ -255,10 +254,10 @@ fn assignment_is_cast_expr(value: &Expr, target: &Expr, semantic: &SemanticModel semantic.match_typing_expr(func, "cast") } -fn assignment_targets_from_expr<'a, U>( - expr: &'a Expr, +fn assignment_targets_from_expr<'a>( + expr: &'a Expr, dummy_variable_rgx: &'a Regex, -) -> Box> + 'a> { +) -> Box + 'a> { // The Box is necessary to ensure the match arms have the same return type - we can't use // a cast to "impl Iterator", since at the time of writing that is only allowed for // return types and argument types. @@ -308,77 +307,73 @@ fn assignment_targets_from_expr<'a, U>( } } -fn assignment_targets_from_with_items<'a, U>( - items: &'a [Withitem], +fn assignment_targets_from_with_items<'a>( + items: &'a [Withitem], dummy_variable_rgx: &'a Regex, -) -> impl Iterator> + 'a { +) -> impl Iterator + 'a { items .iter() .filter_map(|item| { item.optional_vars .as_ref() - .map(|expr| assignment_targets_from_expr(&**expr, dummy_variable_rgx)) + .map(|expr| assignment_targets_from_expr(expr, dummy_variable_rgx)) }) .flatten() } -fn assignment_targets_from_assign_targets<'a, U>( - targets: &'a [Expr], +fn assignment_targets_from_assign_targets<'a>( + targets: &'a [Expr], dummy_variable_rgx: &'a Regex, -) -> impl Iterator> + 'a { +) -> impl Iterator + 'a { targets .iter() .flat_map(|target| assignment_targets_from_expr(target, dummy_variable_rgx)) } /// PLW2901 -pub(crate) fn redefined_loop_name<'a, 'b>(checker: &'a mut Checker<'b>, node: &Node<'b>) { - let (outer_assignment_targets, inner_assignment_targets) = match node { - Node::Stmt(stmt) => match stmt { - // With. - Stmt::With(ast::StmtWith { items, body, .. }) => { - let outer_assignment_targets: Vec> = - assignment_targets_from_with_items(items, &checker.settings.dummy_variable_rgx) - .map(|expr| ExprWithOuterBindingKind { - expr, - binding_kind: OuterBindingKind::With, - }) - .collect(); - let mut visitor = InnerForWithAssignTargetsVisitor { - context: checker.semantic(), - dummy_variable_rgx: &checker.settings.dummy_variable_rgx, - assignment_targets: vec![], - }; - for stmt in body { - visitor.visit_stmt(stmt); - } - (outer_assignment_targets, visitor.assignment_targets) +pub(crate) fn redefined_loop_name(checker: &mut Checker, stmt: &Stmt) { + let (outer_assignment_targets, inner_assignment_targets) = match stmt { + Stmt::With(ast::StmtWith { items, body, .. }) + | Stmt::AsyncWith(ast::StmtAsyncWith { items, body, .. }) => { + let outer_assignment_targets: Vec = + assignment_targets_from_with_items(items, &checker.settings.dummy_variable_rgx) + .map(|expr| ExprWithOuterBindingKind { + expr, + binding_kind: OuterBindingKind::With, + }) + .collect(); + let mut visitor = InnerForWithAssignTargetsVisitor { + context: checker.semantic(), + dummy_variable_rgx: &checker.settings.dummy_variable_rgx, + assignment_targets: vec![], + }; + for stmt in body { + visitor.visit_stmt(stmt); } - // For and async for. - Stmt::For(ast::StmtFor { target, body, .. }) - | Stmt::AsyncFor(ast::StmtAsyncFor { target, body, .. }) => { - let outer_assignment_targets: Vec> = - assignment_targets_from_expr(target, &checker.settings.dummy_variable_rgx) - .map(|expr| ExprWithOuterBindingKind { - expr, - binding_kind: OuterBindingKind::For, - }) - .collect(); - let mut visitor = InnerForWithAssignTargetsVisitor { - context: checker.semantic(), - dummy_variable_rgx: &checker.settings.dummy_variable_rgx, - assignment_targets: vec![], - }; - for stmt in body { - visitor.visit_stmt(stmt); - } - (outer_assignment_targets, visitor.assignment_targets) + (outer_assignment_targets, visitor.assignment_targets) + } + Stmt::For(ast::StmtFor { target, body, .. }) + | Stmt::AsyncFor(ast::StmtAsyncFor { target, body, .. }) => { + let outer_assignment_targets: Vec = + assignment_targets_from_expr(target, &checker.settings.dummy_variable_rgx) + .map(|expr| ExprWithOuterBindingKind { + expr, + binding_kind: OuterBindingKind::For, + }) + .collect(); + let mut visitor = InnerForWithAssignTargetsVisitor { + context: checker.semantic(), + dummy_variable_rgx: &checker.settings.dummy_variable_rgx, + assignment_targets: vec![], + }; + for stmt in body { + visitor.visit_stmt(stmt); } - _ => panic!( - "redefined_loop_name called on Statement that is not a With, For, or AsyncFor" - ), - }, - Node::Expr(_) => panic!("redefined_loop_name called on Node that is not a Statement"), + (outer_assignment_targets, visitor.assignment_targets) + } + _ => panic!( + "redefined_loop_name called on Statement that is not a With, For, AsyncWith, or AsyncFor" + ) }; let mut diagnostics = Vec::new(); diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap index 10ef840c63..7ca951e257 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap @@ -37,197 +37,215 @@ redefined_loop_name.py:21:18: PLW2901 Outer `with` statement variable `i` overwr 22 | pass | -redefined_loop_name.py:37:13: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target +redefined_loop_name.py:36:18: PLW2901 Outer `with` statement variable `i` overwritten by inner `with` statement target | -35 | for i in []: -36 | for j in []: -37 | for i in []: # error - | ^ PLW2901 -38 | pass +34 | # Async with -> with, variable reused +35 | async with None as i: +36 | with None as i: # error + | ^ PLW2901 +37 | pass | -redefined_loop_name.py:43:13: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target +redefined_loop_name.py:46:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -41 | for i in []: -42 | for j in []: -43 | for i in []: # error - | ^ PLW2901 -44 | for j in []: # error -45 | pass +44 | # Async for -> for, variable reused +45 | async for i in []: +46 | for i in []: # error + | ^ PLW2901 +47 | pass | -redefined_loop_name.py:44:17: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target +redefined_loop_name.py:52:13: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -42 | for j in []: -43 | for i in []: # error -44 | for j in []: # error +50 | for i in []: +51 | for j in []: +52 | for i in []: # error + | ^ PLW2901 +53 | pass + | + +redefined_loop_name.py:58:13: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target + | +56 | for i in []: +57 | for j in []: +58 | for i in []: # error + | ^ PLW2901 +59 | for j in []: # error +60 | pass + | + +redefined_loop_name.py:59:17: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target + | +57 | for j in []: +58 | for i in []: # error +59 | for j in []: # error | ^ PLW2901 -45 | pass +60 | pass | -redefined_loop_name.py:52:5: PLW2901 `for` loop variable `i` overwritten by assignment target +redefined_loop_name.py:67:5: PLW2901 `for` loop variable `i` overwritten by assignment target | -50 | i = cast(int, i) -51 | i = typing.cast(int, i) -52 | i = 5 # error +65 | i = cast(int, i) +66 | i = typing.cast(int, i) +67 | i = 5 # error | ^ PLW2901 -53 | -54 | # For -> augmented assignment +68 | +69 | # For -> augmented assignment | -redefined_loop_name.py:56:5: PLW2901 `for` loop variable `i` overwritten by assignment target +redefined_loop_name.py:71:5: PLW2901 `for` loop variable `i` overwritten by assignment target | -54 | # For -> augmented assignment -55 | for i in []: -56 | i += 5 # error +69 | # For -> augmented assignment +70 | for i in []: +71 | i += 5 # error | ^ PLW2901 -57 | -58 | # For -> annotated assignment +72 | +73 | # For -> annotated assignment | -redefined_loop_name.py:60:5: PLW2901 `for` loop variable `i` overwritten by assignment target +redefined_loop_name.py:75:5: PLW2901 `for` loop variable `i` overwritten by assignment target | -58 | # For -> annotated assignment -59 | for i in []: -60 | i: int = 5 # error +73 | # For -> annotated assignment +74 | for i in []: +75 | i: int = 5 # error | ^ PLW2901 -61 | -62 | # For -> annotated assignment without value - | - -redefined_loop_name.py:68:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target - | -66 | # Async for -> for, variable reused -67 | async for i in []: -68 | for i in []: # error - | ^ PLW2901 -69 | pass - | - -redefined_loop_name.py:73:15: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target - | -71 | # For -> async for, variable reused -72 | for i in []: -73 | async for i in []: # error - | ^ PLW2901 -74 | pass - | - -redefined_loop_name.py:78:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target - | -76 | # For -> for, outer loop unpacks tuple -77 | for i, j in enumerate([]): -78 | for i in []: # error - | ^ PLW2901 -79 | pass +76 | +77 | # For -> annotated assignment without value | redefined_loop_name.py:83:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -81 | # For -> for, inner loop unpacks tuple -82 | for i in []: -83 | for i, j in enumerate([]): # error +81 | # Async for -> for, variable reused +82 | async for i in []: +83 | for i in []: # error | ^ PLW2901 84 | pass | -redefined_loop_name.py:88:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target +redefined_loop_name.py:88:15: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -86 | # For -> for, both loops unpack tuple -87 | for (i, (j, k)) in []: -88 | for i, j in enumerate([]): # two errors +86 | # For -> async for, variable reused +87 | for i in []: +88 | async for i in []: # error + | ^ PLW2901 +89 | pass + | + +redefined_loop_name.py:93:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target + | +91 | # For -> for, outer loop unpacks tuple +92 | for i, j in enumerate([]): +93 | for i in []: # error | ^ PLW2901 -89 | pass +94 | pass | -redefined_loop_name.py:88:12: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target +redefined_loop_name.py:98:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -86 | # For -> for, both loops unpack tuple -87 | for (i, (j, k)) in []: -88 | for i, j in enumerate([]): # two errors - | ^ PLW2901 -89 | pass +96 | # For -> for, inner loop unpacks tuple +97 | for i in []: +98 | for i, j in enumerate([]): # error + | ^ PLW2901 +99 | pass | -redefined_loop_name.py:105:9: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target +redefined_loop_name.py:103:9: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -103 | # For -> for, outer loop unpacks with asterisk -104 | for i, *j in []: -105 | for j in []: # error +101 | # For -> for, both loops unpack tuple +102 | for (i, (j, k)) in []: +103 | for i, j in enumerate([]): # two errors | ^ PLW2901 -106 | pass +104 | pass | -redefined_loop_name.py:122:13: PLW2901 `for` loop variable `i` overwritten by assignment target +redefined_loop_name.py:103:12: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target | -120 | def f(): -121 | for i in []: # no error -122 | i = 2 # error +101 | # For -> for, both loops unpack tuple +102 | for (i, (j, k)) in []: +103 | for i, j in enumerate([]): # two errors + | ^ PLW2901 +104 | pass + | + +redefined_loop_name.py:120:9: PLW2901 Outer `for` loop variable `j` overwritten by inner `for` loop target + | +118 | # For -> for, outer loop unpacks with asterisk +119 | for i, *j in []: +120 | for j in []: # error + | ^ PLW2901 +121 | pass + | + +redefined_loop_name.py:137:13: PLW2901 `for` loop variable `i` overwritten by assignment target + | +135 | def f(): +136 | for i in []: # no error +137 | i = 2 # error | ^ PLW2901 -123 | -124 | # For -> class definition -> for -> for +138 | +139 | # For -> class definition -> for -> for | -redefined_loop_name.py:128:17: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target +redefined_loop_name.py:143:17: PLW2901 Outer `for` loop variable `i` overwritten by inner `for` loop target | -126 | class A: -127 | for i in []: # no error -128 | for i in []: # error +141 | class A: +142 | for i in []: # no error +143 | for i in []: # error | ^ PLW2901 -129 | pass +144 | pass | -redefined_loop_name.py:143:5: PLW2901 `for` loop variable `a[0]` overwritten by assignment target +redefined_loop_name.py:158:5: PLW2901 `for` loop variable `a[0]` overwritten by assignment target | -141 | # For target with subscript -> assignment -142 | for a[0] in []: -143 | a[0] = 2 # error +156 | # For target with subscript -> assignment +157 | for a[0] in []: +158 | a[0] = 2 # error | ^^^^ PLW2901 -144 | a[1] = 2 # no error +159 | a[1] = 2 # no error | -redefined_loop_name.py:148:5: PLW2901 `for` loop variable `a['i']` overwritten by assignment target +redefined_loop_name.py:163:5: PLW2901 `for` loop variable `a['i']` overwritten by assignment target | -146 | # For target with subscript -> assignment -147 | for a['i'] in []: -148 | a['i'] = 2 # error +161 | # For target with subscript -> assignment +162 | for a['i'] in []: +163 | a['i'] = 2 # error | ^^^^^^ PLW2901 -149 | a['j'] = 2 # no error +164 | a['j'] = 2 # no error | -redefined_loop_name.py:153:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target +redefined_loop_name.py:168:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target | -151 | # For target with attribute -> assignment -152 | for a.i in []: -153 | a.i = 2 # error +166 | # For target with attribute -> assignment +167 | for a.i in []: +168 | a.i = 2 # error | ^^^ PLW2901 -154 | a.j = 2 # no error +169 | a.j = 2 # no error | -redefined_loop_name.py:158:5: PLW2901 `for` loop variable `a.i.j` overwritten by assignment target +redefined_loop_name.py:173:5: PLW2901 `for` loop variable `a.i.j` overwritten by assignment target | -156 | # For target with double nested attribute -> assignment -157 | for a.i.j in []: -158 | a.i.j = 2 # error +171 | # For target with double nested attribute -> assignment +172 | for a.i.j in []: +173 | a.i.j = 2 # error | ^^^^^ PLW2901 -159 | a.j.i = 2 # no error +174 | a.j.i = 2 # no error | -redefined_loop_name.py:163:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target +redefined_loop_name.py:178:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target | -161 | # For target with attribute -> assignment with different spacing -162 | for a.i in []: -163 | a. i = 2 # error +176 | # For target with attribute -> assignment with different spacing +177 | for a.i in []: +178 | a. i = 2 # error | ^^^^ PLW2901 -164 | for a. i in []: -165 | a.i = 2 # error +179 | for a. i in []: +180 | a.i = 2 # error | -redefined_loop_name.py:165:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target +redefined_loop_name.py:180:5: PLW2901 `for` loop variable `a.i` overwritten by assignment target | -163 | a. i = 2 # error -164 | for a. i in []: -165 | a.i = 2 # error +178 | a. i = 2 # error +179 | for a. i in []: +180 | a.i = 2 # error | ^^^ PLW2901 |