From 70fb21868d317eee2cb0cf8b6cc7032f36d369b5 Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Fri, 19 Jan 2024 14:52:26 +0100
Subject: [PATCH 01/16] feat(rules): Implement flake8-bugbear B038 rule

The B038 rule checks for mutation of loop iterators in the body
of a for loop and alerts when found.
Editing the loop iterator can lead to undesired behavior and is probably
a bug in most cases.
---
 .../test/fixtures/flake8_bugbear/B038.py      |  75 +++++++
 .../ast/analyze/deferred_for_loops.rs         |   3 +
 .../src/checkers/ast/analyze/statement.rs     |   1 +
 crates/ruff_linter/src/codes.rs               |   1 +
 .../src/rules/flake8_bugbear/mod.rs           |   1 +
 .../rules/loop_iterator_mutated.rs            | 185 ++++++++++++++++++
 .../src/rules/flake8_bugbear/rules/mod.rs     |   2 +
 ...__flake8_bugbear__tests__B038_B038.py.snap | 137 +++++++++++++
 ruff.schema.json                              |   1 +
 9 files changed, 406 insertions(+)
 create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
 create mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs
 create mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
new file mode 100644
index 0000000000000..6e116034fe801
--- /dev/null
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
@@ -0,0 +1,75 @@
+"""
+Should emit:
+B999 - on lines 11, 25, 26, 40, 46
+"""
+
+
+some_list = [1, 2, 3]
+for elem in some_list:
+    print(elem)
+    if elem % 2 == 0:
+        some_list.remove(elem)  # should error
+
+some_list = [1, 2, 3]
+some_other_list = [1, 2, 3]
+for elem in some_list:
+    print(elem)
+    if elem % 2 == 0:
+        some_other_list.remove(elem)  # should not error
+        del some_other_list
+
+
+some_list = [1, 2, 3]
+for elem in some_list:
+    print(elem)
+    if elem % 2 == 0:
+        del some_list[2]  # should error
+        del some_list
+
+
+class A:
+    some_list: list
+
+    def __init__(self, ls):
+        self.some_list = list(ls)
+
+
+a = A((1, 2, 3))
+for elem in a.some_list:
+    print(elem)
+    if elem % 2 == 0:
+        a.some_list.remove(elem)  # should error
+
+a = A((1, 2, 3))
+for elem in a.some_list:
+    print(elem)
+    if elem % 2 == 0:
+        del a.some_list[2]  # should error
+
+
+
+some_list = [1, 2, 3]
+for elem in some_list:
+    print(elem)
+    if elem == 2:
+        found_idx = some_list.index(elem)  # should not error
+        some_list.append(elem)  # should error
+        some_list.sort()  # should error
+        some_list.reverse()  # should error
+        some_list.clear()  # should error
+        some_list.extend([1,2])  # should error
+        some_list.insert(1, 1)  # should error
+        some_list.pop(1) # should error
+        some_list.pop() # should error
+        some_list = 3 # should error
+        break
+
+
+
+mydicts = {'a': {'foo': 1, 'bar': 2}}
+
+for mydict in mydicts:
+    if mydicts.get('a', ''):
+        print(mydict['foo'])  # should not error
+        mydicts.popitem() # should error
+        
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
index ee2378dd8bd0e..90acb6d6a300a 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
@@ -30,6 +30,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
             if checker.enabled(Rule::EnumerateForLoop) {
                 flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
             }
+            if checker.enabled(Rule::LoopIteratorMutated) {
+                flake8_bugbear::rules::loop_iterator_mutated(checker, stmt_for);
+            }
         }
     }
 }
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
index c31f147997663..9e21d527bbfdd 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -1266,6 +1266,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::UnnecessaryEnumerate,
                 Rule::UnusedLoopControlVariable,
                 Rule::YieldInForLoop,
+                Rule::LoopIteratorMutated,
             ]) {
                 checker.analyze.for_loops.push(checker.semantic.snapshot());
             }
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index d073f0223f089..7df72c78633d1 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -348,6 +348,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "032") => (RuleGroup::Stable, rules::flake8_bugbear::rules::UnintentionalTypeAnnotation),
         (Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
         (Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
+        (Flake8Bugbear, "038") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutated),
         (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
 
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
index 8cde263044c04..29aa39b46acf5 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
@@ -60,6 +60,7 @@ mod tests {
     #[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.py"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.ipynb"))]
+    #[test_case(Rule::LoopIteratorMutated, Path::new("B038.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs
new file mode 100644
index 0000000000000..fa9637562b478
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs
@@ -0,0 +1,185 @@
+use ruff_diagnostics::Violation;
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{
+    visitor::{self, Visitor},
+    Expr, ExprAttribute, ExprCall, ExprContext, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
+};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+use ruff_diagnostics::Diagnostic;
+
+static MUTATING_FUNCTIONS: &'static [&'static str] = &[
+    "append", "sort", "reverse", "remove", "clear", "extend", "insert", "pop", "popitem",
+];
+
+/// ## What it does
+/// Checks for mutation of the iterator of a loop in the loop's body
+///
+/// ## Why is this bad?
+/// Changing the structure that is being iterated over will usually lead to
+/// unintended behavior as not all elements will be addressed.
+///
+/// ## Example
+/// ```python
+/// some_list = [1,2,3]
+/// for i in some_list:
+///   some_list.remove(i) # this will lead to not all elements being printed
+///   print(i)
+/// ```
+///
+///
+/// ## References
+/// - [Python documentation: Mutable Sequence Types](https://docs.python.org/3/library/stdtypes.html#typesseq-mutable)
+#[violation]
+pub struct LoopIteratorMutated;
+
+impl Violation for LoopIteratorMutated {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("editing a loop's mutable iterable often leads to unexpected results/bugs")
+    }
+}
+
+fn _to_name_str(node: &Expr) -> String {
+    match node {
+        Expr::Name(ExprName { id, .. }) => {
+            return id.to_string();
+        }
+        Expr::Attribute(ExprAttribute {
+            range: _,
+            value,
+            attr,
+            ..
+        }) => {
+            let mut inner = _to_name_str(value);
+            match inner.as_str() {
+                "" => {
+                    return "".into();
+                }
+                _ => {
+                    inner.push_str(".");
+                    inner.push_str(attr);
+                    return inner;
+                }
+            }
+        }
+        Expr::Call(ExprCall { range: _, func, .. }) => {
+            return _to_name_str(func);
+        }
+        _ => {
+            return "".into();
+        }
+    }
+}
+// B038
+pub(crate) fn loop_iterator_mutated(checker: &mut Checker, stmt_for: &StmtFor) {
+    let StmtFor {
+        target: _,
+        iter,
+        body,
+        orelse: _,
+        is_async: _,
+        range: _,
+    } = stmt_for;
+    let name;
+
+    match iter.as_ref() {
+        Expr::Name(ExprName { .. }) => {
+            name = _to_name_str(iter.as_ref());
+        }
+        Expr::Attribute(ExprAttribute { .. }) => {
+            name = _to_name_str(iter.as_ref());
+        }
+        _ => {
+            println!("Shouldn't happen");
+            return;
+        }
+    }
+    let mut visitor = LoopMutationsVisitor {
+        name: &name,
+        mutations: Vec::new(),
+    };
+    visitor.visit_body(body);
+    for mutation in visitor.mutations {
+        checker
+            .diagnostics
+            .push(Diagnostic::new(LoopIteratorMutated {}, mutation.range()));
+    }
+}
+struct LoopMutationsVisitor<'a> {
+    name: &'a String,
+    mutations: Vec<Box<dyn Ranged + 'a>>,
+}
+
+/// `Visitor` to collect all used identifiers in a statement.
+impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
+    fn visit_stmt(&mut self, stmt: &'a Stmt) {
+        match stmt {
+            Stmt::Delete(StmtDelete { range, targets }) => {
+                for target in targets {
+                    let name;
+                    match target {
+                        Expr::Subscript(ExprSubscript {
+                            range: _,
+                            value,
+                            slice: _,
+                            ctx: _,
+                        }) => {
+                            name = _to_name_str(value);
+                        }
+
+                        Expr::Attribute(_) | Expr::Name(_) => {
+                            name = _to_name_str(target);
+                        }
+                        _ => {
+                            name = String::new();
+                            visitor::walk_expr(self, target);
+                        }
+                    }
+                    if self.name.eq(&name) {
+                        self.mutations.push(Box::new(range));
+                    }
+                }
+            }
+            _ => {
+                visitor::walk_stmt(self, stmt);
+            }
+        }
+    }
+
+    fn visit_expr(&mut self, expr: &'a Expr) {
+        match expr {
+            Expr::Name(ExprName { range: _, id, ctx }) => {
+                if self.name.eq(id) {
+                    match ctx {
+                        ExprContext::Del => {
+                            self.mutations.push(Box::new(expr));
+                        }
+                        _ => {}
+                    }
+                }
+            }
+            Expr::Call(ExprCall {
+                range: _,
+                func,
+                arguments: _,
+            }) => match func.as_ref() {
+                Expr::Attribute(ExprAttribute {
+                    range: _,
+                    value,
+                    attr,
+                    ctx: _,
+                }) => {
+                    let name = _to_name_str(value);
+                    if self.name.eq(&name) && MUTATING_FUNCTIONS.contains(&attr.as_str()) {
+                        self.mutations.push(Box::new(expr));
+                    }
+                }
+                _ => {}
+            },
+            _ => {}
+        }
+        visitor::walk_expr(self, expr);
+    }
+}
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
index 3f8a1fdaf646f..4ddfce6d95cb4 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
@@ -12,6 +12,7 @@ pub(crate) use function_call_in_argument_default::*;
 pub(crate) use function_uses_loop_variable::*;
 pub(crate) use getattr_with_constant::*;
 pub(crate) use jump_statement_in_finally::*;
+pub(crate) use loop_iterator_mutated::*;
 pub(crate) use loop_variable_overrides_iterator::*;
 pub(crate) use mutable_argument_default::*;
 pub(crate) use no_explicit_stacklevel::*;
@@ -46,6 +47,7 @@ mod function_call_in_argument_default;
 mod function_uses_loop_variable;
 mod getattr_with_constant;
 mod jump_statement_in_finally;
+mod loop_iterator_mutated;
 mod loop_variable_overrides_iterator;
 mod mutable_argument_default;
 mod no_explicit_stacklevel;
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
new file mode 100644
index 0000000000000..043c533b3a6ea
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
@@ -0,0 +1,137 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+---
+B038.py:11:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+ 9 |     print(elem)
+10 |     if elem % 2 == 0:
+11 |         some_list.remove(elem)  # should error
+   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+12 | 
+13 | some_list = [1, 2, 3]
+   |
+
+B038.py:26:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+24 |     print(elem)
+25 |     if elem % 2 == 0:
+26 |         del some_list[2]  # should error
+   |         ^^^^^^^^^^^^^^^^ B038
+27 |         del some_list
+   |
+
+B038.py:27:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+25 |     if elem % 2 == 0:
+26 |         del some_list[2]  # should error
+27 |         del some_list
+   |         ^^^^^^^^^^^^^ B038
+   |
+
+B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+39 |     print(elem)
+40 |     if elem % 2 == 0:
+41 |         a.some_list.remove(elem)  # should error
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ B038
+42 | 
+43 | a = A((1, 2, 3))
+   |
+
+B038.py:47:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+45 |     print(elem)
+46 |     if elem % 2 == 0:
+47 |         del a.some_list[2]  # should error
+   |         ^^^^^^^^^^^^^^^^^^ B038
+   |
+
+B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+54 |     if elem == 2:
+55 |         found_idx = some_list.index(elem)  # should not error
+56 |         some_list.append(elem)  # should error
+   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+57 |         some_list.sort()  # should error
+58 |         some_list.reverse()  # should error
+   |
+
+B038.py:57:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+55 |         found_idx = some_list.index(elem)  # should not error
+56 |         some_list.append(elem)  # should error
+57 |         some_list.sort()  # should error
+   |         ^^^^^^^^^^^^^^^^ B038
+58 |         some_list.reverse()  # should error
+59 |         some_list.clear()  # should error
+   |
+
+B038.py:58:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+56 |         some_list.append(elem)  # should error
+57 |         some_list.sort()  # should error
+58 |         some_list.reverse()  # should error
+   |         ^^^^^^^^^^^^^^^^^^^ B038
+59 |         some_list.clear()  # should error
+60 |         some_list.extend([1,2])  # should error
+   |
+
+B038.py:59:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+57 |         some_list.sort()  # should error
+58 |         some_list.reverse()  # should error
+59 |         some_list.clear()  # should error
+   |         ^^^^^^^^^^^^^^^^^ B038
+60 |         some_list.extend([1,2])  # should error
+61 |         some_list.insert(1, 1)  # should error
+   |
+
+B038.py:60:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+58 |         some_list.reverse()  # should error
+59 |         some_list.clear()  # should error
+60 |         some_list.extend([1,2])  # should error
+   |         ^^^^^^^^^^^^^^^^^^^^^^^ B038
+61 |         some_list.insert(1, 1)  # should error
+62 |         some_list.pop(1) # should error
+   |
+
+B038.py:61:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+59 |         some_list.clear()  # should error
+60 |         some_list.extend([1,2])  # should error
+61 |         some_list.insert(1, 1)  # should error
+   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+62 |         some_list.pop(1) # should error
+63 |         some_list.pop() # should error
+   |
+
+B038.py:62:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+60 |         some_list.extend([1,2])  # should error
+61 |         some_list.insert(1, 1)  # should error
+62 |         some_list.pop(1) # should error
+   |         ^^^^^^^^^^^^^^^^ B038
+63 |         some_list.pop() # should error
+64 |         some_list = 3 # should error
+   |
+
+B038.py:63:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+61 |         some_list.insert(1, 1)  # should error
+62 |         some_list.pop(1) # should error
+63 |         some_list.pop() # should error
+   |         ^^^^^^^^^^^^^^^ B038
+64 |         some_list = 3 # should error
+65 |         break
+   |
+
+B038.py:74:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+72 |     if mydicts.get('a', ''):
+73 |         print(mydict['foo'])  # should not error
+74 |         mydicts.popitem() # should error
+   |         ^^^^^^^^^^^^^^^^^ B038
+   |
+
+
diff --git a/ruff.schema.json b/ruff.schema.json
index ef61102357472..c74ff4d4835c4 100644
--- a/ruff.schema.json
+++ b/ruff.schema.json
@@ -2600,6 +2600,7 @@
         "B032",
         "B033",
         "B034",
+        "B038",
         "B9",
         "B90",
         "B904",

From ba1cc7a741290499f1a26a8ba2bbac2423fba02b Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 25 Jan 2024 13:06:42 +0100
Subject: [PATCH 02/16] refactor(flake8-bugbear): Rename final based on PR
 comments

---
 .../src/checkers/ast/analyze/deferred_for_loops.rs        | 4 ++--
 crates/ruff_linter/src/checkers/ast/analyze/statement.rs  | 2 +-
 crates/ruff_linter/src/codes.rs                           | 2 +-
 crates/ruff_linter/src/rules/flake8_bugbear/mod.rs        | 2 +-
 ...loop_iterator_mutated.rs => loop_iterator_mutation.rs} | 8 ++++----
 crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs  | 4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)
 rename crates/ruff_linter/src/rules/flake8_bugbear/rules/{loop_iterator_mutated.rs => loop_iterator_mutation.rs} (95%)

diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
index 90acb6d6a300a..0aad5987dc1ce 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs
@@ -30,8 +30,8 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
             if checker.enabled(Rule::EnumerateForLoop) {
                 flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
             }
-            if checker.enabled(Rule::LoopIteratorMutated) {
-                flake8_bugbear::rules::loop_iterator_mutated(checker, stmt_for);
+            if checker.enabled(Rule::LoopIteratorMutation) {
+                flake8_bugbear::rules::loop_iterator_mutation(checker, stmt_for);
             }
         }
     }
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
index 9e21d527bbfdd..eec7b641e85d1 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -1266,7 +1266,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::UnnecessaryEnumerate,
                 Rule::UnusedLoopControlVariable,
                 Rule::YieldInForLoop,
-                Rule::LoopIteratorMutated,
+                Rule::LoopIteratorMutation,
             ]) {
                 checker.analyze.for_loops.push(checker.semantic.snapshot());
             }
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index 7df72c78633d1..5118f73563727 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -348,7 +348,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "032") => (RuleGroup::Stable, rules::flake8_bugbear::rules::UnintentionalTypeAnnotation),
         (Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
         (Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
-        (Flake8Bugbear, "038") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutated),
+        (Flake8Bugbear, "038") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
         (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
 
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
index 29aa39b46acf5..56223505a1707 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
@@ -60,7 +60,7 @@ mod tests {
     #[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.py"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.ipynb"))]
-    #[test_case(Rule::LoopIteratorMutated, Path::new("B038.py"))]
+    #[test_case(Rule::LoopIteratorMutation, Path::new("B038.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs
similarity index 95%
rename from crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs
rename to crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs
index fa9637562b478..509b90305bcf8 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutated.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs
@@ -32,9 +32,9 @@ static MUTATING_FUNCTIONS: &'static [&'static str] = &[
 /// ## References
 /// - [Python documentation: Mutable Sequence Types](https://docs.python.org/3/library/stdtypes.html#typesseq-mutable)
 #[violation]
-pub struct LoopIteratorMutated;
+pub struct LoopIteratorMutation;
 
-impl Violation for LoopIteratorMutated {
+impl Violation for LoopIteratorMutation {
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("editing a loop's mutable iterable often leads to unexpected results/bugs")
@@ -73,7 +73,7 @@ fn _to_name_str(node: &Expr) -> String {
     }
 }
 // B038
-pub(crate) fn loop_iterator_mutated(checker: &mut Checker, stmt_for: &StmtFor) {
+pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor) {
     let StmtFor {
         target: _,
         iter,
@@ -104,7 +104,7 @@ pub(crate) fn loop_iterator_mutated(checker: &mut Checker, stmt_for: &StmtFor) {
     for mutation in visitor.mutations {
         checker
             .diagnostics
-            .push(Diagnostic::new(LoopIteratorMutated {}, mutation.range()));
+            .push(Diagnostic::new(LoopIteratorMutation {}, mutation.range()));
     }
 }
 struct LoopMutationsVisitor<'a> {
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
index 4ddfce6d95cb4..5e5e6067b1bba 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
@@ -12,7 +12,7 @@ pub(crate) use function_call_in_argument_default::*;
 pub(crate) use function_uses_loop_variable::*;
 pub(crate) use getattr_with_constant::*;
 pub(crate) use jump_statement_in_finally::*;
-pub(crate) use loop_iterator_mutated::*;
+pub(crate) use loop_iterator_mutation::*;
 pub(crate) use loop_variable_overrides_iterator::*;
 pub(crate) use mutable_argument_default::*;
 pub(crate) use no_explicit_stacklevel::*;
@@ -47,7 +47,7 @@ mod function_call_in_argument_default;
 mod function_uses_loop_variable;
 mod getattr_with_constant;
 mod jump_statement_in_finally;
-mod loop_iterator_mutated;
+mod loop_iterator_mutation;
 mod loop_variable_overrides_iterator;
 mod mutable_argument_default;
 mod no_explicit_stacklevel;

From d32ae7798f499d2fc5cab5f08689e1dda4995794 Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 25 Jan 2024 13:55:17 +0100
Subject: [PATCH 03/16] fix: Fix issues based on PR comments

---
 .../rules/loop_iterator_mutation.rs           | 46 +++++++++----------
 ...__flake8_bugbear__tests__B038_B038.py.snap | 22 ++++-----
 2 files changed, 34 insertions(+), 34 deletions(-)

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 509b90305bcf8..5e506a4563c71 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
@@ -2,17 +2,27 @@ use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{
     visitor::{self, Visitor},
-    Expr, ExprAttribute, ExprCall, ExprContext, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
+    Expr, ExprAttribute, ExprCall, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
 };
-use ruff_text_size::Ranged;
+use ruff_text_size::TextRange;
 
 use crate::checkers::ast::Checker;
 use ruff_diagnostics::Diagnostic;
 
-static MUTATING_FUNCTIONS: &'static [&'static str] = &[
-    "append", "sort", "reverse", "remove", "clear", "extend", "insert", "pop", "popitem",
-];
-
+fn is_mutating_function(function_name: &str) -> bool {
+    matches!(
+        function_name,
+        "append"
+            | "sort"
+            | "reverse"
+            | "remove"
+            | "clear"
+            | "extend"
+            | "insert"
+            | "pop"
+            | "popitem"
+    )
+}
 /// ## What it does
 /// Checks for mutation of the iterator of a loop in the loop's body
 ///
@@ -104,12 +114,12 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
     for mutation in visitor.mutations {
         checker
             .diagnostics
-            .push(Diagnostic::new(LoopIteratorMutation {}, mutation.range()));
+            .push(Diagnostic::new(LoopIteratorMutation, mutation));
     }
 }
 struct LoopMutationsVisitor<'a> {
-    name: &'a String,
-    mutations: Vec<Box<dyn Ranged + 'a>>,
+    name: &'a str,
+    mutations: Vec<TextRange>,
 }
 
 /// `Visitor` to collect all used identifiers in a statement.
@@ -138,7 +148,7 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
                         }
                     }
                     if self.name.eq(&name) {
-                        self.mutations.push(Box::new(range));
+                        self.mutations.push(*range);
                     }
                 }
             }
