Remove discard, remove, and pop allowance for loop-iterator-mutation

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

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);

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)
     |