fix: avoid flagging unused loop variable (B007) with globals(), vars() or eval() (#2166)

README.md

@@ -853,7 +853,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/) on PyPI
 | B004 | unreliable-callable-check |  Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use `callable(x)` for consistent results. |  |
 | B005 | strip-with-multi-characters | Using `.strip()` with multi-character strings is misleading the reader |  |
 | B006 | mutable-argument-default | Do not use mutable data structures for argument defaults |  |
-| B007 | unused-loop-control-variable | Loop control variable `{name}` not used within the loop body | 🛠 |
+| B007 | unused-loop-control-variable | Loop control variable `{name}` not used within loop body |  |
 | B008 | function-call-argument-default | Do not perform function call `{name}` in argument defaults |  |
 | B009 | get-attr-with-constant | Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. | 🛠 |
 | B010 | set-attr-with-constant | Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | 🛠 |

resources/test/fixtures/flake8_bugbear/B007.py

@@ -34,3 +34,14 @@ FMT = "{foo} {bar}"
 for foo, bar in [(1, 2)]:
     if foo:
         print(FMT.format(**locals()))
+
+for foo, bar in [(1, 2)]:
+    if foo:
+        print(FMT.format(**globals()))
+
+for foo, bar in [(1, 2)]:
+    if foo:
+        print(FMT.format(**vars()))
+
+for foo, bar in [(1, 2)]:
+    print(FMT.format(foo=foo, bar=eval('bar')))

src/ast/helpers.rs

@@ -4,8 +4,8 @@ use once_cell::sync::Lazy;
 use regex::Regex;
 use rustc_hash::{FxHashMap, FxHashSet};
 use rustpython_ast::{
-    Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Keyword,
-    KeywordData, Located, Location, Stmt, StmtKind,
+    Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Keyword, KeywordData,
+    Located, Location, Stmt, StmtKind,
 };
 use rustpython_parser::lexer;
 use rustpython_parser::lexer::Tok;
@@ -562,15 +562,17 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool {
     }
 }
 
-/// Return `true` if the body uses `locals()`.
-pub fn uses_locals(body: &[Stmt]) -> bool {
+/// Return `true` if the body uses `locals()`, `globals()`, `vars()`, `eval()`.
+pub fn uses_magic_variable_access(checker: &Checker, body: &[Stmt]) -> bool {
     any_over_body(body, &|expr| {
         if let ExprKind::Call { func, .. } = &expr.node {
-            if let ExprKind::Name { id, ctx } = &func.node {
-                id == "locals" && matches!(ctx, ExprContext::Load)
-            } else {
-                false
-            }
+            checker.resolve_call_path(func).map_or(false, |call_path| {
+                call_path.as_slice() == ["", "locals"]
+                    || call_path.as_slice() == ["", "globals"]
+                    || call_path.as_slice() == ["", "vars"]
+                    || call_path.as_slice() == ["", "eval"]
+                    || call_path.as_slice() == ["", "exec"]
+            })
         } else {
             false
         }

src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs

@@ -57,10 +57,6 @@ where
 
 /// B007
 pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body: &[Stmt]) {
-    if helpers::uses_locals(body) {
-        return;
-    }
-
     let control_names = {
         let mut finder = NameFinder::new();
         finder.visit_expr(target);
@@ -86,11 +82,15 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body:
             continue;
         }
 
+        let safe = !helpers::uses_magic_variable_access(checker, body);
         let mut diagnostic = Diagnostic::new(
-            violations::UnusedLoopControlVariable(name.to_string()),
+            violations::UnusedLoopControlVariable {
+                name: name.to_string(),
+                safe,
+            },
             Range::from_located(expr),
         );
-        if checker.patch(diagnostic.kind.rule()) {
+        if safe && checker.patch(diagnostic.kind.rule()) {
             // Prefix the variable name with an underscore.
             diagnostic.amend(Fix::replacement(
                 format!("_{name}"),

src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B007_B007.py.snap.new

@@ -0,0 +1,130 @@
+---
+source: src/rules/flake8_bugbear/mod.rs
+assertion_line: 52
+expression: diagnostics
+---
+- kind:
+    UnusedLoopControlVariable:
+      name: i
+      safe: true
+  location:
+    row: 6
+    column: 4
+  end_location:
+    row: 6
+    column: 5
+  fix:
+    content: _i
+    location:
+      row: 6
+      column: 4
+    end_location:
+      row: 6
+      column: 5
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: k
+      safe: true
+  location:
+    row: 18
+    column: 12
+  end_location:
+    row: 18
+    column: 13
+  fix:
+    content: _k
+    location:
+      row: 18
+      column: 12
+    end_location:
+      row: 18
+      column: 13
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: i
+      safe: true
+  location:
+    row: 30
+    column: 4
+  end_location:
+    row: 30
+    column: 5
+  fix:
+    content: _i
+    location:
+      row: 30
+      column: 4
+    end_location:
+      row: 30
+      column: 5
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: k
+      safe: true
+  location:
+    row: 30
+    column: 12
+  end_location:
+    row: 30
+    column: 13
+  fix:
+    content: _k
+    location:
+      row: 30
+      column: 12
+    end_location:
+      row: 30
+      column: 13
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: bar
+      safe: false
+  location:
+    row: 34
+    column: 9
+  end_location:
+    row: 34
+    column: 12
+  fix: ~
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: bar
+      safe: false
+  location:
+    row: 38
+    column: 9
+  end_location:
+    row: 38
+    column: 12
+  fix: ~
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: bar
+      safe: false
+  location:
+    row: 42
+    column: 9
+  end_location:
+    row: 42
+    column: 12
+  fix: ~
+  parent: ~
+- kind:
+    UnusedLoopControlVariable:
+      name: bar
+      safe: false
+  location:
+    row: 46
+    column: 9
+  end_location:
+    row: 46
+    column: 12
+  fix: ~
+  parent: ~
+

src/violations.rs

@@ -1181,18 +1181,35 @@ impl Violation for MutableArgumentDefault {
 }
 
 define_violation!(
-    pub struct UnusedLoopControlVariable(pub String);
+    pub struct UnusedLoopControlVariable {
+        /// The name of the loop control variable.
+        pub name: String,
+        /// Whether the variable is safe to rename. A variable is unsafe to
+        /// rename if it _might_ be used by magic control flow (e.g.,
+        /// `locals()`).
+        pub safe: bool,
+    }
 );
-impl AlwaysAutofixableViolation for UnusedLoopControlVariable {
+impl Violation for UnusedLoopControlVariable {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let UnusedLoopControlVariable(name) = self;
-        format!("Loop control variable `{name}` not used within the loop body")
+        let UnusedLoopControlVariable { name, safe } = self;
+        if *safe {
+            format!("Loop control variable `{name}` not used within loop body")
+        } else {
+            format!("Loop control variable `{name}` may not be used within loop body")
+        }
     }
 
-    fn autofix_title(&self) -> String {
-        let UnusedLoopControlVariable(name) = self;
+    fn autofix_title_formatter(&self) -> Option<fn(&Self) -> String> {
+        let UnusedLoopControlVariable { safe, .. } = self;
+        if *safe {
+            Some(|UnusedLoopControlVariable { name, .. }| {
                 format!("Rename unused `{name}` to `_{name}`")
+            })
+        } else {
+            None
+        }
     }
 }