Implement B007 (unused loop control variable) (#473)

README.md

@@ -381,6 +381,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
 | Code | Name | Message | Fix |
 | ---- | ---- | ------- | --- |
 | B002 | UnaryPrefixIncrement | Python does not support the unary prefix increment. |  |
+| B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 |
 | B011 | DoNotAssertFalse | Do not `assert False` (`python -O` removes these calls), raise `AssertionError()` | 🛠 |
 | B014 | DuplicateHandlerException | Exception handler with duplicate exception: `ValueError` | 🛠 |
 | B017 | NoAssertRaisesException | `assertRaises(Exception):` should be considered evil. |  |
@@ -473,7 +474,7 @@ including:
 - [`flake8-super`](https://pypi.org/project/flake8-super/)
 - [`flake8-print`](https://pypi.org/project/flake8-print/)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
-- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (8/32)
+- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (9/32)
 - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
 - [`autoflake`](https://pypi.org/project/autoflake/) (1/7)
 
@@ -493,7 +494,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
 - [`flake8-super`](https://pypi.org/project/flake8-super/)
 - [`flake8-print`](https://pypi.org/project/flake8-print/)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
-- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (8/32)
+- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (9/32)
 
 Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa),
 and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34).

resources/test/fixtures/B007.py

@@ -0,0 +1,31 @@
+for i in range(10):
+    print(i)
+
+print(i)  # name no longer defined on Python 3; no warning yet
+
+for i in range(10):  # name not used within the loop; B007
+    print(10)
+
+print(i)  # name no longer defined on Python 3; no warning yet
+
+
+for _ in range(10):  # _ is okay for a throw-away variable
+    print(10)
+
+
+for i in range(10):
+    for j in range(10):
+        for k in range(10):  # k not used, i and j used transitively
+            print(i + j)
+
+
+def strange_generator():
+    for i in range(10):
+        for j in range(10):
+            for k in range(10):
+                for l in range(10):
+                    yield i, (j, (k, l))
+
+
+for i, (j, (k, l)) in strange_generator():  # i, k not used
+    print(j, l)

src/check_ast.rs

@@ -687,6 +687,11 @@ where
                     flake8_bugbear::plugins::assert_raises_exception(self, stmt, items);
                 }
             }
+            StmtKind::For { target, body, .. } => {
+                if self.settings.enabled.contains(&CheckCode::B007) {
+                    flake8_bugbear::plugins::unused_loop_control_variable(self, target, body);
+                }
+            }
             StmtKind::Try { handlers, .. } => {
                 if self.settings.enabled.contains(&CheckCode::F707) {
                     if let Some(check) = pyflakes::checks::default_except_not_last(handlers) {

src/checks.rs

@@ -76,6 +76,7 @@ pub enum CheckCode {
     A003,
     // flake8-bugbear
     B002,
+    B007,
     B011,
     B014,
     B017,
@@ -268,6 +269,7 @@ pub enum CheckKind {
     BuiltinAttributeShadowing(String),
     // flake8-bugbear
     UnaryPrefixIncrement,
+    UnusedLoopControlVariable(String),
     DoNotAssertFalse,
     DuplicateHandlerException(Vec<String>),
     NoAssertRaisesException,
@@ -429,6 +431,7 @@ impl CheckCode {
             CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
             // flake8-bugbear
             CheckCode::B002 => CheckKind::UnaryPrefixIncrement,
+            CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()),
             CheckCode::B011 => CheckKind::DoNotAssertFalse,
             CheckCode::B014 => CheckKind::DuplicateHandlerException(vec!["ValueError".to_string()]),
             CheckCode::B017 => CheckKind::NoAssertRaisesException,
@@ -605,6 +608,7 @@ impl CheckCode {
             CheckCode::A002 => CheckCategory::Flake8Builtins,
             CheckCode::A003 => CheckCategory::Flake8Builtins,
             CheckCode::B002 => CheckCategory::Flake8Bugbear,
+            CheckCode::B007 => CheckCategory::Flake8Bugbear,
             CheckCode::B011 => CheckCategory::Flake8Bugbear,
             CheckCode::B014 => CheckCategory::Flake8Bugbear,
             CheckCode::B017 => CheckCategory::Flake8Bugbear,
@@ -750,6 +754,7 @@ impl CheckKind {
             CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
             // flake8-bugbear
             CheckKind::UnaryPrefixIncrement => &CheckCode::B002,
+            CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007,
             CheckKind::DoNotAssertFalse => &CheckCode::B011,
             CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
             CheckKind::NoAssertRaisesException => &CheckCode::B017,
@@ -990,6 +995,7 @@ impl CheckKind {
             }
             // flake8-bugbear
             CheckKind::UnaryPrefixIncrement => "Python does not support the unary prefix increment. Writing `++n` is equivalent to `+(+(n))`, which equals `n`. You meant `n += 1`.".to_string(),
+            CheckKind::UnusedLoopControlVariable(name) => format!("Loop control variable `{name}` not used within the loop body. If this is intended, start the name with an underscore."),
             CheckKind::DoNotAssertFalse => {
                 "Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`"
                     .to_string()
@@ -1278,6 +1284,9 @@ impl CheckKind {
             CheckKind::UnaryPrefixIncrement => {
                 "Python does not support the unary prefix increment.".to_string()
             }
+            CheckKind::UnusedLoopControlVariable(name) => {
+                format!("Loop control variable `{name}` not used within the loop body.")
+            }
             CheckKind::NoAssertRaisesException => {
                 "`assertRaises(Exception):` should be considered evil.".to_string()
             }
@@ -1320,6 +1329,7 @@ impl CheckKind {
                 | CheckKind::TypeOfPrimitive(_)
                 | CheckKind::UnnecessaryAbspath
                 | CheckKind::UnusedImport(_)
+                | CheckKind::UnusedLoopControlVariable(_)
                 | CheckKind::UnusedNOQA(_)
                 | CheckKind::UsePEP585Annotation(_)
                 | CheckKind::UsePEP604Annotation

src/flake8_bugbear/plugins/mod.rs

@@ -3,8 +3,10 @@ pub use assert_raises_exception::assert_raises_exception;
 pub use duplicate_exceptions::duplicate_exceptions;
 pub use duplicate_exceptions::duplicate_handler_exceptions;
 pub use unary_prefix_increment::unary_prefix_increment;
+pub use unused_loop_control_variable::unused_loop_control_variable;
 
 mod assert_false;
 mod assert_raises_exception;
 mod duplicate_exceptions;
 mod unary_prefix_increment;
+mod unused_loop_control_variable;

src/flake8_bugbear/plugins/unused_loop_control_variable.rs

@@ -0,0 +1,79 @@
+use std::collections::BTreeMap;
+
+use rustpython_ast::{Expr, ExprKind, Stmt};
+
+use crate::ast::types::Range;
+use crate::ast::visitor;
+use crate::ast::visitor::Visitor;
+use crate::autofix::Fix;
+use crate::check_ast::Checker;
+use crate::checks::{Check, CheckKind};
+
+/// Identify all `ExprKind::Name` nodes in an AST.
+struct NameFinder<'a> {
+    /// A map from identifier to defining expression.
+    names: BTreeMap<&'a str, &'a Expr>,
+}
+
+impl NameFinder<'_> {
+    fn new() -> Self {
+        NameFinder {
+            names: Default::default(),
+        }
+    }
+}
+
+impl<'a, 'b> Visitor<'b> for NameFinder<'a>
+where
+    'b: 'a,
+{
+    fn visit_expr(&mut self, expr: &'a Expr) {
+        if let ExprKind::Name { id, .. } = &expr.node {
+            self.names.insert(id, expr);
+        }
+        visitor::walk_expr(self, expr);
+    }
+}
+
+/// B007
+pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body: &[Stmt]) {
+    let control_names = {
+        let mut finder = NameFinder::new();
+        finder.visit_expr(target);
+        finder.names
+    };
+
+    let used_names = {
+        let mut finder = NameFinder::new();
+        for stmt in body {
+            finder.visit_stmt(stmt);
+        }
+        finder.names
+    };
+
+    for (name, expr) in control_names {
+        // Ignore names that are already underscore-prefixed.
+        if name.starts_with('_') {
+            continue;
+        }
+
+        // Ignore any names that are actually used in the loop body.
+        if used_names.contains_key(name) {
+            continue;
+        }
+
+        let mut check = Check::new(
+            CheckKind::UnusedLoopControlVariable(name.to_string()),
+            Range::from_located(expr),
+        );
+        if checker.patch() {
+            // Prefix the variable name with an underscore.
+            check.amend(Fix::replacement(
+                format!("_{name}"),
+                expr.location,
+                expr.end_location.unwrap(),
+            ))
+        }
+        checker.add_check(check);
+    }
+}

src/linter.rs

@@ -246,6 +246,7 @@ mod tests {
     #[test_case(CheckCode::A002, Path::new("A002.py"); "A002")]
     #[test_case(CheckCode::A003, Path::new("A003.py"); "A003")]
     #[test_case(CheckCode::B002, Path::new("B002.py"); "B002")]
+    #[test_case(CheckCode::B007, Path::new("B007.py"); "B007")]
     #[test_case(CheckCode::B011, Path::new("B011.py"); "B011")]
     #[test_case(CheckCode::B014, Path::new("B014.py"); "B014")]
     #[test_case(CheckCode::B017, Path::new("B017.py"); "B017")]

src/snapshots/ruff__linter__tests__B007_B007.py.snap

@@ -0,0 +1,77 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    UnusedLoopControlVariable: i
+  location:
+    row: 6
+    column: 5
+  end_location:
+    row: 6
+    column: 6
+  fix:
+    patch:
+      content: _i
+      location:
+        row: 6
+        column: 5
+      end_location:
+        row: 6
+        column: 6
+    applied: false
+- kind:
+    UnusedLoopControlVariable: k
+  location:
+    row: 18
+    column: 13
+  end_location:
+    row: 18
+    column: 14
+  fix:
+    patch:
+      content: _k
+      location:
+        row: 18
+        column: 13
+      end_location:
+        row: 18
+        column: 14
+    applied: false
+- kind:
+    UnusedLoopControlVariable: i
+  location:
+    row: 30
+    column: 5
+  end_location:
+    row: 30
+    column: 6
+  fix:
+    patch:
+      content: _i
+      location:
+        row: 30
+        column: 5
+      end_location:
+        row: 30
+        column: 6
+    applied: false
+- kind:
+    UnusedLoopControlVariable: k
+  location:
+    row: 30
+    column: 13
+  end_location:
+    row: 30
+    column: 14
+  fix:
+    patch:
+      content: _k
+      location:
+        row: 30
+        column: 13
+      end_location:
+        row: 30
+        column: 14
+    applied: false
+