@@ -150,30 +160,20 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
 
     fn visit_expr(&mut self, expr: &'a Expr) {
         match expr {
-            Expr::Name(ExprName { range: _, id, ctx }) => {
-                if self.name.eq(id) {
-                    match ctx {
-                        ExprContext::Del => {
-                            self.mutations.push(Box::new(expr));
-                        }
-                        _ => {}
-                    }
-                }
-            }
             Expr::Call(ExprCall {
                 range: _,
                 func,
                 arguments: _,
             }) => match func.as_ref() {
                 Expr::Attribute(ExprAttribute {
-                    range: _,
+                    range,
                     value,
                     attr,
                     ctx: _,
                 }) => {
                     let name = _to_name_str(value);
-                    if self.name.eq(&name) && MUTATING_FUNCTIONS.contains(&attr.as_str()) {
-                        self.mutations.push(Box::new(expr));
+                    if self.name.eq(&name) && is_mutating_function(&attr.as_str()) {
+                        self.mutations.push(*range);
                     }
                 }
                 _ => {}
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
index 043c533b3a6ea..4dd4a3105d735 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
@@ -6,7 +6,7 @@ B038.py:11:9: B038 editing a loop's mutable iterable often leads to unexpected r
  9 |     print(elem)
 10 |     if elem % 2 == 0:
 11 |         some_list.remove(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^ B038
 12 | 
 13 | some_list = [1, 2, 3]
    |
@@ -33,7 +33,7 @@ B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected r
 39 |     print(elem)
 40 |     if elem % 2 == 0:
 41 |         a.some_list.remove(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^^^ B038
 42 | 
 43 | a = A((1, 2, 3))
    |
@@ -51,7 +51,7 @@ B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected r
 54 |     if elem == 2:
 55 |         found_idx = some_list.index(elem)  # should not error
 56 |         some_list.append(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^ B038
 57 |         some_list.sort()  # should error
 58 |         some_list.reverse()  # should error
    |
@@ -61,7 +61,7 @@ B038.py:57:9: B038 editing a loop's mutable iterable often leads to unexpected r
 55 |         found_idx = some_list.index(elem)  # should not error
 56 |         some_list.append(elem)  # should error
 57 |         some_list.sort()  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^ B038
 58 |         some_list.reverse()  # should error
 59 |         some_list.clear()  # should error
    |
@@ -71,7 +71,7 @@ B038.py:58:9: B038 editing a loop's mutable iterable often leads to unexpected r
 56 |         some_list.append(elem)  # should error
 57 |         some_list.sort()  # should error
 58 |         some_list.reverse()  # should error
-   |         ^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^^ B038
 59 |         some_list.clear()  # should error
 60 |         some_list.extend([1,2])  # should error
    |
@@ -81,7 +81,7 @@ B038.py:59:9: B038 editing a loop's mutable iterable often leads to unexpected r
 57 |         some_list.sort()  # should error
 58 |         some_list.reverse()  # should error
 59 |         some_list.clear()  # should error
-   |         ^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^ B038
 60 |         some_list.extend([1,2])  # should error
 61 |         some_list.insert(1, 1)  # should error
    |
@@ -91,7 +91,7 @@ B038.py:60:9: B038 editing a loop's mutable iterable often leads to unexpected r
 58 |         some_list.reverse()  # should error
 59 |         some_list.clear()  # should error
 60 |         some_list.extend([1,2])  # should error
-   |         ^^^^^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^ B038
 61 |         some_list.insert(1, 1)  # should error
 62 |         some_list.pop(1) # should error
    |
@@ -101,7 +101,7 @@ B038.py:61:9: B038 editing a loop's mutable iterable often leads to unexpected r
 59 |         some_list.clear()  # should error
 60 |         some_list.extend([1,2])  # should error
 61 |         some_list.insert(1, 1)  # should error
-   |         ^^^^^^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^^ B038
 62 |         some_list.pop(1) # should error
 63 |         some_list.pop() # should error
    |
@@ -111,7 +111,7 @@ B038.py:62:9: B038 editing a loop's mutable iterable often leads to unexpected r
 60 |         some_list.extend([1,2])  # should error
 61 |         some_list.insert(1, 1)  # should error
 62 |         some_list.pop(1) # should error
-   |         ^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^ B038
 63 |         some_list.pop() # should error
 64 |         some_list = 3 # should error
    |
@@ -121,7 +121,7 @@ B038.py:63:9: B038 editing a loop's mutable iterable often leads to unexpected r
 61 |         some_list.insert(1, 1)  # should error
 62 |         some_list.pop(1) # should error
 63 |         some_list.pop() # should error
-   |         ^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^ B038
 64 |         some_list = 3 # should error
 65 |         break
    |
@@ -131,7 +131,7 @@ B038.py:74:9: B038 editing a loop's mutable iterable often leads to unexpected r
 72 |     if mydicts.get('a', ''):
 73 |         print(mydict['foo'])  # should not error
 74 |         mydicts.popitem() # should error
-   |         ^^^^^^^^^^^^^^^^^ B038
+   |         ^^^^^^^^^^^^^^^ B038
    |
 
 

From fe9e35d81cfdb097d1c82a0a74bad8c4d4dc0b5b Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 25 Jan 2024 11:49:20 -0500
Subject: [PATCH 04/16] Format fixture

---
 .../test/fixtures/flake8_bugbear/B038.py      |  25 +--
 ...__flake8_bugbear__tests__B038_B038.py.snap | 160 +++++++++---------
 2 files changed, 88 insertions(+), 97 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
index 6e116034fe801..093ad7cd1fb41 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
@@ -1,9 +1,3 @@
-"""
-Should emit:
-B999 - on lines 11, 25, 26, 40, 46
-"""
-
-
 some_list = [1, 2, 3]
 for elem in some_list:
     print(elem)
@@ -47,7 +41,6 @@ def __init__(self, ls):
         del a.some_list[2]  # should error
 
 
-
 some_list = [1, 2, 3]
 for elem in some_list:
     print(elem)
@@ -57,19 +50,17 @@ def __init__(self, ls):
         some_list.sort()  # should error
         some_list.reverse()  # should error
         some_list.clear()  # should error
-        some_list.extend([1,2])  # should error
+        some_list.extend([1, 2])  # should error
         some_list.insert(1, 1)  # should error
-        some_list.pop(1) # should error
-        some_list.pop() # should error
-        some_list = 3 # should error
+        some_list.pop(1)  # should error
+        some_list.pop()  # should error
+        some_list = 3  # should error
         break
 
 
-
-mydicts = {'a': {'foo': 1, 'bar': 2}}
+mydicts = {"a": {"foo": 1, "bar": 2}}
 
 for mydict in mydicts:
-    if mydicts.get('a', ''):
-        print(mydict['foo'])  # should not error
-        mydicts.popitem() # should error
-        
+    if mydicts.get("a", ""):
+        print(mydict["foo"])  # should not error
+        mydicts.popitem()  # should error
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
index 4dd4a3105d735..d6a5f341c7d42 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
@@ -1,136 +1,136 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
 ---
-B038.py:11:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
- 9 |     print(elem)
-10 |     if elem % 2 == 0:
-11 |         some_list.remove(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
-12 | 
-13 | some_list = [1, 2, 3]
-   |
+B038.py:5:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+  |
+3 |     print(elem)
+4 |     if elem % 2 == 0:
+5 |         some_list.remove(elem)  # should error
+  |         ^^^^^^^^^^^^^^^^ B038
+6 | 
+7 | some_list = [1, 2, 3]
+  |
 
-B038.py:26:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:20:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-24 |     print(elem)
-25 |     if elem % 2 == 0:
-26 |         del some_list[2]  # should error
+18 |     print(elem)
+19 |     if elem % 2 == 0:
+20 |         del some_list[2]  # should error
    |         ^^^^^^^^^^^^^^^^ B038
-27 |         del some_list
+21 |         del some_list
    |
 
-B038.py:27:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:21:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-25 |     if elem % 2 == 0:
-26 |         del some_list[2]  # should error
-27 |         del some_list
+19 |     if elem % 2 == 0:
+20 |         del some_list[2]  # should error
+21 |         del some_list
    |         ^^^^^^^^^^^^^ B038
    |
 
-B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:35:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-39 |     print(elem)
-40 |     if elem % 2 == 0:
-41 |         a.some_list.remove(elem)  # should error
+33 |     print(elem)
+34 |     if elem % 2 == 0:
+35 |         a.some_list.remove(elem)  # should error
    |         ^^^^^^^^^^^^^^^^^^ B038
-42 | 
-43 | a = A((1, 2, 3))
+36 | 
+37 | a = A((1, 2, 3))
    |
 
-B038.py:47:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-45 |     print(elem)
-46 |     if elem % 2 == 0:
-47 |         del a.some_list[2]  # should error
+39 |     print(elem)
+40 |     if elem % 2 == 0:
+41 |         del a.some_list[2]  # should error
    |         ^^^^^^^^^^^^^^^^^^ B038
    |
 
-B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:49:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-54 |     if elem == 2:
-55 |         found_idx = some_list.index(elem)  # should not error
-56 |         some_list.append(elem)  # should error
+47 |     if elem == 2:
+48 |         found_idx = some_list.index(elem)  # should not error
+49 |         some_list.append(elem)  # should error
    |         ^^^^^^^^^^^^^^^^ B038
-57 |         some_list.sort()  # should error
-58 |         some_list.reverse()  # should error
+50 |         some_list.sort()  # should error
+51 |         some_list.reverse()  # should error
    |
 
-B038.py:57:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:50:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-55 |         found_idx = some_list.index(elem)  # should not error
-56 |         some_list.append(elem)  # should error
-57 |         some_list.sort()  # should error
+48 |         found_idx = some_list.index(elem)  # should not error
+49 |         some_list.append(elem)  # should error
+50 |         some_list.sort()  # should error
    |         ^^^^^^^^^^^^^^ B038
-58 |         some_list.reverse()  # should error
-59 |         some_list.clear()  # should error
+51 |         some_list.reverse()  # should error
+52 |         some_list.clear()  # should error
    |
 
-B038.py:58:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:51:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-56 |         some_list.append(elem)  # should error
-57 |         some_list.sort()  # should error
-58 |         some_list.reverse()  # should error
+49 |         some_list.append(elem)  # should error
+50 |         some_list.sort()  # should error
+51 |         some_list.reverse()  # should error
    |         ^^^^^^^^^^^^^^^^^ B038
-59 |         some_list.clear()  # should error
-60 |         some_list.extend([1,2])  # should error
+52 |         some_list.clear()  # should error
+53 |         some_list.extend([1, 2])  # should error
    |
 
-B038.py:59:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:52:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-57 |         some_list.sort()  # should error
-58 |         some_list.reverse()  # should error
-59 |         some_list.clear()  # should error
+50 |         some_list.sort()  # should error
+51 |         some_list.reverse()  # should error
+52 |         some_list.clear()  # should error
    |         ^^^^^^^^^^^^^^^ B038
-60 |         some_list.extend([1,2])  # should error
-61 |         some_list.insert(1, 1)  # should error
+53 |         some_list.extend([1, 2])  # should error
+54 |         some_list.insert(1, 1)  # should error
    |
 
-B038.py:60:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:53:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-58 |         some_list.reverse()  # should error
-59 |         some_list.clear()  # should error
-60 |         some_list.extend([1,2])  # should error
+51 |         some_list.reverse()  # should error
+52 |         some_list.clear()  # should error
+53 |         some_list.extend([1, 2])  # should error
    |         ^^^^^^^^^^^^^^^^ B038
-61 |         some_list.insert(1, 1)  # should error
-62 |         some_list.pop(1) # should error
+54 |         some_list.insert(1, 1)  # should error
+55 |         some_list.pop(1)  # should error
    |
 
-B038.py:61:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:54:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-59 |         some_list.clear()  # should error
-60 |         some_list.extend([1,2])  # should error
-61 |         some_list.insert(1, 1)  # should error
+52 |         some_list.clear()  # should error
+53 |         some_list.extend([1, 2])  # should error
+54 |         some_list.insert(1, 1)  # should error
    |         ^^^^^^^^^^^^^^^^ B038
-62 |         some_list.pop(1) # should error
-63 |         some_list.pop() # should error
+55 |         some_list.pop(1)  # should error
+56 |         some_list.pop()  # should error
    |
 
-B038.py:62:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:55:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-60 |         some_list.extend([1,2])  # should error
-61 |         some_list.insert(1, 1)  # should error
-62 |         some_list.pop(1) # should error
+53 |         some_list.extend([1, 2])  # should error
+54 |         some_list.insert(1, 1)  # should error
+55 |         some_list.pop(1)  # should error
    |         ^^^^^^^^^^^^^ B038
-63 |         some_list.pop() # should error
-64 |         some_list = 3 # should error
+56 |         some_list.pop()  # should error
+57 |         some_list = 3  # should error
    |
 
-B038.py:63:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-61 |         some_list.insert(1, 1)  # should error
-62 |         some_list.pop(1) # should error
-63 |         some_list.pop() # should error
+54 |         some_list.insert(1, 1)  # should error
+55 |         some_list.pop(1)  # should error
+56 |         some_list.pop()  # should error
    |         ^^^^^^^^^^^^^ B038
-64 |         some_list = 3 # should error
-65 |         break
+57 |         some_list = 3  # should error
+58 |         break
    |
 
-B038.py:74:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
+B038.py:66:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
-72 |     if mydicts.get('a', ''):
-73 |         print(mydict['foo'])  # should not error
-74 |         mydicts.popitem() # should error
+64 |     if mydicts.get("a", ""):
+65 |         print(mydict["foo"])  # should not error
+66 |         mydicts.popitem()  # should error
    |         ^^^^^^^^^^^^^^^ B038
    |
 

From 51d1c91f9d156ea984c3d5a2ed28c1cbd7753637 Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 11 Apr 2024 13:55:07 +0200
Subject: [PATCH 05/16] refactor(flake8-bugbear): Rename rule B909 to B909 for
 parity

---
 .../test/fixtures/flake8_bugbear/B909.py      | 129 ++++++++++++++++++
 crates/ruff_linter/src/codes.rs               |   2 +-
 .../src/rules/flake8_bugbear/mod.rs           |   2 +-
 .../rules/loop_iterator_mutation.rs           |   2 +-
 4 files changed, 132 insertions(+), 3 deletions(-)
 create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
new file mode 100644
index 0000000000000..00ee343d86b13
--- /dev/null
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -0,0 +1,129 @@
+"""
+Should emit:
+B999 - on lines 11, 25, 26, 40, 46
+"""
+
+# lists
+
+some_list = [1, 2, 3]
+some_other_list = [1, 2, 3]
+for elem in some_list:
+    # errors
+    some_list.remove(elem)
+    del some_list[2]
+    some_list.append(elem)
+    some_list.sort()
+    some_list.reverse()
+    some_list.clear()
+    some_list.extend([1, 2])
+    some_list.insert(1, 1)
+    some_list.pop(1)
+    some_list.pop()
+
+    # conditional break should error
+    if elem == 2:
+        some_list.remove(elem)
+        if elem == 3:
+            break
+
+    # non-errors
+    some_other_list.remove(elem)
+    del some_list
+    del some_other_list
+    found_idx = some_list.index(elem)
+    some_list = 3
+
+    # unconditional break should not error
+    if elem == 2:
+        some_list.remove(elem)
+        break
+
+
+# dicts
+mydicts = {'a': {'foo': 1, 'bar': 2}}
+
+for elem in mydicts:
+    # errors
+    mydicts.popitem()
+    mydicts.setdefault('foo', 1)
+    mydicts.update({'foo': 'bar'})
+
+    # no errors
+    elem.popitem()
+    elem.setdefault('foo', 1)
+    elem.update({'foo': 'bar'})
+
+# sets
+
+myset = {1, 2, 3}
+
+for _ in myset:
+    # errors
+    myset.update({4, 5})
+    myset.intersection_update({4, 5})
+    myset.difference_update({4, 5})
+    myset.symmetric_difference_update({4, 5})
+    myset.add(4)
+    myset.discard(3)
+
+    # no errors
+    del myset
+
+
+# members
+class A:
+    some_list: list
+
+    def __init__(self, ls):
+        self.some_list = list(ls)
+
+
+a = A((1, 2, 3))
+# ensure member accesses are handled
+for elem in a.some_list:
+    a.some_list.remove(elem)
+    del a.some_list[2]
+
+
+# Augassign
+
+foo = [1, 2, 3]
+bar = [4, 5, 6]
+for _ in foo:
+    foo *= 2
+    foo += bar
+    foo[1] = 9 #todo
+    foo[1:2] = bar
+    foo[1:2:3] = bar
+
+foo = {1,2,3}
+bar = {4,5,6}
+for _ in foo:
+    foo |= bar
+    foo &= bar
+    foo -= bar
+    foo ^= bar
+
+
+# more tests for unconditional breaks
+for _ in foo:
+    foo.remove(1)
+    for _ in bar:
+        bar.remove(1)
+        break
+    break
+
+# should not error
+for _ in foo:
+    foo.remove(1)
+    for _ in bar:
+        ...
+    break
+
+# should error (?)
+for _ in foo:
+    foo.remove(1)
+    if bar:
+        bar.remove(1)
+        break
+    break
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index 158eea5b6a029..7fc0d1fb823e5 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -376,7 +376,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
         (Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
         (Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
-        (Flake8Bugbear, "038") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
+        (Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
         (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
 
diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
index 2aaf082e286db..4049d4a0149eb 100644
--- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
@@ -61,7 +61,7 @@ mod tests {
     #[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.ipynb"))]
     #[test_case(Rule::UselessExpression, Path::new("B018.py"))]
-    #[test_case(Rule::LoopIteratorMutation, Path::new("B038.py"))]
+    #[test_case(Rule::LoopIteratorMutation, Path::new("B909.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(
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 5e506a4563c71..ac770d6f513a2 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
@@ -82,7 +82,7 @@ fn _to_name_str(node: &Expr) -> String {
         }
     }
 }
-// B038
+// B909
 pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor) {
     let StmtFor {
         target: _,

From 5e9ccd7f0f7af666d682121bfd02f11ba6896180 Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 11 Apr 2024 16:57:08 +0200
Subject: [PATCH 06/16] feat(flake8-bugbear): Implement B909 parity

---
 .../test/fixtures/flake8_bugbear/B909.py      |  10 +-
 .../rules/loop_iterator_mutation.rs           | 187 ++++++++---
 ...__flake8_bugbear__tests__B909_B909.py.snap | 312 ++++++++++++++++++
 ruff.schema.json                              |   4 +-
 4 files changed, 461 insertions(+), 52 deletions(-)
 create mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap

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 00ee343d86b13..456cdefec76d7 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -79,33 +79,33 @@ def __init__(self, ls):
 
 
 a = A((1, 2, 3))
-# ensure member accesses are handled
+# ensure member accesses are handled as errors
 for elem in a.some_list:
     a.some_list.remove(elem)
     del a.some_list[2]
 
 
-# Augassign
+# Augassign should error
 
 foo = [1, 2, 3]
 bar = [4, 5, 6]
 for _ in foo:
     foo *= 2
     foo += bar
-    foo[1] = 9 #todo
+    foo[1] = 9
     foo[1:2] = bar
     foo[1:2:3] = bar
 
 foo = {1,2,3}
 bar = {4,5,6}
-for _ in foo:
+for _ in foo: # should error
     foo |= bar
     foo &= bar
     foo -= bar
     foo ^= bar
 
 
-# more tests for unconditional breaks
+# more tests for unconditional breaks - should not error
 for _ in foo:
     foo.remove(1)
     for _ in bar:
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 ac770d6f513a2..e2689ec40db53 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
@@ -1,8 +1,11 @@
+use std::collections::HashMap;
+
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{
     visitor::{self, Visitor},
-    Expr, ExprAttribute, ExprCall, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
+    Arguments, ElifElseClause, Expr, ExprAttribute, ExprCall, ExprName, ExprSubscript, Operator,
+    Stmt, StmtAssign, StmtAugAssign, StmtBreak, StmtDelete, StmtFor, StmtIf,
 };
 use ruff_text_size::TextRange;
 
@@ -21,6 +24,13 @@ fn is_mutating_function(function_name: &str) -> bool {
             | "insert"
             | "pop"
             | "popitem"
+            | "setdefault"
+            | "update"
+            | "intersection_update"
+            | "difference_update"
+            | "symmetric_difference_update"
+            | "add"
+            | "discard"
     )
 }
 /// ## What it does
@@ -108,48 +118,148 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
     }
     let mut visitor = LoopMutationsVisitor {
         name: &name,
-        mutations: Vec::new(),
+        mutations: HashMap::new(),
+        _contidional_block: 0,
     };
     visitor.visit_body(body);
-    for mutation in visitor.mutations {
+    for mutation in visitor.mutations.values().flatten() {
         checker
             .diagnostics
-            .push(Diagnostic::new(LoopIteratorMutation, mutation));
+            .push(Diagnostic::new(LoopIteratorMutation, *mutation));
     }
 }
+
 struct LoopMutationsVisitor<'a> {
     name: &'a str,
-    mutations: Vec<TextRange>,
+    mutations: HashMap<u8, Vec<TextRange>>,
+    _contidional_block: u8,
 }
 
+impl<'a> LoopMutationsVisitor<'a> {
+    fn add_mutation(&mut self, range: &TextRange) {
+        if !self.mutations.contains_key(&self._contidional_block) {
+            self.mutations.insert(self._contidional_block, Vec::new());
+        }
+        match self.mutations.get_mut(&self._contidional_block) {
+            Some(a) => a.push(*range),
+            None => {}
+        }
+    }
+
+    fn handle_delete(&mut self, range: &TextRange, targets: &'a Vec<Expr>) {
+        for target in targets {
+            let name;
+            match target {
+                Expr::Subscript(ExprSubscript {
+                    range: _,
+                    value,
+                    slice: _,
+                    ctx: _,
+                }) => {
+                    name = _to_name_str(value);
+                }
+
+                Expr::Attribute(_) | Expr::Name(_) => {
+                    name = String::new(); // ignore reference deletion
+                }
+                _ => {
+                    name = String::new();
+                    visitor::walk_expr(self, target);
+                }
+            }
+            if self.name.eq(&name) {
+                self.add_mutation(range);
+            }
+        }
+    }
+
+    fn handle_assign(&mut self, range: &TextRange, targets: &'a Vec<Expr>, _value: &Box<Expr>) {
+        for target in targets {
+            match target {
+                Expr::Subscript(ExprSubscript {
+                    range: _,
+                    value,
+                    slice: _,
+                    ctx: _,
+                }) => {
+                    if self.name.eq(&_to_name_str(value)) {
+                        self.add_mutation(range)
+                    }
+                }
+                _ => visitor::walk_expr(self, target),
+            }
+        }
+    }
+
+    fn handle_aug_assign(
+        &mut self,
+        range: &TextRange,
+        target: &Box<Expr>,
+        _op: &Operator,
+        _value: &Box<Expr>,
+    ) {
+        if self.name.eq(&_to_name_str(target)) {
+            self.add_mutation(range)
+        }
+    }
+
+    fn handle_call(&mut self, _range: &TextRange, func: &Box<Expr>, _arguments: &Arguments) {
+        match func.as_ref() {
+            Expr::Attribute(ExprAttribute {
+                range,
+                value,
+                attr,
+                ctx: _,
+            }) => {
+                let name = _to_name_str(value);
+                if self.name.eq(&name) && is_mutating_function(&attr.as_str()) {
+                    self.add_mutation(range);
+                }
+            }
+            _ => {}
+        }
+    }
+
+    fn handle_if(
+        &mut self,
+        _range: &TextRange,
+        _test: &Box<Expr>,
+        body: &'a Vec<Stmt>,
+        _elif_else_clauses: &Vec<ElifElseClause>,
+    ) {
+        self._contidional_block += 1;
+        self.visit_body(body);
+        self._contidional_block += 1;
+    }
+}
 /// `Visitor` to collect all used identifiers in a statement.
 impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
     fn visit_stmt(&mut self, stmt: &'a Stmt) {
         match stmt {
-            Stmt::Delete(StmtDelete { range, targets }) => {
-                for target in targets {
-                    let name;
-                    match target {
-                        Expr::Subscript(ExprSubscript {
-                            range: _,
-                            value,
-                            slice: _,
-                            ctx: _,
-                        }) => {
-                            name = _to_name_str(value);
-                        }
-
-                        Expr::Attribute(_) | Expr::Name(_) => {
-                            name = _to_name_str(target);
-                        }
-                        _ => {
-                            name = String::new();
-                            visitor::walk_expr(self, target);
-                        }
-                    }
-                    if self.name.eq(&name) {
-                        self.mutations.push(*range);
-                    }
+            Stmt::Delete(StmtDelete { range, targets }) => self.handle_delete(range, targets),
+            Stmt::Assign(StmtAssign {
+                range,
+                targets,
+                value,
+            }) => self.handle_assign(range, targets, value),
+            Stmt::AugAssign(StmtAugAssign {
+                range,
+                target,
+                op,
+                value,
+            }) => {
+                self.handle_aug_assign(range, target, op, value);
+            }
+            Stmt::If(StmtIf {
+                range,
+                test,
+                body,
+                elif_else_clauses,
+            }) => self.handle_if(range, test, body, elif_else_clauses),
+            Stmt::Break(StmtBreak { range: _ }) => {
+                match self.mutations.get_mut(&self._contidional_block) {
+                    Some(a) => a.clear(),
+                    None => {}
                 }
             }
             _ => {
@@ -161,23 +271,10 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
     fn visit_expr(&mut self, expr: &'a Expr) {
         match expr {
             Expr::Call(ExprCall {
-                range: _,
+                range,
                 func,
-                arguments: _,
-            }) => match func.as_ref() {
-                Expr::Attribute(ExprAttribute {
-                    range,
-                    value,
-                    attr,
-                    ctx: _,
-                }) => {
-                    let name = _to_name_str(value);
-                    if self.name.eq(&name) && is_mutating_function(&attr.as_str()) {
-                        self.mutations.push(*range);
-                    }
-                }
-                _ => {}
-            },
+                arguments,
+            }) => self.handle_call(range, 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
new file mode 100644
index 0000000000000..1be1f6a404587
--- /dev/null
+++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B909_B909.py.snap
@@ -0,0 +1,312 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+---
+B909.py:12:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+10 | for elem in some_list:
+11 |     # errors
+12 |     some_list.remove(elem)
+   |     ^^^^^^^^^^^^^^^^ B909
+13 |     del some_list[2]
+14 |     some_list.append(elem)
+   |
+
+B909.py:13:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+11 |     # errors
+12 |     some_list.remove(elem)
+13 |     del some_list[2]
+   |     ^^^^^^^^^^^^^^^^ B909
+14 |     some_list.append(elem)
+15 |     some_list.sort()
+   |
+
+B909.py:14:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+12 |     some_list.remove(elem)
+13 |     del some_list[2]
+14 |     some_list.append(elem)
+   |     ^^^^^^^^^^^^^^^^ B909
+15 |     some_list.sort()
+16 |     some_list.reverse()
+   |
+
+B909.py:15:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+13 |     del some_list[2]
+14 |     some_list.append(elem)
+15 |     some_list.sort()
+   |     ^^^^^^^^^^^^^^ B909
+16 |     some_list.reverse()
+17 |     some_list.clear()
+   |
+
+B909.py:16:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+14 |     some_list.append(elem)
+15 |     some_list.sort()
+16 |     some_list.reverse()
+   |     ^^^^^^^^^^^^^^^^^ B909
+17 |     some_list.clear()
+18 |     some_list.extend([1, 2])
+   |
+
+B909.py:17:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+15 |     some_list.sort()
+16 |     some_list.reverse()
+17 |     some_list.clear()
+   |     ^^^^^^^^^^^^^^^ B909
+18 |     some_list.extend([1, 2])
+19 |     some_list.insert(1, 1)
+   |
+
+B909.py:18:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+16 |     some_list.reverse()
+17 |     some_list.clear()
+18 |     some_list.extend([1, 2])
+   |     ^^^^^^^^^^^^^^^^ B909
+19 |     some_list.insert(1, 1)
+20 |     some_list.pop(1)
+   |
+
+B909.py:19:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+17 |     some_list.clear()
+18 |     some_list.extend([1, 2])
+19 |     some_list.insert(1, 1)
+   |     ^^^^^^^^^^^^^^^^ B909
+20 |     some_list.pop(1)
+21 |     some_list.pop()
+   |
+
+B909.py:20:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+18 |     some_list.extend([1, 2])
+19 |     some_list.insert(1, 1)
+20 |     some_list.pop(1)
+   |     ^^^^^^^^^^^^^ B909
+21 |     some_list.pop()
+   |
+
+B909.py:21:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+19 |     some_list.insert(1, 1)
+20 |     some_list.pop(1)
+21 |     some_list.pop()
+   |     ^^^^^^^^^^^^^ B909
+22 | 
+23 |     # conditional break should error
+   |
+
+B909.py:25:9: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+23 |     # conditional break should error
+24 |     if elem == 2:
+25 |         some_list.remove(elem)
+   |         ^^^^^^^^^^^^^^^^ B909
+26 |         if elem == 3:
+27 |             break
+   |
+
+B909.py:47:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+45 | for elem in mydicts:
+46 |     # errors
+47 |     mydicts.popitem()
+   |     ^^^^^^^^^^^^^^^ B909
+48 |     mydicts.setdefault('foo', 1)
+49 |     mydicts.update({'foo': 'bar'})
+   |
+
+B909.py:48:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+46 |     # errors
+47 |     mydicts.popitem()
+48 |     mydicts.setdefault('foo', 1)
+   |     ^^^^^^^^^^^^^^^^^^ B909
+49 |     mydicts.update({'foo': 'bar'})
+   |
+
+B909.py:49:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+47 |     mydicts.popitem()
+48 |     mydicts.setdefault('foo', 1)
+49 |     mydicts.update({'foo': 'bar'})
+   |     ^^^^^^^^^^^^^^ B909
+50 | 
+51 |     # no errors
+   |
+
+B909.py:62:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+60 | for _ in myset:
+61 |     # errors
+62 |     myset.update({4, 5})
+   |     ^^^^^^^^^^^^ B909
+63 |     myset.intersection_update({4, 5})
+64 |     myset.difference_update({4, 5})
+   |
+
+B909.py:63:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+61 |     # errors
+62 |     myset.update({4, 5})
+63 |     myset.intersection_update({4, 5})
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ B909
+64 |     myset.difference_update({4, 5})
+65 |     myset.symmetric_difference_update({4, 5})
+   |
+
+B909.py:64:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+62 |     myset.update({4, 5})
+63 |     myset.intersection_update({4, 5})
+64 |     myset.difference_update({4, 5})
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ B909
+65 |     myset.symmetric_difference_update({4, 5})
+66 |     myset.add(4)
+   |
+
+B909.py:65:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+63 |     myset.intersection_update({4, 5})
+64 |     myset.difference_update({4, 5})
+65 |     myset.symmetric_difference_update({4, 5})
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B909
+66 |     myset.add(4)
+67 |     myset.discard(3)
+   |
+
+B909.py:66:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+64 |     myset.difference_update({4, 5})
+65 |     myset.symmetric_difference_update({4, 5})
+66 |     myset.add(4)
+   |     ^^^^^^^^^ B909
+67 |     myset.discard(3)
+   |
+
+B909.py:67:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+65 |     myset.symmetric_difference_update({4, 5})
+66 |     myset.add(4)
+67 |     myset.discard(3)
+   |     ^^^^^^^^^^^^^ B909
+68 | 
+69 |     # no errors
+   |
+
+B909.py:84:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+82 | # ensure member accesses are handled as errors
+83 | for elem in a.some_list:
+84 |     a.some_list.remove(elem)
+   |     ^^^^^^^^^^^^^^^^^^ B909
+85 |     del a.some_list[2]
+   |
+
+B909.py:85:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+83 | for elem in a.some_list:
+84 |     a.some_list.remove(elem)
+85 |     del a.some_list[2]
+   |     ^^^^^^^^^^^^^^^^^^ B909
+   |
+
+B909.py:93:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+91 | bar = [4, 5, 6]
+92 | for _ in foo:
+93 |     foo *= 2
+   |     ^^^^^^^^ B909
+94 |     foo += bar
+95 |     foo[1] = 9
+   |
+
+B909.py:94:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+92 | for _ in foo:
+93 |     foo *= 2
+94 |     foo += bar
+   |     ^^^^^^^^^^ B909
+95 |     foo[1] = 9
+96 |     foo[1:2] = bar
+   |
+
+B909.py:95:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+93 |     foo *= 2
+94 |     foo += bar
+95 |     foo[1] = 9
+   |     ^^^^^^^^^^ B909
+96 |     foo[1:2] = bar
+97 |     foo[1:2:3] = bar
+   |
+
+B909.py:96:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+94 |     foo += bar
+95 |     foo[1] = 9
+96 |     foo[1:2] = bar
+   |     ^^^^^^^^^^^^^^ B909
+97 |     foo[1:2:3] = bar
+   |
+
+B909.py:97:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+   |
+95 |     foo[1] = 9
+96 |     foo[1:2] = bar
+97 |     foo[1:2:3] = bar
+   |     ^^^^^^^^^^^^^^^^ B909
+98 | 
+99 | foo = {1,2,3}
+   |
+
+B909.py:102:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+    |
+100 | bar = {4,5,6}
+101 | for _ in foo: # should error
+102 |     foo |= bar
+    |     ^^^^^^^^^^ B909
+103 |     foo &= bar
+104 |     foo -= bar
+    |
+
+B909.py:103:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+    |
+101 | for _ in foo: # should error
+102 |     foo |= bar
+103 |     foo &= bar
+    |     ^^^^^^^^^^ B909
+104 |     foo -= bar
+105 |     foo ^= bar
+    |
+
+B909.py:104:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+    |
+102 |     foo |= bar
+103 |     foo &= bar
+104 |     foo -= bar
+    |     ^^^^^^^^^^ B909
+105 |     foo ^= bar
+    |
+
+B909.py:105:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+    |
+103 |     foo &= bar
+104 |     foo -= bar
+105 |     foo ^= bar
+    |     ^^^^^^^^^^ B909
+    |
+
+B909.py:125:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+    |
+123 | # should error (?)
+124 | for _ in foo:
+125 |     foo.remove(1)
+    |     ^^^^^^^^^^ B909
+126 |     if bar:
+127 |         bar.remove(1)
+    |
diff --git a/ruff.schema.json b/ruff.schema.json
index 3e00d0eee041f..198970035e1f5 100644
--- a/ruff.schema.json
+++ b/ruff.schema.json
@@ -2723,11 +2723,11 @@
         "B033",
         "B034",
         "B035",
-        "B038",
         "B9",
         "B90",
         "B904",
         "B905",
+        "B909",
         "BLE",
         "BLE0",
         "BLE00",
@@ -3945,4 +3945,4 @@
       ]
     }
   }
-}
+}
\ No newline at end of file

From 404c0b903aea7aa604e03b584764ccbb3ad38904 Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 11 Apr 2024 17:10:01 +0200
Subject: [PATCH 07/16] refactor(flake8-bugbear): Use unreachable!() marcro
 appropriately

---
 .../src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs   | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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 e2689ec40db53..b0f9c2b2a2c9f 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
@@ -112,8 +112,7 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
             name = _to_name_str(iter.as_ref());
         }
         _ => {
-            println!("Shouldn't happen");
-            return;
+            unreachable!()
         }
     }
     let mut visitor = LoopMutationsVisitor {

From 95bf019b352c4108a00b175e5640f7f45f012efb Mon Sep 17 00:00:00 2001
From: Martin Imre <martinimre25@gmail.com>
Date: Thu, 11 Apr 2024 17:18:59 +0200
Subject: [PATCH 08/16] chore: Delete unused file

---
 .../test/fixtures/flake8_bugbear/B038.py      | 66 -------------------
 1 file changed, 66 deletions(-)
 delete mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py

diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
deleted file mode 100644
index 093ad7cd1fb41..0000000000000
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B038.py
+++ /dev/null
@@ -1,66 +0,0 @@
-some_list = [1, 2, 3]
-for elem in some_list:
-    print(elem)
-    if elem % 2 == 0:
-        some_list.remove(elem)  # should error
-
-some_list = [1, 2, 3]
-some_other_list = [1, 2, 3]
-for elem in some_list:
-    print(elem)
-    if elem % 2 == 0:
-        some_other_list.remove(elem)  # should not error
-        del some_other_list
-
-
-some_list = [1, 2, 3]
-for elem in some_list:
-    print(elem)
-    if elem % 2 == 0:
-        del some_list[2]  # should error
-        del some_list
-
-
-class A:
-    some_list: list
-
-    def __init__(self, ls):
-        self.some_list = list(ls)
-
-
-a = A((1, 2, 3))
-for elem in a.some_list:
-    print(elem)
-    if elem % 2 == 0:
-        a.some_list.remove(elem)  # should error
-
-a = A((1, 2, 3))
-for elem in a.some_list:
-    print(elem)
-    if elem % 2 == 0:
-        del a.some_list[2]  # should error
-
-
-some_list = [1, 2, 3]
-for elem in some_list:
-    print(elem)
-    if elem == 2:
-        found_idx = some_list.index(elem)  # should not error
-        some_list.append(elem)  # should error
-        some_list.sort()  # should error
-        some_list.reverse()  # should error
-        some_list.clear()  # should error
-        some_list.extend([1, 2])  # should error
-        some_list.insert(1, 1)  # should error
-        some_list.pop(1)  # should error
-        some_list.pop()  # should error
-        some_list = 3  # should error
-        break
-
-
-mydicts = {"a": {"foo": 1, "bar": 2}}
-
-for mydict in mydicts:
-    if mydicts.get("a", ""):
-        print(mydict["foo"])  # should not error
-        mydicts.popitem()  # should error

From 6418c18dd97dc3578273609893cbdfe3098596c5 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 13:47:44 -0400
Subject: [PATCH 09/16] Format fixture

---
 .../test/fixtures/flake8_bugbear/B909.py      | 18 ++++++++---------
 ...__flake8_bugbear__tests__B909_B909.py.snap | 20 +++++++++----------
 2 files changed, 19 insertions(+), 19 deletions(-)

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 456cdefec76d7..5fa828b87aaa2 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -1,6 +1,6 @@
 """
 Should emit:
-B999 - on lines 11, 25, 26, 40, 46
+B909 - on lines 11, 25, 26, 40, 46
 """
 
 # lists
@@ -40,18 +40,18 @@
 
 
 # dicts
-mydicts = {'a': {'foo': 1, 'bar': 2}}
+mydicts = {"a": {"foo": 1, "bar": 2}}
 
 for elem in mydicts:
     # errors
     mydicts.popitem()
-    mydicts.setdefault('foo', 1)
-    mydicts.update({'foo': 'bar'})
+    mydicts.setdefault("foo", 1)
+    mydicts.update({"foo": "bar"})
 
     # no errors
     elem.popitem()
-    elem.setdefault('foo', 1)
-    elem.update({'foo': 'bar'})
+    elem.setdefault("foo", 1)
+    elem.update({"foo": "bar"})
 
 # sets
 
@@ -96,9 +96,9 @@ def __init__(self, ls):
     foo[1:2] = bar
     foo[1:2:3] = bar
 
-foo = {1,2,3}
-bar = {4,5,6}
-for _ in foo: # should error
+foo = {1, 2, 3}
+bar = {4, 5, 6}
+for _ in foo:  # should error
     foo |= bar
     foo &= bar
     foo -= bar
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 1be1f6a404587..0e5dffe49954e 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
@@ -116,24 +116,24 @@ B909.py:47:5: B909 editing a loop's mutable iterable often leads to unexpected r
 46 |     # errors
 47 |     mydicts.popitem()
    |     ^^^^^^^^^^^^^^^ B909
-48 |     mydicts.setdefault('foo', 1)
-49 |     mydicts.update({'foo': 'bar'})
+48 |     mydicts.setdefault("foo", 1)
+49 |     mydicts.update({"foo": "bar"})
    |
 
 B909.py:48:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
 46 |     # errors
 47 |     mydicts.popitem()
-48 |     mydicts.setdefault('foo', 1)
+48 |     mydicts.setdefault("foo", 1)
    |     ^^^^^^^^^^^^^^^^^^ B909
-49 |     mydicts.update({'foo': 'bar'})
+49 |     mydicts.update({"foo": "bar"})
    |
 
 B909.py:49:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
    |
 47 |     mydicts.popitem()
-48 |     mydicts.setdefault('foo', 1)
-49 |     mydicts.update({'foo': 'bar'})
+48 |     mydicts.setdefault("foo", 1)
+49 |     mydicts.update({"foo": "bar"})
    |     ^^^^^^^^^^^^^^ B909
 50 | 
 51 |     # no errors
@@ -261,13 +261,13 @@ B909.py:97:5: B909 editing a loop's mutable iterable often leads to unexpected r
 97 |     foo[1:2:3] = bar
    |     ^^^^^^^^^^^^^^^^ B909
 98 | 
-99 | foo = {1,2,3}
+99 | foo = {1, 2, 3}
    |
 
 B909.py:102:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
     |
-100 | bar = {4,5,6}
-101 | for _ in foo: # should error
+100 | bar = {4, 5, 6}
+101 | for _ in foo:  # should error
 102 |     foo |= bar
     |     ^^^^^^^^^^ B909
 103 |     foo &= bar
@@ -276,7 +276,7 @@ B909.py:102:5: B909 editing a loop's mutable iterable often leads to unexpected
 
 B909.py:103:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
     |
-101 | for _ in foo: # should error
+101 | for _ in foo:  # should error
 102 |     foo |= bar
 103 |     foo &= bar
     |     ^^^^^^^^^^ B909

From 88c4e2fcd67df1ca5c7ba080b3007d74222374e1 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 13:55:13 -0400
Subject: [PATCH 10/16] Modify documentation

---
 .../src/checkers/ast/analyze/statement.rs     |  2 +-
 crates/ruff_linter/src/codes.rs               |  2 +-
 .../rules/loop_iterator_mutation.rs           | 93 ++++++++++---------
 ...__flake8_bugbear__tests__B909_B909.py.snap | 64 ++++++-------
 4 files changed, 82 insertions(+), 79 deletions(-)

diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
index 45f799561a177..96dcf5df55681 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -1268,10 +1268,10 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.any_enabled(&[
                 Rule::EnumerateForLoop,
                 Rule::IncorrectDictIterator,
+                Rule::LoopIteratorMutation,
                 Rule::UnnecessaryEnumerate,
                 Rule::UnusedLoopControlVariable,
                 Rule::YieldInForLoop,
-                Rule::LoopIteratorMutation,
             ]) {
                 checker.analyze.for_loops.push(checker.semantic.snapshot());
             }
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index 7fc0d1fb823e5..3bde1d732a614 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -376,9 +376,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
         (Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
         (Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
-        (Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
         (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
+        (Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
 
         // flake8-blind-except
         (Flake8BlindExcept, "001") => (RuleGroup::Stable, rules::flake8_blind_except::rules::BlindExcept),
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 b0f9c2b2a2c9f..3bbabe0b2e024 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
@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 
+use ruff_diagnostics::Diagnostic;
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{
@@ -10,44 +11,23 @@ use ruff_python_ast::{
 use ruff_text_size::TextRange;
 
 use crate::checkers::ast::Checker;
-use ruff_diagnostics::Diagnostic;
 
-fn is_mutating_function(function_name: &str) -> bool {
-    matches!(
-        function_name,
-        "append"
-            | "sort"
-            | "reverse"
-            | "remove"
-            | "clear"
-            | "extend"
-            | "insert"
-            | "pop"
-            | "popitem"
-            | "setdefault"
-            | "update"
-            | "intersection_update"
-            | "difference_update"
-            | "symmetric_difference_update"
-            | "add"
-            | "discard"
-    )
-}
 /// ## What it does
-/// Checks for mutation of the iterator of a loop in the loop's body
+/// Checks for mutations to an iterable during a loop iteration.
 ///
 /// ## Why is this bad?
-/// Changing the structure that is being iterated over will usually lead to
-/// unintended behavior as not all elements will be addressed.
+/// When iterating over an iterable, mutating the iterable can lead to unexpected
+/// behavior, like skipping elements or infinite loops.
 ///
 /// ## Example
 /// ```python
-/// some_list = [1,2,3]
-/// for i in some_list:
-///   some_list.remove(i) # this will lead to not all elements being printed
-///   print(i)
-/// ```
+/// items = [1,2,3]
+/// for item in items:
+///     print(item)
 ///
+///     # Create an infinite loop by appending to the list.
+///     items.append(item)
+/// ```
 ///
 /// ## References
 /// - [Python documentation: Mutable Sequence Types](https://docs.python.org/3/library/stdtypes.html#typesseq-mutable)
@@ -57,7 +37,7 @@ pub struct LoopIteratorMutation;
 impl Violation for LoopIteratorMutation {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("editing a loop's mutable iterable often leads to unexpected results/bugs")
+        format!("Mutation to loop iterable during iteration")
     }
 }
 
@@ -92,7 +72,8 @@ fn _to_name_str(node: &Expr) -> String {
         }
     }
 }
-// B909
+
+/// B909
 pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor) {
     let StmtFor {
         target: _,
@@ -118,7 +99,7 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
     let mut visitor = LoopMutationsVisitor {
         name: &name,
         mutations: HashMap::new(),
-        _contidional_block: 0,
+        branch: 0,
     };
     visitor.visit_body(body);
     for mutation in visitor.mutations.values().flatten() {
@@ -128,18 +109,41 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
     }
 }
 
+/// Returns `true` if the method mutates when called on an iterator.
+fn is_mutating_function(function_name: &str) -> bool {
+    matches!(
+        function_name,
+        "append"
+            | "sort"
+            | "reverse"
+            | "remove"
+            | "clear"
+            | "extend"
+            | "insert"
+            | "pop"
+            | "popitem"
+            | "setdefault"
+            | "update"
+            | "intersection_update"
+            | "difference_update"
+            | "symmetric_difference_update"
+            | "add"
+            | "discard"
+    )
+}
+
 struct LoopMutationsVisitor<'a> {
     name: &'a str,
     mutations: HashMap<u8, Vec<TextRange>>,
-    _contidional_block: u8,
+    branch: u8,
 }
 
 impl<'a> LoopMutationsVisitor<'a> {
     fn add_mutation(&mut self, range: &TextRange) {
-        if !self.mutations.contains_key(&self._contidional_block) {
-            self.mutations.insert(self._contidional_block, Vec::new());
+        if !self.mutations.contains_key(&self.branch) {
+            self.mutations.insert(self.branch, Vec::new());
         }
-        match self.mutations.get_mut(&self._contidional_block) {
+        match self.mutations.get_mut(&self.branch) {
             Some(a) => a.push(*range),
             None => {}
         }
@@ -226,11 +230,12 @@ impl<'a> LoopMutationsVisitor<'a> {
         body: &'a Vec<Stmt>,
         _elif_else_clauses: &Vec<ElifElseClause>,
     ) {
-        self._contidional_block += 1;
+        self.branch += 1;
         self.visit_body(body);
-        self._contidional_block += 1;
+        self.branch += 1;
     }
 }
+
 /// `Visitor` to collect all used identifiers in a statement.
 impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
     fn visit_stmt(&mut self, stmt: &'a Stmt) {
@@ -255,12 +260,10 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
                 body,
                 elif_else_clauses,
             }) => self.handle_if(range, test, body, elif_else_clauses),
-            Stmt::Break(StmtBreak { range: _ }) => {
-                match self.mutations.get_mut(&self._contidional_block) {
-                    Some(a) => a.clear(),
-                    None => {}
-                }
-            }
+            Stmt::Break(StmtBreak { range: _ }) => match self.mutations.get_mut(&self.branch) {
+                Some(a) => a.clear(),
+                None => {}
+            },
             _ => {
                 visitor::walk_stmt(self, stmt);
             }
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 0e5dffe49954e..98e85f8345c2f 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
@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
 ---
-B909.py:12:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:12:5: B909 Mutation to loop iterable during iteration
    |
 10 | for elem in some_list:
 11 |     # errors
@@ -11,7 +11,7 @@ B909.py:12:5: B909 editing a loop's mutable iterable often leads to unexpected r
 14 |     some_list.append(elem)
    |
 
-B909.py:13:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:13:5: B909 Mutation to loop iterable during iteration
    |
 11 |     # errors
 12 |     some_list.remove(elem)
@@ -21,7 +21,7 @@ B909.py:13:5: B909 editing a loop's mutable iterable often leads to unexpected r
 15 |     some_list.sort()
    |
 
-B909.py:14:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:14:5: B909 Mutation to loop iterable during iteration
    |
 12 |     some_list.remove(elem)
 13 |     del some_list[2]
@@ -31,7 +31,7 @@ B909.py:14:5: B909 editing a loop's mutable iterable often leads to unexpected r
 16 |     some_list.reverse()
    |
 
-B909.py:15:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:15:5: B909 Mutation to loop iterable during iteration
    |
 13 |     del some_list[2]
 14 |     some_list.append(elem)
@@ -41,7 +41,7 @@ B909.py:15:5: B909 editing a loop's mutable iterable often leads to unexpected r
 17 |     some_list.clear()
    |
 
-B909.py:16:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:16:5: B909 Mutation to loop iterable during iteration
    |
 14 |     some_list.append(elem)
 15 |     some_list.sort()
@@ -51,7 +51,7 @@ B909.py:16:5: B909 editing a loop's mutable iterable often leads to unexpected r
 18 |     some_list.extend([1, 2])
    |
 
-B909.py:17:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:17:5: B909 Mutation to loop iterable during iteration
    |
 15 |     some_list.sort()
 16 |     some_list.reverse()
@@ -61,7 +61,7 @@ B909.py:17:5: B909 editing a loop's mutable iterable often leads to unexpected r
 19 |     some_list.insert(1, 1)
    |
 
-B909.py:18:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:18:5: B909 Mutation to loop iterable during iteration
    |
 16 |     some_list.reverse()
 17 |     some_list.clear()
@@ -71,7 +71,7 @@ B909.py:18:5: B909 editing a loop's mutable iterable often leads to unexpected r
 20 |     some_list.pop(1)
    |
 
-B909.py:19:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:19:5: B909 Mutation to loop iterable during iteration
    |
 17 |     some_list.clear()
 18 |     some_list.extend([1, 2])
@@ -81,7 +81,7 @@ B909.py:19:5: B909 editing a loop's mutable iterable often leads to unexpected r
 21 |     some_list.pop()
    |
 
-B909.py:20:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:20:5: B909 Mutation to loop iterable during iteration
    |
 18 |     some_list.extend([1, 2])
 19 |     some_list.insert(1, 1)
@@ -90,7 +90,7 @@ B909.py:20:5: B909 editing a loop's mutable iterable often leads to unexpected r
 21 |     some_list.pop()
    |
 
-B909.py:21:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:21:5: B909 Mutation to loop iterable during iteration
    |
 19 |     some_list.insert(1, 1)
 20 |     some_list.pop(1)
@@ -100,7 +100,7 @@ B909.py:21:5: B909 editing a loop's mutable iterable often leads to unexpected r
 23 |     # conditional break should error
    |
 
-B909.py:25:9: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:25:9: B909 Mutation to loop iterable during iteration
    |
 23 |     # conditional break should error
 24 |     if elem == 2:
@@ -110,7 +110,7 @@ B909.py:25:9: B909 editing a loop's mutable iterable often leads to unexpected r
 27 |             break
    |
 
-B909.py:47:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:47:5: B909 Mutation to loop iterable during iteration
    |
 45 | for elem in mydicts:
 46 |     # errors
@@ -120,7 +120,7 @@ B909.py:47:5: B909 editing a loop's mutable iterable often leads to unexpected r
 49 |     mydicts.update({"foo": "bar"})
    |
 
-B909.py:48:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:48:5: B909 Mutation to loop iterable during iteration
    |
 46 |     # errors
 47 |     mydicts.popitem()
@@ -129,7 +129,7 @@ B909.py:48:5: B909 editing a loop's mutable iterable often leads to unexpected r
 49 |     mydicts.update({"foo": "bar"})
    |
 
-B909.py:49:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:49:5: B909 Mutation to loop iterable during iteration
    |
 47 |     mydicts.popitem()
 48 |     mydicts.setdefault("foo", 1)
@@ -139,7 +139,7 @@ B909.py:49:5: B909 editing a loop's mutable iterable often leads to unexpected r
 51 |     # no errors
    |
 
-B909.py:62:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:62:5: B909 Mutation to loop iterable during iteration
    |
 60 | for _ in myset:
 61 |     # errors
@@ -149,7 +149,7 @@ B909.py:62:5: B909 editing a loop's mutable iterable often leads to unexpected r
 64 |     myset.difference_update({4, 5})
    |
 
-B909.py:63:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:63:5: B909 Mutation to loop iterable during iteration
    |
 61 |     # errors
 62 |     myset.update({4, 5})
@@ -159,7 +159,7 @@ B909.py:63:5: B909 editing a loop's mutable iterable often leads to unexpected r
 65 |     myset.symmetric_difference_update({4, 5})
    |
 
-B909.py:64:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:64:5: B909 Mutation to loop iterable during iteration
    |
 62 |     myset.update({4, 5})
 63 |     myset.intersection_update({4, 5})
@@ -169,7 +169,7 @@ B909.py:64:5: B909 editing a loop's mutable iterable often leads to unexpected r
 66 |     myset.add(4)
    |
 
-B909.py:65:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:65:5: B909 Mutation to loop iterable during iteration
    |
 63 |     myset.intersection_update({4, 5})
 64 |     myset.difference_update({4, 5})
@@ -179,7 +179,7 @@ B909.py:65:5: B909 editing a loop's mutable iterable often leads to unexpected r
 67 |     myset.discard(3)
    |
 
-B909.py:66:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:66:5: B909 Mutation to loop iterable during iteration
    |
 64 |     myset.difference_update({4, 5})
 65 |     myset.symmetric_difference_update({4, 5})
@@ -188,7 +188,7 @@ B909.py:66:5: B909 editing a loop's mutable iterable often leads to unexpected r
 67 |     myset.discard(3)
    |
 
-B909.py:67:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:67:5: B909 Mutation to loop iterable during iteration
    |
 65 |     myset.symmetric_difference_update({4, 5})
 66 |     myset.add(4)
@@ -198,7 +198,7 @@ B909.py:67:5: B909 editing a loop's mutable iterable often leads to unexpected r
 69 |     # no errors
    |
 
-B909.py:84:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:84:5: B909 Mutation to loop iterable during iteration
    |
 82 | # ensure member accesses are handled as errors
 83 | for elem in a.some_list:
@@ -207,7 +207,7 @@ B909.py:84:5: B909 editing a loop's mutable iterable often leads to unexpected r
 85 |     del a.some_list[2]
    |
 
-B909.py:85:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:85:5: B909 Mutation to loop iterable during iteration
    |
 83 | for elem in a.some_list:
 84 |     a.some_list.remove(elem)
@@ -215,7 +215,7 @@ B909.py:85:5: B909 editing a loop's mutable iterable often leads to unexpected r
    |     ^^^^^^^^^^^^^^^^^^ B909
    |
 
-B909.py:93:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:93:5: B909 Mutation to loop iterable during iteration
    |
 91 | bar = [4, 5, 6]
 92 | for _ in foo:
@@ -225,7 +225,7 @@ B909.py:93:5: B909 editing a loop's mutable iterable often leads to unexpected r
 95 |     foo[1] = 9
    |
 
-B909.py:94:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:94:5: B909 Mutation to loop iterable during iteration
    |
 92 | for _ in foo:
 93 |     foo *= 2
@@ -235,7 +235,7 @@ B909.py:94:5: B909 editing a loop's mutable iterable often leads to unexpected r
 96 |     foo[1:2] = bar
    |
 
-B909.py:95:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:95:5: B909 Mutation to loop iterable during iteration
    |
 93 |     foo *= 2
 94 |     foo += bar
@@ -245,7 +245,7 @@ B909.py:95:5: B909 editing a loop's mutable iterable often leads to unexpected r
 97 |     foo[1:2:3] = bar
    |
 
-B909.py:96:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:96:5: B909 Mutation to loop iterable during iteration
    |
 94 |     foo += bar
 95 |     foo[1] = 9
@@ -254,7 +254,7 @@ B909.py:96:5: B909 editing a loop's mutable iterable often leads to unexpected r
 97 |     foo[1:2:3] = bar
    |
 
-B909.py:97:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:97:5: B909 Mutation to loop iterable during iteration
    |
 95 |     foo[1] = 9
 96 |     foo[1:2] = bar
@@ -264,7 +264,7 @@ B909.py:97:5: B909 editing a loop's mutable iterable often leads to unexpected r
 99 | foo = {1, 2, 3}
    |
 
-B909.py:102:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:102:5: B909 Mutation to loop iterable during iteration
     |
 100 | bar = {4, 5, 6}
 101 | for _ in foo:  # should error
@@ -274,7 +274,7 @@ B909.py:102:5: B909 editing a loop's mutable iterable often leads to unexpected
 104 |     foo -= bar
     |
 
-B909.py:103:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:103:5: B909 Mutation to loop iterable during iteration
     |
 101 | for _ in foo:  # should error
 102 |     foo |= bar
@@ -284,7 +284,7 @@ B909.py:103:5: B909 editing a loop's mutable iterable often leads to unexpected
 105 |     foo ^= bar
     |
 
-B909.py:104:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:104:5: B909 Mutation to loop iterable during iteration
     |
 102 |     foo |= bar
 103 |     foo &= bar
@@ -293,7 +293,7 @@ B909.py:104:5: B909 editing a loop's mutable iterable often leads to unexpected
 105 |     foo ^= bar
     |
 
-B909.py:105:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:105:5: B909 Mutation to loop iterable during iteration
     |
 103 |     foo &= bar
 104 |     foo -= bar
@@ -301,7 +301,7 @@ B909.py:105:5: B909 editing a loop's mutable iterable often leads to unexpected
     |     ^^^^^^^^^^ B909
     |
 
-B909.py:125:5: B909 editing a loop's mutable iterable often leads to unexpected results/bugs
+B909.py:125:5: B909 Mutation to loop iterable during iteration
     |
 123 | # should error (?)
 124 | for _ in foo:

From 5dbf18a737c8c5817edde8e89d78ba3b982b0498 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 14:19:31 -0400
Subject: [PATCH 11/16] Add more comments, add else branches, add name to
 diagnostic

---
 .../test/fixtures/flake8_bugbear/B909.py      |   7 +
 .../rules/loop_iterator_mutation.rs           | 255 +++++++++---------
 ...__flake8_bugbear__tests__B909_B909.py.snap |  72 ++---
 3 files changed, 173 insertions(+), 161 deletions(-)

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 5fa828b87aaa2..4381aa0d5787a 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -127,3 +127,10 @@ def __init__(self, ls):
         bar.remove(1)
         break
     break
+
+# should error
+for _ in foo:
+    if bar:
+        pass
+    else:
+        foo.remove(1)
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 3bbabe0b2e024..c2cf1de19bf5c 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
@@ -3,14 +3,17 @@ use std::collections::HashMap;
 use ruff_diagnostics::Diagnostic;
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::name::UnqualifiedName;
 use ruff_python_ast::{
     visitor::{self, Visitor},
-    Arguments, ElifElseClause, Expr, ExprAttribute, ExprCall, ExprName, ExprSubscript, Operator,
-    Stmt, StmtAssign, StmtAugAssign, StmtBreak, StmtDelete, StmtFor, StmtIf,
+    ElifElseClause, Expr, ExprAttribute, ExprCall, ExprSubscript, Operator, Stmt, StmtAssign,
+    StmtAugAssign, StmtBreak, StmtDelete, StmtFor, StmtIf,
 };
 use ruff_text_size::TextRange;
 
 use crate::checkers::ast::Checker;
+use crate::fix::snippet::SourceCodeSnippet;
 
 /// ## What it does
 /// Checks for mutations to an iterable during a loop iteration.
@@ -22,6 +25,7 @@ use crate::checkers::ast::Checker;
 /// ## Example
 /// ```python
 /// items = [1,2,3]
+///
 /// for item in items:
 ///     print(item)
 ///
@@ -32,43 +36,19 @@ use crate::checkers::ast::Checker;
 /// ## References
 /// - [Python documentation: Mutable Sequence Types](https://docs.python.org/3/library/stdtypes.html#typesseq-mutable)
 #[violation]
-pub struct LoopIteratorMutation;
+pub struct LoopIteratorMutation {
+    name: Option<SourceCodeSnippet>,
+}
 
 impl Violation for LoopIteratorMutation {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("Mutation to loop iterable during iteration")
-    }
-}
+        let LoopIteratorMutation { name } = self;
 
-fn _to_name_str(node: &Expr) -> String {
-    match node {
-        Expr::Name(ExprName { id, .. }) => {
-            return id.to_string();
-        }
-        Expr::Attribute(ExprAttribute {
-            range: _,
-            value,
-            attr,
-            ..
-        }) => {
-            let mut inner = _to_name_str(value);
-            match inner.as_str() {
-                "" => {
-                    return "".into();
-                }
-                _ => {
-                    inner.push_str(".");
-                    inner.push_str(attr);
-                    return inner;
-                }
-            }
-        }
-        Expr::Call(ExprCall { range: _, func, .. }) => {
-            return _to_name_str(func);
-        }
-        _ => {
-            return "".into();
+        if let Some(name) = name.as_ref().and_then(SourceCodeSnippet::full_display) {
+            format!("Mutation to loop iterable `{}` during iteration", name)
+        } else {
+            format!("Mutation to loop iterable during iteration")
         }
     }
 }
@@ -83,29 +63,26 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
         is_async: _,
         range: _,
     } = stmt_for;
-    let name;
 
-    match iter.as_ref() {
-        Expr::Name(ExprName { .. }) => {
-            name = _to_name_str(iter.as_ref());
-        }
-        Expr::Attribute(ExprAttribute { .. }) => {
-            name = _to_name_str(iter.as_ref());
-        }
-        _ => {
-            unreachable!()
-        }
+    if !matches!(iter.as_ref(), Expr::Name(_) | Expr::Attribute(_)) {
+        return;
     }
-    let mut visitor = LoopMutationsVisitor {
-        name: &name,
-        mutations: HashMap::new(),
-        branch: 0,
+
+    // Collect mutations to the iterable.
+    let mutations = {
+        let mut visitor = LoopMutationsVisitor::new(iter);
+        visitor.visit_body(body);
+        visitor.mutations
     };
-    visitor.visit_body(body);
-    for mutation in visitor.mutations.values().flatten() {
+
+    // Create a diagnostic for each mutation.
+    for mutation in mutations.values().flatten() {
+        let name = UnqualifiedName::from_expr(iter)
+            .map(|name| name.to_string())
+            .map(SourceCodeSnippet::new);
         checker
             .diagnostics
-            .push(Diagnostic::new(LoopIteratorMutation, *mutation));
+            .push(Diagnostic::new(LoopIteratorMutation { name }, *mutation));
     }
 }
 
@@ -132,107 +109,110 @@ fn is_mutating_function(function_name: &str) -> bool {
     )
 }
 
+/// A visitor to collect mutations to a variable in a loop.
+#[derive(Debug, Clone)]
 struct LoopMutationsVisitor<'a> {
-    name: &'a str,
+    name: &'a Expr,
     mutations: HashMap<u8, Vec<TextRange>>,
+    branches: Vec<u8>,
     branch: u8,
 }
 
 impl<'a> LoopMutationsVisitor<'a> {
-    fn add_mutation(&mut self, range: &TextRange) {
-        if !self.mutations.contains_key(&self.branch) {
-            self.mutations.insert(self.branch, Vec::new());
-        }
-        match self.mutations.get_mut(&self.branch) {
-            Some(a) => a.push(*range),
-            None => {}
+    /// Initialize the visitor.
+    fn new(name: &'a Expr) -> Self {
+        Self {
+            name,
+            mutations: HashMap::new(),
+            branches: vec![0],
+            branch: 0,
         }
     }
 
-    fn handle_delete(&mut self, range: &TextRange, targets: &'a Vec<Expr>) {
-        for target in targets {
-            let name;
-            match target {
-                Expr::Subscript(ExprSubscript {
-                    range: _,
-                    value,
-                    slice: _,
-                    ctx: _,
-                }) => {
-                    name = _to_name_str(value);
-                }
+    /// Register a mutation.
+    fn add_mutation(&mut self, range: TextRange) {
+        self.mutations
+            .entry(self.branch)
+            .or_insert(Vec::new())
+            .push(range);
+    }
 
-                Expr::Attribute(_) | Expr::Name(_) => {
-                    name = String::new(); // ignore reference deletion
-                }
-                _ => {
-                    name = String::new();
-                    visitor::walk_expr(self, target);
+    /// Handle, e.g., `del items[0]`.
+    fn handle_delete(&mut self, range: TextRange, targets: &'a Vec<Expr>) {
+        for target in targets {
+            if let Expr::Subscript(ExprSubscript {
+                range: _,
+                value,
+                slice: _,
+                ctx: _,
+            }) = target
+            {
+                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
+                    self.add_mutation(range);
                 }
             }
-            if self.name.eq(&name) {
-                self.add_mutation(range);
-            }
         }
     }
 
-    fn handle_assign(&mut self, range: &TextRange, targets: &'a Vec<Expr>, _value: &Box<Expr>) {
+    /// Handle, e.g., `items[0] = 1`.
+    fn handle_assign(&mut self, range: TextRange, targets: &'a Vec<Expr>, _value: &Box<Expr>) {
         for target in targets {
-            match target {
-                Expr::Subscript(ExprSubscript {
-                    range: _,
-                    value,
-                    slice: _,
-                    ctx: _,
-                }) => {
-                    if self.name.eq(&_to_name_str(value)) {
-                        self.add_mutation(range)
-                    }
+            if let Expr::Subscript(ExprSubscript {
+                range: _,
+                value,
+                slice: _,
+                ctx: _,
+            }) = target
+            {
+                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
+                    self.add_mutation(range);
                 }
-                _ => visitor::walk_expr(self, target),
             }
         }
     }
 
+    /// Handle, e.g., `items += [1]`.
     fn handle_aug_assign(
         &mut self,
-        range: &TextRange,
+        range: TextRange,
         target: &Box<Expr>,
         _op: &Operator,
         _value: &Box<Expr>,
     ) {
-        if self.name.eq(&_to_name_str(target)) {
-            self.add_mutation(range)
+        if ComparableExpr::from(self.name) == ComparableExpr::from(target) {
+            self.add_mutation(range);
         }
     }
 
-    fn handle_call(&mut self, _range: &TextRange, func: &Box<Expr>, _arguments: &Arguments) {
-        match func.as_ref() {
-            Expr::Attribute(ExprAttribute {
-                range,
-                value,
-                attr,
-                ctx: _,
-            }) => {
-                let name = _to_name_str(value);
-                if self.name.eq(&name) && is_mutating_function(&attr.as_str()) {
-                    self.add_mutation(range);
+    /// Handle, e.g., `items.append(1)`.
+    fn handle_call(&mut self, func: &Expr) {
+        if let Expr::Attribute(ExprAttribute {
+            range,
+            value,
+            attr,
+            ctx: _,
+        }) = func
+        {
+            if is_mutating_function(attr.as_str()) {
+                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
+                    self.add_mutation(*range);
                 }
             }
-            _ => {}
         }
     }
 
-    fn handle_if(
-        &mut self,
-        _range: &TextRange,
-        _test: &Box<Expr>,
-        body: &'a Vec<Stmt>,
-        _elif_else_clauses: &Vec<ElifElseClause>,
-    ) {
+    fn handle_branches(&mut self, body: &'a [Stmt], elif_else_clauses: &'a [ElifElseClause]) {
         self.branch += 1;
+        self.branches.push(self.branch);
         self.visit_body(body);
-        self.branch += 1;
+        self.branches.pop();
+
+        for clause in elif_else_clauses {
+            self.branch += 1;
+            self.branches.push(self.branch);
+            self.visit_body(&clause.body);
+            self.branches.pop();
+        }
     }
 }
 
@@ -240,30 +220,50 @@ impl<'a> LoopMutationsVisitor<'a> {
 impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
     fn visit_stmt(&mut self, stmt: &'a Stmt) {
         match stmt {
-            Stmt::Delete(StmtDelete { range, targets }) => self.handle_delete(range, targets),
+            // Ex) `del items[0]`
+            Stmt::Delete(StmtDelete { range, targets }) => {
+                self.handle_delete(*range, targets);
+                visitor::walk_stmt(self, stmt);
+            }
+
+            // Ex) `items[0] = 1`
             Stmt::Assign(StmtAssign {
                 range,
                 targets,
                 value,
-            }) => self.handle_assign(range, targets, value),
+            }) => {
+                self.handle_assign(*range, targets, value);
+                visitor::walk_stmt(self, stmt);
+            }
+
+            // Ex) `items += [1]`
             Stmt::AugAssign(StmtAugAssign {
                 range,
                 target,
                 op,
                 value,
             }) => {
-                self.handle_aug_assign(range, target, op, value);
+                self.handle_aug_assign(*range, target, op, value);
+                visitor::walk_stmt(self, stmt);
             }
+
+            // Ex) `if True: items.append(1)`
             Stmt::If(StmtIf {
-                range,
-                test,
                 body,
                 elif_else_clauses,
-            }) => self.handle_if(range, test, body, elif_else_clauses),
+                ..
+            }) => self.handle_branches(body, elif_else_clauses),
+
+            // On break, clear the mutations for the current branch.
             Stmt::Break(StmtBreak { range: _ }) => match self.mutations.get_mut(&self.branch) {
                 Some(a) => a.clear(),
                 None => {}
             },
+
+            // Avoid recursion for class and function definitions.
+            Stmt::ClassDef(_) | Stmt::FunctionDef(_) => {}
+
+            // Default case.
             _ => {
                 visitor::walk_stmt(self, stmt);
             }
@@ -271,14 +271,11 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
     }
 
     fn visit_expr(&mut self, expr: &'a Expr) {
-        match expr {
-            Expr::Call(ExprCall {
-                range,
-                func,
-                arguments,
-            }) => self.handle_call(range, func, arguments),
-            _ => {}
+        // Ex) `items.append(1)`
+        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 98e85f8345c2f..d6cf21e4f18a3 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
@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
 ---
-B909.py:12:5: B909 Mutation to loop iterable during iteration
+B909.py:12:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 10 | for elem in some_list:
 11 |     # errors
@@ -11,7 +11,7 @@ B909.py:12:5: B909 Mutation to loop iterable during iteration
 14 |     some_list.append(elem)
    |
 
-B909.py:13:5: B909 Mutation to loop iterable during iteration
+B909.py:13:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 11 |     # errors
 12 |     some_list.remove(elem)
@@ -21,7 +21,7 @@ B909.py:13:5: B909 Mutation to loop iterable during iteration
 15 |     some_list.sort()
    |
 
-B909.py:14:5: B909 Mutation to loop iterable during iteration
+B909.py:14:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 12 |     some_list.remove(elem)
 13 |     del some_list[2]
@@ -31,7 +31,7 @@ B909.py:14:5: B909 Mutation to loop iterable during iteration
 16 |     some_list.reverse()
    |
 
-B909.py:15:5: B909 Mutation to loop iterable during iteration
+B909.py:15:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 13 |     del some_list[2]
 14 |     some_list.append(elem)
@@ -41,7 +41,7 @@ B909.py:15:5: B909 Mutation to loop iterable during iteration
 17 |     some_list.clear()
    |
 
-B909.py:16:5: B909 Mutation to loop iterable during iteration
+B909.py:16:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 14 |     some_list.append(elem)
 15 |     some_list.sort()
@@ -51,7 +51,7 @@ B909.py:16:5: B909 Mutation to loop iterable during iteration
 18 |     some_list.extend([1, 2])
    |
 
-B909.py:17:5: B909 Mutation to loop iterable during iteration
+B909.py:17:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 15 |     some_list.sort()
 16 |     some_list.reverse()
@@ -61,7 +61,7 @@ B909.py:17:5: B909 Mutation to loop iterable during iteration
 19 |     some_list.insert(1, 1)
    |
 
-B909.py:18:5: B909 Mutation to loop iterable during iteration
+B909.py:18:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 16 |     some_list.reverse()
 17 |     some_list.clear()
@@ -71,7 +71,7 @@ B909.py:18:5: B909 Mutation to loop iterable during iteration
 20 |     some_list.pop(1)
    |
 
-B909.py:19:5: B909 Mutation to loop iterable during iteration
+B909.py:19:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 17 |     some_list.clear()
 18 |     some_list.extend([1, 2])
@@ -81,7 +81,7 @@ B909.py:19:5: B909 Mutation to loop iterable during iteration
 21 |     some_list.pop()
    |
 
-B909.py:20:5: B909 Mutation to loop iterable during iteration
+B909.py:20:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 18 |     some_list.extend([1, 2])
 19 |     some_list.insert(1, 1)
@@ -90,7 +90,7 @@ B909.py:20:5: B909 Mutation to loop iterable during iteration
 21 |     some_list.pop()
    |
 
-B909.py:21:5: B909 Mutation to loop iterable during iteration
+B909.py:21:5: B909 Mutation to loop iterable `some_list` during iteration
    |
 19 |     some_list.insert(1, 1)
 20 |     some_list.pop(1)
@@ -100,7 +100,7 @@ B909.py:21:5: B909 Mutation to loop iterable during iteration
 23 |     # conditional break should error
    |
 
-B909.py:25:9: B909 Mutation to loop iterable during iteration
+B909.py:25:9: B909 Mutation to loop iterable `some_list` during iteration
    |
 23 |     # conditional break should error
 24 |     if elem == 2:
@@ -110,7 +110,7 @@ B909.py:25:9: B909 Mutation to loop iterable during iteration
 27 |             break
    |
 
-B909.py:47:5: B909 Mutation to loop iterable during iteration
+B909.py:47:5: B909 Mutation to loop iterable `mydicts` during iteration
    |
 45 | for elem in mydicts:
 46 |     # errors
@@ -120,7 +120,7 @@ B909.py:47:5: B909 Mutation to loop iterable during iteration
 49 |     mydicts.update({"foo": "bar"})
    |
 
-B909.py:48:5: B909 Mutation to loop iterable during iteration
+B909.py:48:5: B909 Mutation to loop iterable `mydicts` during iteration
    |
 46 |     # errors
 47 |     mydicts.popitem()
@@ -129,7 +129,7 @@ B909.py:48:5: B909 Mutation to loop iterable during iteration
 49 |     mydicts.update({"foo": "bar"})
    |
 
-B909.py:49:5: B909 Mutation to loop iterable during iteration
+B909.py:49:5: B909 Mutation to loop iterable `mydicts` during iteration
    |
 47 |     mydicts.popitem()
 48 |     mydicts.setdefault("foo", 1)
@@ -139,7 +139,7 @@ B909.py:49:5: B909 Mutation to loop iterable during iteration
 51 |     # no errors
    |
 
-B909.py:62:5: B909 Mutation to loop iterable during iteration
+B909.py:62:5: B909 Mutation to loop iterable `myset` during iteration
    |
 60 | for _ in myset:
 61 |     # errors
@@ -149,7 +149,7 @@ B909.py:62:5: B909 Mutation to loop iterable during iteration
 64 |     myset.difference_update({4, 5})
    |
 
-B909.py:63:5: B909 Mutation to loop iterable during iteration
+B909.py:63:5: B909 Mutation to loop iterable `myset` during iteration
    |
 61 |     # errors
 62 |     myset.update({4, 5})
@@ -159,7 +159,7 @@ B909.py:63:5: B909 Mutation to loop iterable during iteration
 65 |     myset.symmetric_difference_update({4, 5})
    |
 
-B909.py:64:5: B909 Mutation to loop iterable during iteration
+B909.py:64:5: B909 Mutation to loop iterable `myset` during iteration
    |
 62 |     myset.update({4, 5})
 63 |     myset.intersection_update({4, 5})
@@ -169,7 +169,7 @@ B909.py:64:5: B909 Mutation to loop iterable during iteration
 66 |     myset.add(4)
    |
 
-B909.py:65:5: B909 Mutation to loop iterable during iteration
+B909.py:65:5: B909 Mutation to loop iterable `myset` during iteration
    |
 63 |     myset.intersection_update({4, 5})
 64 |     myset.difference_update({4, 5})
@@ -179,7 +179,7 @@ B909.py:65:5: B909 Mutation to loop iterable during iteration
 67 |     myset.discard(3)
    |
 
-B909.py:66:5: B909 Mutation to loop iterable during iteration
+B909.py:66:5: B909 Mutation to loop iterable `myset` during iteration
    |
 64 |     myset.difference_update({4, 5})
 65 |     myset.symmetric_difference_update({4, 5})
@@ -188,7 +188,7 @@ B909.py:66:5: B909 Mutation to loop iterable during iteration
 67 |     myset.discard(3)
    |
 
-B909.py:67:5: B909 Mutation to loop iterable during iteration
+B909.py:67:5: B909 Mutation to loop iterable `myset` during iteration
    |
 65 |     myset.symmetric_difference_update({4, 5})
 66 |     myset.add(4)
@@ -198,7 +198,7 @@ B909.py:67:5: B909 Mutation to loop iterable during iteration
 69 |     # no errors
    |
 
-B909.py:84:5: B909 Mutation to loop iterable during iteration
+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:
@@ -207,7 +207,7 @@ B909.py:84:5: B909 Mutation to loop iterable during iteration
 85 |     del a.some_list[2]
    |
 
-B909.py:85:5: B909 Mutation to loop iterable 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)
@@ -215,7 +215,7 @@ B909.py:85:5: B909 Mutation to loop iterable during iteration
    |     ^^^^^^^^^^^^^^^^^^ B909
    |
 
-B909.py:93:5: B909 Mutation to loop iterable during iteration
+B909.py:93:5: B909 Mutation to loop iterable `foo` during iteration
    |
 91 | bar = [4, 5, 6]
 92 | for _ in foo:
@@ -225,7 +225,7 @@ B909.py:93:5: B909 Mutation to loop iterable during iteration
 95 |     foo[1] = 9
    |
 
-B909.py:94:5: B909 Mutation to loop iterable during iteration
+B909.py:94:5: B909 Mutation to loop iterable `foo` during iteration
    |
 92 | for _ in foo:
 93 |     foo *= 2
@@ -235,7 +235,7 @@ B909.py:94:5: B909 Mutation to loop iterable during iteration
 96 |     foo[1:2] = bar
    |
 
-B909.py:95:5: B909 Mutation to loop iterable during iteration
+B909.py:95:5: B909 Mutation to loop iterable `foo` during iteration
    |
 93 |     foo *= 2
 94 |     foo += bar
@@ -245,7 +245,7 @@ B909.py:95:5: B909 Mutation to loop iterable during iteration
 97 |     foo[1:2:3] = bar
    |
 
-B909.py:96:5: B909 Mutation to loop iterable during iteration
+B909.py:96:5: B909 Mutation to loop iterable `foo` during iteration
    |
 94 |     foo += bar
 95 |     foo[1] = 9
@@ -254,7 +254,7 @@ B909.py:96:5: B909 Mutation to loop iterable during iteration
 97 |     foo[1:2:3] = bar
    |
 
-B909.py:97:5: B909 Mutation to loop iterable during iteration
+B909.py:97:5: B909 Mutation to loop iterable `foo` during iteration
    |
 95 |     foo[1] = 9
 96 |     foo[1:2] = bar
@@ -264,7 +264,7 @@ B909.py:97:5: B909 Mutation to loop iterable during iteration
 99 | foo = {1, 2, 3}
    |
 
-B909.py:102:5: B909 Mutation to loop iterable during iteration
+B909.py:102:5: B909 Mutation to loop iterable `foo` during iteration
     |
 100 | bar = {4, 5, 6}
 101 | for _ in foo:  # should error
@@ -274,7 +274,7 @@ B909.py:102:5: B909 Mutation to loop iterable during iteration
 104 |     foo -= bar
     |
 
-B909.py:103:5: B909 Mutation to loop iterable during iteration
+B909.py:103:5: B909 Mutation to loop iterable `foo` during iteration
     |
 101 | for _ in foo:  # should error
 102 |     foo |= bar
@@ -284,7 +284,7 @@ B909.py:103:5: B909 Mutation to loop iterable during iteration
 105 |     foo ^= bar
     |
 
-B909.py:104:5: B909 Mutation to loop iterable during iteration
+B909.py:104:5: B909 Mutation to loop iterable `foo` during iteration
     |
 102 |     foo |= bar
 103 |     foo &= bar
@@ -293,7 +293,7 @@ B909.py:104:5: B909 Mutation to loop iterable during iteration
 105 |     foo ^= bar
     |
 
-B909.py:105:5: B909 Mutation to loop iterable during iteration
+B909.py:105:5: B909 Mutation to loop iterable `foo` during iteration
     |
 103 |     foo &= bar
 104 |     foo -= bar
@@ -301,7 +301,7 @@ B909.py:105:5: B909 Mutation to loop iterable during iteration
     |     ^^^^^^^^^^ B909
     |
 
-B909.py:125:5: B909 Mutation to loop iterable during iteration
+B909.py:125:5: B909 Mutation to loop iterable `foo` during iteration
     |
 123 | # should error (?)
 124 | for _ in foo:
@@ -310,3 +310,11 @@ B909.py:125:5: B909 Mutation to loop iterable during iteration
 126 |     if bar:
 127 |         bar.remove(1)
     |
+
+B909.py:136:9: B909 Mutation to loop iterable `foo` during iteration
+    |
+134 |         pass
+135 |     else:
+136 |         foo.remove(1)
+    |         ^^^^^^^^^^ B909
+    |

From 8fe10406c5a80c76af2233de533be1847cac9e53 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 14:26:33 -0400
Subject: [PATCH 12/16] Tweak branches again

---
 .../test/fixtures/flake8_bugbear/B909.py      | 17 ++++
 .../rules/loop_iterator_mutation.rs           | 87 ++++++++-----------
 ...__flake8_bugbear__tests__B909_B909.py.snap | 21 +++++
 3 files changed, 76 insertions(+), 49 deletions(-)

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 4381aa0d5787a..7a1146d84fac0 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -134,3 +134,20 @@ def __init__(self, ls):
         pass
     else:
         foo.remove(1)
+
+# should error
+for elem in some_list:
+    if some_list.pop() == 2:
+        pass
+
+# should not error
+for elem in some_list:
+    if some_list.pop() == 2:
+        break
+
+# should error
+for elem in some_list:
+    if some_list.pop() == 2:
+        pass
+    else:
+        break
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 c2cf1de19bf5c..f8735a9498561 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},
-    ElifElseClause, Expr, ExprAttribute, ExprCall, ExprSubscript, Operator, Stmt, StmtAssign,
-    StmtAugAssign, StmtBreak, StmtDelete, StmtFor, StmtIf,
+    Expr, ExprAttribute, ExprCall, ExprSubscript, Stmt, StmtAssign, StmtAugAssign, StmtBreak,
+    StmtDelete, StmtFor, StmtIf,
 };
 use ruff_text_size::TextRange;
 
@@ -46,7 +46,7 @@ impl Violation for LoopIteratorMutation {
         let LoopIteratorMutation { name } = self;
 
         if let Some(name) = name.as_ref().and_then(SourceCodeSnippet::full_display) {
-            format!("Mutation to loop iterable `{}` during iteration", name)
+            format!("Mutation to loop iterable `{name}` during iteration")
         } else {
             format!("Mutation to loop iterable during iteration")
         }
@@ -131,14 +131,11 @@ impl<'a> LoopMutationsVisitor<'a> {
 
     /// Register a mutation.
     fn add_mutation(&mut self, range: TextRange) {
-        self.mutations
-            .entry(self.branch)
-            .or_insert(Vec::new())
-            .push(range);
+        self.mutations.entry(self.branch).or_default().push(range);
     }
 
     /// Handle, e.g., `del items[0]`.
-    fn handle_delete(&mut self, range: TextRange, targets: &'a Vec<Expr>) {
+    fn handle_delete(&mut self, range: TextRange, targets: &'a [Expr]) {
         for target in targets {
             if let Expr::Subscript(ExprSubscript {
                 range: _,
@@ -155,7 +152,7 @@ impl<'a> LoopMutationsVisitor<'a> {
     }
 
     /// Handle, e.g., `items[0] = 1`.
-    fn handle_assign(&mut self, range: TextRange, targets: &'a Vec<Expr>, _value: &Box<Expr>) {
+    fn handle_assign(&mut self, range: TextRange, targets: &[Expr]) {
         for target in targets {
             if let Expr::Subscript(ExprSubscript {
                 range: _,
@@ -172,13 +169,7 @@ impl<'a> LoopMutationsVisitor<'a> {
     }
 
     /// Handle, e.g., `items += [1]`.
-    fn handle_aug_assign(
-        &mut self,
-        range: TextRange,
-        target: &Box<Expr>,
-        _op: &Operator,
-        _value: &Box<Expr>,
-    ) {
+    fn handle_aug_assign(&mut self, range: TextRange, target: &Expr) {
         if ComparableExpr::from(self.name) == ComparableExpr::from(target) {
             self.add_mutation(range);
         }
@@ -200,20 +191,6 @@ impl<'a> LoopMutationsVisitor<'a> {
             }
         }
     }
-
-    fn handle_branches(&mut self, body: &'a [Stmt], elif_else_clauses: &'a [ElifElseClause]) {
-        self.branch += 1;
-        self.branches.push(self.branch);
-        self.visit_body(body);
-        self.branches.pop();
-
-        for clause in elif_else_clauses {
-            self.branch += 1;
-            self.branches.push(self.branch);
-            self.visit_body(&clause.body);
-            self.branches.pop();
-        }
-    }
 }
 
 /// `Visitor` to collect all used identifiers in a statement.
@@ -227,38 +204,50 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
             }
 
             // Ex) `items[0] = 1`
-            Stmt::Assign(StmtAssign {
-                range,
-                targets,
-                value,
-            }) => {
-                self.handle_assign(*range, targets, value);
+            Stmt::Assign(StmtAssign { range, targets, .. }) => {
+                self.handle_assign(*range, targets);
                 visitor::walk_stmt(self, stmt);
             }
 
             // Ex) `items += [1]`
-            Stmt::AugAssign(StmtAugAssign {
-                range,
-                target,
-                op,
-                value,
-            }) => {
-                self.handle_aug_assign(*range, target, op, value);
+            Stmt::AugAssign(StmtAugAssign { range, target, .. }) => {
+                self.handle_aug_assign(*range, target);
                 visitor::walk_stmt(self, stmt);
             }
 
             // Ex) `if True: items.append(1)`
             Stmt::If(StmtIf {
+                test,
                 body,
                 elif_else_clauses,
                 ..
-            }) => self.handle_branches(body, elif_else_clauses),
+            }) => {
+                // Handle the `if` branch.
+                self.branch += 1;
+                self.branches.push(self.branch);
+                self.visit_expr(test);
+                self.visit_body(body);
+                self.branches.pop();
+
+                // Handle the `elif` and `else` branches.
+                for clause in elif_else_clauses {
+                    self.branch += 1;
+                    self.branches.push(self.branch);
+                    if let Some(test) = &clause.test {
+                        self.visit_expr(test);
+                    }
+                    self.visit_body(&clause.body);
+                    self.branches.pop();
+                }
+            }
 
             // On break, clear the mutations for the current branch.
-            Stmt::Break(StmtBreak { range: _ }) => match self.mutations.get_mut(&self.branch) {
-                Some(a) => a.clear(),
-                None => {}
-            },
+            Stmt::Break(StmtBreak { range: _ }) => {
+                if let Some(mutations) = self.mutations.get_mut(&self.branch) {
+                    mutations.clear();
+                }
+                visitor::walk_stmt(self, stmt);
+            }
 
             // Avoid recursion for class and function definitions.
             Stmt::ClassDef(_) | Stmt::FunctionDef(_) => {}
@@ -273,7 +262,7 @@ 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);
+            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 d6cf21e4f18a3..2d03e647fe77e 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
@@ -317,4 +317,25 @@ B909.py:136:9: B909 Mutation to loop iterable `foo` during iteration
 135 |     else:
 136 |         foo.remove(1)
     |         ^^^^^^^^^^ B909
+137 | 
+138 | # should error
+    |
+
+B909.py:140:8: B909 Mutation to loop iterable `some_list` during iteration
+    |
+138 | # should error
+139 | for elem in some_list:
+140 |     if some_list.pop() == 2:
+    |        ^^^^^^^^^^^^^ B909
+141 |         pass
+    |
+
+B909.py:150:8: B909 Mutation to loop iterable `some_list` during iteration
+    |
+148 | # should error
+149 | for elem in some_list:
+150 |     if some_list.pop() == 2:
+    |        ^^^^^^^^^^^^^ B909
+151 |         pass
+152 |     else:
     |

From 5bdb18f50e895150d354a58f8925dcaa8f422bcf Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 14:28:41 -0400
Subject: [PATCH 13/16] Remove lifetime

---
 .../src/rules/flake8_bugbear/rules/loop_iterator_mutation.rs  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 f8735a9498561..d98d2c5b47aa9 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
@@ -24,7 +24,7 @@ use crate::fix::snippet::SourceCodeSnippet;
 ///
 /// ## Example
 /// ```python
-/// items = [1,2,3]
+/// items = [1, 2, 3]
 ///
 /// for item in items:
 ///     print(item)
@@ -135,7 +135,7 @@ impl<'a> LoopMutationsVisitor<'a> {
     }
 
     /// Handle, e.g., `del items[0]`.
-    fn handle_delete(&mut self, range: TextRange, targets: &'a [Expr]) {
+    fn handle_delete(&mut self, range: TextRange, targets: &[Expr]) {
         for target in targets {
             if let Expr::Subscript(ExprSubscript {
                 range: _,

From f5c370ecb89aedf6183681ab3ae20f3db3bcc193 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 14:47:15 -0400
Subject: [PATCH 14/16] Remove unused fixture

---
 ...__flake8_bugbear__tests__B038_B038.py.snap | 137 ------------------
 1 file changed, 137 deletions(-)
 delete mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
deleted file mode 100644
index d6a5f341c7d42..0000000000000
--- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B038_B038.py.snap
+++ /dev/null
@@ -1,137 +0,0 @@
----
-source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
----
-B038.py:5:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-  |
-3 |     print(elem)
-4 |     if elem % 2 == 0:
-5 |         some_list.remove(elem)  # should error
-  |         ^^^^^^^^^^^^^^^^ B038
-6 | 
-7 | some_list = [1, 2, 3]
-  |
-
-B038.py:20:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-18 |     print(elem)
-19 |     if elem % 2 == 0:
-20 |         del some_list[2]  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
-21 |         del some_list
-   |
-
-B038.py:21:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-19 |     if elem % 2 == 0:
-20 |         del some_list[2]  # should error
-21 |         del some_list
-   |         ^^^^^^^^^^^^^ B038
-   |
-
-B038.py:35:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-33 |     print(elem)
-34 |     if elem % 2 == 0:
-35 |         a.some_list.remove(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^^^ B038
-36 | 
-37 | a = A((1, 2, 3))
-   |
-
-B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-39 |     print(elem)
-40 |     if elem % 2 == 0:
-41 |         del a.some_list[2]  # should error
-   |         ^^^^^^^^^^^^^^^^^^ B038
-   |
-
-B038.py:49:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-47 |     if elem == 2:
-48 |         found_idx = some_list.index(elem)  # should not error
-49 |         some_list.append(elem)  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
-50 |         some_list.sort()  # should error
-51 |         some_list.reverse()  # should error
-   |
-
-B038.py:50:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-48 |         found_idx = some_list.index(elem)  # should not error
-49 |         some_list.append(elem)  # should error
-50 |         some_list.sort()  # should error
-   |         ^^^^^^^^^^^^^^ B038
-51 |         some_list.reverse()  # should error
-52 |         some_list.clear()  # should error
-   |
-
-B038.py:51:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-49 |         some_list.append(elem)  # should error
-50 |         some_list.sort()  # should error
-51 |         some_list.reverse()  # should error
-   |         ^^^^^^^^^^^^^^^^^ B038
-52 |         some_list.clear()  # should error
-53 |         some_list.extend([1, 2])  # should error
-   |
-
-B038.py:52:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-50 |         some_list.sort()  # should error
-51 |         some_list.reverse()  # should error
-52 |         some_list.clear()  # should error
-   |         ^^^^^^^^^^^^^^^ B038
-53 |         some_list.extend([1, 2])  # should error
-54 |         some_list.insert(1, 1)  # should error
-   |
-
-B038.py:53:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-51 |         some_list.reverse()  # should error
-52 |         some_list.clear()  # should error
-53 |         some_list.extend([1, 2])  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
-54 |         some_list.insert(1, 1)  # should error
-55 |         some_list.pop(1)  # should error
-   |
-
-B038.py:54:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-52 |         some_list.clear()  # should error
-53 |         some_list.extend([1, 2])  # should error
-54 |         some_list.insert(1, 1)  # should error
-   |         ^^^^^^^^^^^^^^^^ B038
-55 |         some_list.pop(1)  # should error
-56 |         some_list.pop()  # should error
-   |
-
-B038.py:55:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-53 |         some_list.extend([1, 2])  # should error
-54 |         some_list.insert(1, 1)  # should error
-55 |         some_list.pop(1)  # should error
-   |         ^^^^^^^^^^^^^ B038
-56 |         some_list.pop()  # should error
-57 |         some_list = 3  # should error
-   |
-
-B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-54 |         some_list.insert(1, 1)  # should error
-55 |         some_list.pop(1)  # should error
-56 |         some_list.pop()  # should error
-   |         ^^^^^^^^^^^^^ B038
-57 |         some_list = 3  # should error
-58 |         break
-   |
-
-B038.py:66:9: B038 editing a loop's mutable iterable often leads to unexpected results/bugs
-   |
-64 |     if mydicts.get("a", ""):
-65 |         print(mydict["foo"])  # should not error
-66 |         mydicts.popitem()  # should error
-   |         ^^^^^^^^^^^^^^^ B038
-   |
-
-

From 42d4b6909852b9e5dc36290f7b32bc6bb791b16d Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 15:22:46 -0400
Subject: [PATCH 15/16] Allow mutations with iterator key

---
 .../test/fixtures/flake8_bugbear/B909.py      |  5 +++
 .../rules/loop_iterator_mutation.rs           | 36 ++++++++++++-------
 2 files changed, 28 insertions(+), 13 deletions(-)

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 7a1146d84fac0..cce53638372d2 100644
--- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
+++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -151,3 +151,8 @@ def __init__(self, ls):
         pass
     else:
         break
+
+# should not error
+for elem in some_list:
+    del some_list[elem]
+    some_list[elem] = 1
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 d98d2c5b47aa9..f6def259bc039 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
@@ -56,7 +56,7 @@ impl Violation for LoopIteratorMutation {
 /// B909
 pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor) {
     let StmtFor {
-        target: _,
+        target,
         iter,
         body,
         orelse: _,
@@ -70,7 +70,7 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
 
     // Collect mutations to the iterable.
     let mutations = {
-        let mut visitor = LoopMutationsVisitor::new(iter);
+        let mut visitor = LoopMutationsVisitor::new(iter, target);
         visitor.visit_body(body);
         visitor.mutations
     };
@@ -112,7 +112,8 @@ fn is_mutating_function(function_name: &str) -> bool {
 /// A visitor to collect mutations to a variable in a loop.
 #[derive(Debug, Clone)]
 struct LoopMutationsVisitor<'a> {
-    name: &'a Expr,
+    iter: &'a Expr,
+    target: &'a Expr,
     mutations: HashMap<u8, Vec<TextRange>>,
     branches: Vec<u8>,
     branch: u8,
@@ -120,9 +121,10 @@ struct LoopMutationsVisitor<'a> {
 
 impl<'a> LoopMutationsVisitor<'a> {
     /// Initialize the visitor.
-    fn new(name: &'a Expr) -> Self {
+    fn new(iter: &'a Expr, target: &'a Expr) -> Self {
         Self {
-            name,
+            iter,
+            target,
             mutations: HashMap::new(),
             branches: vec![0],
             branch: 0,
@@ -140,12 +142,16 @@ impl<'a> LoopMutationsVisitor<'a> {
             if let Expr::Subscript(ExprSubscript {
                 range: _,
                 value,
-                slice: _,
+                slice,
                 ctx: _,
             }) = target
             {
-                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
-                    self.add_mutation(range);
+                // 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.target) != ComparableExpr::from(slice) {
+                        self.add_mutation(range);
+                    }
                 }
             }
         }
@@ -157,12 +163,16 @@ impl<'a> LoopMutationsVisitor<'a> {
             if let Expr::Subscript(ExprSubscript {
                 range: _,
                 value,
-                slice: _,
+                slice,
                 ctx: _,
             }) = target
             {
-                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
-                    self.add_mutation(range);
+                // Find, e.g., `items[0] = 1`.
+                if ComparableExpr::from(self.iter) == ComparableExpr::from(value) {
+                    // But allow, e.g., `for item in items: items[item] = 1`.
+                    if ComparableExpr::from(self.target) != ComparableExpr::from(slice) {
+                        self.add_mutation(range);
+                    }
                 }
             }
         }
@@ -170,7 +180,7 @@ impl<'a> LoopMutationsVisitor<'a> {
 
     /// Handle, e.g., `items += [1]`.
     fn handle_aug_assign(&mut self, range: TextRange, target: &Expr) {
-        if ComparableExpr::from(self.name) == ComparableExpr::from(target) {
+        if ComparableExpr::from(self.iter) == ComparableExpr::from(target) {
             self.add_mutation(range);
         }
     }
@@ -185,7 +195,7 @@ impl<'a> LoopMutationsVisitor<'a> {
         }) = func
         {
             if is_mutating_function(attr.as_str()) {
-                if ComparableExpr::from(self.name) == ComparableExpr::from(value) {
+                if ComparableExpr::from(self.iter) == ComparableExpr::from(value) {
                     self.add_mutation(*range);
                 }
             }

From e0a1a4f59bf65bb71fe9c45b5f31581529ec9e2c Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 15:30:04 -0400
Subject: [PATCH 16/16] Allow mutations with iterator key

---
 .../test/fixtures/flake8_bugbear/B909.py      |  8 +++---
 .../rules/loop_iterator_mutation.rs           | 25 +++++++++++++++----
 ...__flake8_bugbear__tests__B909_B909.py.snap | 12 ++++-----
 3 files changed, 31 insertions(+), 14 deletions(-)

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 cce53638372d2..68afaf87fb257 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 f6def259bc039..3c5b31ce7ae98 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,19 @@ 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 matches!(attr.as_str(), "remove" | "discard" | "pop") {
+                        if arguments.len() == 1 {
+                            if let [arg] = &*arguments.args {
+                                if ComparableExpr::from(self.target) == ComparableExpr::from(arg) {
+                                    return;
+                                }
+                            }
+                        }
+                    }
+
                     self.add_mutation(*range);
                 }
             }
@@ -271,8 +283,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 2d03e647fe77e..7f70841c6c066 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
    |