diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py index 1a9d76ecf8e77..b1d064b4c0eb5 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py @@ -152,13 +152,16 @@ def __init__(self, ls): else: break -# should not error +# should error for elem in some_list: del some_list[elem] - some_list[elem] = 1 some_list.remove(elem) some_list.discard(elem) +# should not error +for elem in some_list: + some_list[elem] = 1 + # should error for i, elem in enumerate(some_list): some_list.pop(0) @@ -169,4 +172,4 @@ def __init__(self, ls): # should not error (dict) for i, elem in enumerate(some_list): - some_list[elem] = 1 + some_list[elem] = 1 \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs index 210152c3bc4ac..bb8d70a1e3179 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs @@ -5,8 +5,8 @@ use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::name::UnqualifiedName; use ruff_python_ast::{ visitor::{self, Visitor}, - Arguments, Expr, ExprAttribute, ExprCall, ExprSubscript, ExprTuple, Stmt, StmtAssign, - StmtAugAssign, StmtBreak, StmtDelete, StmtFor, StmtIf, + Expr, ExprAttribute, ExprCall, ExprSubscript, ExprTuple, Stmt, StmtAssign, StmtAugAssign, + StmtBreak, StmtDelete, StmtFor, StmtIf, }; use ruff_text_size::TextRange; use std::collections::HashMap; @@ -175,18 +175,13 @@ impl<'a> LoopMutationsVisitor<'a> { if let Expr::Subscript(ExprSubscript { range: _, value, - slice, + slice: _, ctx: _, }) = target { // Find, e.g., `del items[0]`. if ComparableExpr::from(self.iter) == ComparableExpr::from(value) { - // But allow, e.g., `for item in items: del items[item]`. - if ComparableExpr::from(self.index) != ComparableExpr::from(slice) - && ComparableExpr::from(self.target) != ComparableExpr::from(slice) - { - self.add_mutation(range); - } + self.add_mutation(range); } } } @@ -223,7 +218,7 @@ impl<'a> LoopMutationsVisitor<'a> { } /// Handle, e.g., `items.append(1)`. - fn handle_call(&mut self, func: &Expr, arguments: &Arguments) { + fn handle_call(&mut self, func: &Expr) { if let Expr::Attribute(ExprAttribute { range, value, @@ -234,20 +229,6 @@ impl<'a> LoopMutationsVisitor<'a> { if is_mutating_function(attr.as_str()) { // Find, e.g., `items.remove(1)`. if ComparableExpr::from(self.iter) == ComparableExpr::from(value) { - // But allow, e.g., `for item in items: items.remove(item)`. - if matches!(attr.as_str(), "remove" | "discard" | "pop") { - if arguments.len() == 1 { - if let [arg] = &*arguments.args { - if ComparableExpr::from(self.index) == ComparableExpr::from(arg) - || ComparableExpr::from(self.target) - == ComparableExpr::from(arg) - { - return; - } - } - } - } - self.add_mutation(*range); } } @@ -323,11 +304,8 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> { fn visit_expr(&mut self, expr: &'a Expr) { // Ex) `items.append(1)` - if let Expr::Call(ExprCall { - func, arguments, .. - }) = expr - { - self.handle_call(func, arguments); + if let Expr::Call(ExprCall { func, .. }) = expr { + self.handle_call(func); } visitor::walk_expr(self, expr); diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap index a0fadcf86520f..a7993b4b2b263 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap @@ -340,12 +340,41 @@ B909.py:150:8: B909 Mutation to loop iterable `some_list` during iteration 152 | else: | -B909.py:164:5: B909 Mutation to loop iterable `some_list` during iteration +B909.py:157:5: B909 Mutation to loop iterable `some_list` during iteration | -162 | # should error -163 | for i, elem in enumerate(some_list): -164 | some_list.pop(0) +155 | # should error +156 | for elem in some_list: +157 | del some_list[elem] + | ^^^^^^^^^^^^^^^^^^^ B909 +158 | some_list.remove(elem) +159 | some_list.discard(elem) + | + +B909.py:158:5: B909 Mutation to loop iterable `some_list` during iteration + | +156 | for elem in some_list: +157 | del some_list[elem] +158 | some_list.remove(elem) + | ^^^^^^^^^^^^^^^^ B909 +159 | some_list.discard(elem) + | + +B909.py:159:5: B909 Mutation to loop iterable `some_list` during iteration + | +157 | del some_list[elem] +158 | some_list.remove(elem) +159 | some_list.discard(elem) + | ^^^^^^^^^^^^^^^^^ B909 +160 | +161 | # should not error + | + +B909.py:167:5: B909 Mutation to loop iterable `some_list` during iteration + | +165 | # should error +166 | for i, elem in enumerate(some_list): +167 | some_list.pop(0) | ^^^^^^^^^^^^^ B909 -165 | -166 | # should not error (list) +168 | +169 | # should not error (list) |