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 cce53638372d2a..68afaf87fb257b 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py @@ -9,7 +9,7 @@ some_other_list = [1, 2, 3] for elem in some_list: # errors - some_list.remove(elem) + some_list.remove(0) del some_list[2] some_list.append(elem) some_list.sort() @@ -22,7 +22,7 @@ # conditional break should error if elem == 2: - some_list.remove(elem) + some_list.remove(0) if elem == 3: break @@ -81,7 +81,7 @@ def __init__(self, ls): a = A((1, 2, 3)) # ensure member accesses are handled as errors for elem in a.some_list: - a.some_list.remove(elem) + a.some_list.remove(0) del a.some_list[2] @@ -156,3 +156,5 @@ def __init__(self, ls): for elem in some_list: del some_list[elem] some_list[elem] = 1 + some_list.remove(elem) + some_list.discard(elem) 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 f6def259bc0395..77c4bc0cb3463b 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 @@ -7,8 +7,8 @@ use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::name::UnqualifiedName; use ruff_python_ast::{ visitor::{self, Visitor}, - Expr, ExprAttribute, ExprCall, ExprSubscript, Stmt, StmtAssign, StmtAugAssign, StmtBreak, - StmtDelete, StmtFor, StmtIf, + Arguments, Expr, ExprAttribute, ExprCall, ExprSubscript, Stmt, StmtAssign, StmtAugAssign, + StmtBreak, StmtDelete, StmtFor, StmtIf, }; use ruff_text_size::TextRange; @@ -186,7 +186,7 @@ impl<'a> LoopMutationsVisitor<'a> { } /// Handle, e.g., `items.append(1)`. - fn handle_call(&mut self, func: &Expr) { + fn handle_call(&mut self, func: &Expr, arguments: &Arguments) { if let Expr::Attribute(ExprAttribute { range, value, @@ -195,7 +195,17 @@ impl<'a> LoopMutationsVisitor<'a> { }) = func { 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 arguments.len() == 1 && matches!(attr.as_str(), "remove" | "discard") { + if let [arg] = &*arguments.args { + if ComparableExpr::from(self.target) == ComparableExpr::from(arg) { + return; + } + } + } + self.add_mutation(*range); } } @@ -271,8 +281,11 @@ 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, .. }) = expr { - self.handle_call(func); + if let Expr::Call(ExprCall { + func, arguments, .. + }) = expr + { + self.handle_call(func, arguments); } 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 2d03e647fe77ee..7f70841c6c0664 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 @@ -5,7 +5,7 @@ B909.py:12:5: B909 Mutation to loop iterable `some_list` during iteration | 10 | for elem in some_list: 11 | # errors -12 | some_list.remove(elem) +12 | some_list.remove(0) | ^^^^^^^^^^^^^^^^ B909 13 | del some_list[2] 14 | some_list.append(elem) @@ -14,7 +14,7 @@ B909.py:12:5: B909 Mutation to loop iterable `some_list` during iteration B909.py:13:5: B909 Mutation to loop iterable `some_list` during iteration | 11 | # errors -12 | some_list.remove(elem) +12 | some_list.remove(0) 13 | del some_list[2] | ^^^^^^^^^^^^^^^^ B909 14 | some_list.append(elem) @@ -23,7 +23,7 @@ B909.py:13:5: B909 Mutation to loop iterable `some_list` during iteration B909.py:14:5: B909 Mutation to loop iterable `some_list` during iteration | -12 | some_list.remove(elem) +12 | some_list.remove(0) 13 | del some_list[2] 14 | some_list.append(elem) | ^^^^^^^^^^^^^^^^ B909 @@ -104,7 +104,7 @@ B909.py:25:9: B909 Mutation to loop iterable `some_list` during iteration | 23 | # conditional break should error 24 | if elem == 2: -25 | some_list.remove(elem) +25 | some_list.remove(0) | ^^^^^^^^^^^^^^^^ B909 26 | if elem == 3: 27 | break @@ -202,7 +202,7 @@ B909.py:84:5: B909 Mutation to loop iterable `a.some_list` during iteration | 82 | # ensure member accesses are handled as errors 83 | for elem in a.some_list: -84 | a.some_list.remove(elem) +84 | a.some_list.remove(0) | ^^^^^^^^^^^^^^^^^^ B909 85 | del a.some_list[2] | @@ -210,7 +210,7 @@ B909.py:84:5: B909 Mutation to loop iterable `a.some_list` during iteration B909.py:85:5: B909 Mutation to loop iterable `a.some_list` during iteration | 83 | for elem in a.some_list: -84 | a.some_list.remove(elem) +84 | a.some_list.remove(0) 85 | del a.some_list[2] | ^^^^^^^^^^^^^^^^^^ B909 |