Implement B020 (#753)

README.md

@@ -531,6 +531,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/
 | B017 | NoAssertRaisesException | `assertRaises(Exception)` should be considered evil |  |
 | B018 | UselessExpression | Found useless expression. Either assign it to a variable or remove it. |  |
 | B019 | CachedInstanceMethod | Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks |  |
+| B020 | LoopVariableOverridesIterator | Loop control variable `...` overrides iterable it iterates |  |
 | B021 | FStringDocstring | f-string used as docstring. This will be interpreted by python as a joined string rather than a docstring. |  |
 | B022 | UselessContextlibSuppress | No arguments passed to `contextlib.suppress`. No exceptions will be suppressed and therefore this context manager is redundant |  |
 | B024 | AbstractBaseClassWithoutAbstractMethod | `...` is an abstract base class, but it has no abstract methods |  |
@@ -734,7 +735,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
 - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
 - [`flake8-bandit`](https://pypi.org/project/flake8-bandit/) (6/40)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
-- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (25/32)
+- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (26/32)
 - [`flake8-2020`](https://pypi.org/project/flake8-2020/)
 
 Ruff can also replace [`isort`](https://pypi.org/project/isort/), [`yesqa`](https://github.com/asottile/yesqa),

resources/test/fixtures/B020.py

@@ -0,0 +1,40 @@
+"""
+Should emit:
+B020 - on lines 8, 21, and 36
+"""
+
+items = [1, 2, 3]
+
+for items in items:
+    print(items)
+
+items = [1, 2, 3]
+
+for item in items:
+    print(item)
+
+values = {"secret": 123}
+
+for key, value in values.items():
+    print(f"{key}, {value}")
+
+for key, values in values.items():
+    print(f"{key}, {values}")
+
+# Variables defined in a comprehension are local in scope
+# to that comprehension and are therefore allowed.
+for var in [var for var in range(10)]:
+    print(var)
+
+for var in (var for var in range(10)):
+    print(var)
+
+for k, v in {k: v for k, v in zip(range(10), range(10, 20))}.items():
+    print(k, v)
+
+# However we still call out reassigning the iterable in the comprehension.
+for vars in [i for i in vars]:
+    print(vars)
+
+for var in sorted(range(10), key=lambda var: var.real):
+    print(var)

src/check_ast.rs

@@ -873,10 +873,15 @@ where
                     flake8_bugbear::plugins::assert_raises_exception(self, stmt, items);
                 }
             }
-            StmtKind::For { target, body, .. } => {
+            StmtKind::For {
+                target, body, iter, ..
+            } => {
                 if self.settings.enabled.contains(&CheckCode::B007) {
                     flake8_bugbear::plugins::unused_loop_control_variable(self, target, body);
                 }
+                if self.settings.enabled.contains(&CheckCode::B020) {
+                    flake8_bugbear::plugins::loop_variable_overrides_iterator(self, target, iter);
+                }
             }
             StmtKind::Try { handlers, .. } => {
                 if self.settings.enabled.contains(&CheckCode::F707) {

src/checks.rs

@@ -95,6 +95,7 @@ pub enum CheckCode {
     B017,
     B018,
     B019,
+    B020,
     B021,
     B022,
     B024,
@@ -399,6 +400,7 @@ pub enum CheckKind {
     NoAssertRaisesException,
     UselessExpression,
     CachedInstanceMethod,
+    LoopVariableOverridesIterator(String),
     FStringDocstring,
     UselessContextlibSuppress,
     AbstractBaseClassWithoutAbstractMethod(String),
@@ -645,6 +647,7 @@ impl CheckCode {
             CheckCode::B017 => CheckKind::NoAssertRaisesException,
             CheckCode::B018 => CheckKind::UselessExpression,
             CheckCode::B019 => CheckKind::CachedInstanceMethod,
+            CheckCode::B020 => CheckKind::LoopVariableOverridesIterator("...".to_string()),
             CheckCode::B021 => CheckKind::FStringDocstring,
             CheckCode::B022 => CheckKind::UselessContextlibSuppress,
             CheckCode::B024 => CheckKind::AbstractBaseClassWithoutAbstractMethod("...".to_string()),
@@ -890,6 +893,7 @@ impl CheckCode {
             CheckCode::B017 => CheckCategory::Flake8Bugbear,
             CheckCode::B018 => CheckCategory::Flake8Bugbear,
             CheckCode::B019 => CheckCategory::Flake8Bugbear,
+            CheckCode::B020 => CheckCategory::Flake8Bugbear,
             CheckCode::B021 => CheckCategory::Flake8Bugbear,
             CheckCode::B022 => CheckCategory::Flake8Bugbear,
             CheckCode::B024 => CheckCategory::Flake8Bugbear,
@@ -1098,6 +1102,7 @@ impl CheckKind {
             CheckKind::NoAssertRaisesException => &CheckCode::B017,
             CheckKind::UselessExpression => &CheckCode::B018,
             CheckKind::CachedInstanceMethod => &CheckCode::B019,
+            CheckKind::LoopVariableOverridesIterator(_) => &CheckCode::B020,
             CheckKind::FStringDocstring => &CheckCode::B021,
             CheckKind::UselessContextlibSuppress => &CheckCode::B022,
             CheckKind::AbstractBaseClassWithoutAbstractMethod(_) => &CheckCode::B024,
@@ -1472,6 +1477,9 @@ impl CheckKind {
             CheckKind::CachedInstanceMethod => "Use of `functools.lru_cache` or `functools.cache` \
                                                 on methods can lead to memory leaks"
                 .to_string(),
+            CheckKind::LoopVariableOverridesIterator(name) => {
+                format!("Loop control variable `{name}` overrides iterable it iterates")
+            }
             CheckKind::FStringDocstring => "f-string used as docstring. This will be interpreted \
                                             by python as a joined string rather than a docstring."
                 .to_string(),

src/checks_gen.rs

@@ -56,6 +56,7 @@ pub enum CheckCodePrefix {
     B018,
     B019,
     B02,
+    B020,
     B021,
     B022,
     B024,
@@ -386,6 +387,7 @@ impl CheckCodePrefix {
                 CheckCode::B017,
                 CheckCode::B018,
                 CheckCode::B019,
+                CheckCode::B020,
                 CheckCode::B021,
                 CheckCode::B022,
                 CheckCode::B024,
@@ -412,6 +414,7 @@ impl CheckCodePrefix {
                 CheckCode::B017,
                 CheckCode::B018,
                 CheckCode::B019,
+                CheckCode::B020,
                 CheckCode::B021,
                 CheckCode::B022,
                 CheckCode::B024,
@@ -460,6 +463,7 @@ impl CheckCodePrefix {
             CheckCodePrefix::B018 => vec![CheckCode::B018],
             CheckCodePrefix::B019 => vec![CheckCode::B019],
             CheckCodePrefix::B02 => vec![
+                CheckCode::B020,
                 CheckCode::B021,
                 CheckCode::B022,
                 CheckCode::B024,
@@ -467,6 +471,7 @@ impl CheckCodePrefix {
                 CheckCode::B026,
                 CheckCode::B027,
             ],
+            CheckCodePrefix::B020 => vec![CheckCode::B020],
             CheckCodePrefix::B021 => vec![CheckCode::B021],
             CheckCodePrefix::B022 => vec![CheckCode::B022],
             CheckCodePrefix::B024 => vec![CheckCode::B024],
@@ -1213,6 +1218,7 @@ impl CheckCodePrefix {
             CheckCodePrefix::B018 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B019 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B02 => PrefixSpecificity::Tens,
+            CheckCodePrefix::B020 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B021 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B022 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B024 => PrefixSpecificity::Explicit,

src/flake8_bugbear/plugins/loop_variable_overrides_iterator.rs

@@ -0,0 +1,64 @@
+use fnv::FnvHashMap;
+use rustpython_ast::{Expr, ExprKind};
+
+use crate::ast::types::Range;
+use crate::ast::visitor;
+use crate::ast::visitor::Visitor;
+use crate::check_ast::Checker;
+use crate::checks::{Check, CheckKind};
+
+#[derive(Default)]
+struct NameFinder<'a> {
+    names: FnvHashMap<&'a str, &'a Expr>,
+}
+
+impl<'a, 'b> Visitor<'b> for NameFinder<'a>
+where
+    'b: 'a,
+{
+    fn visit_expr(&mut self, expr: &'b Expr) {
+        match &expr.node {
+            ExprKind::Name { id, .. } => {
+                self.names.insert(id, expr);
+            }
+            ExprKind::ListComp { generators, .. }
+            | ExprKind::DictComp { generators, .. }
+            | ExprKind::SetComp { generators, .. }
+            | ExprKind::GeneratorExp { generators, .. } => {
+                for comp in generators {
+                    self.visit_expr(&comp.iter);
+                }
+            }
+            ExprKind::Lambda { args, body } => {
+                visitor::walk_expr(self, body);
+                for arg in args.args.iter() {
+                    self.names.remove(arg.node.arg.as_str());
+                }
+            }
+            _ => visitor::walk_expr(self, expr),
+        }
+    }
+}
+
+/// B020
+pub fn loop_variable_overrides_iterator(checker: &mut Checker, target: &Expr, iter: &Expr) {
+    let target_names = {
+        let mut target_finder = NameFinder::default();
+        target_finder.visit_expr(target);
+        target_finder.names
+    };
+    let iter_names = {
+        let mut iter_finder = NameFinder::default();
+        iter_finder.visit_expr(iter);
+        iter_finder.names
+    };
+
+    for (name, expr) in target_names {
+        if iter_names.contains_key(name) {
+            checker.add_check(Check::new(
+                CheckKind::LoopVariableOverridesIterator(name.to_string()),
+                Range::from_located(expr),
+            ));
+        }
+    }
+}

src/flake8_bugbear/plugins/mod.rs

@@ -9,6 +9,7 @@ pub use f_string_docstring::f_string_docstring;
 pub use function_call_argument_default::function_call_argument_default;
 pub use getattr_with_constant::getattr_with_constant;
 pub use jump_statement_in_finally::jump_statement_in_finally;
+pub use loop_variable_overrides_iterator::loop_variable_overrides_iterator;
 pub use mutable_argument_default::mutable_argument_default;
 pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler;
 pub use setattr_with_constant::setattr_with_constant;
@@ -32,6 +33,7 @@ mod f_string_docstring;
 mod function_call_argument_default;
 mod getattr_with_constant;
 mod jump_statement_in_finally;
+mod loop_variable_overrides_iterator;
 mod mutable_argument_default;
 mod redundant_tuple_in_exception_handler;
 mod setattr_with_constant;

src/linter.rs

@@ -344,6 +344,7 @@ mod tests {
     #[test_case(CheckCode::B017, Path::new("B017.py"); "B017")]
     #[test_case(CheckCode::B018, Path::new("B018.py"); "B018")]
     #[test_case(CheckCode::B019, Path::new("B019.py"); "B019")]
+    #[test_case(CheckCode::B020, Path::new("B020.py"); "B020")]
     #[test_case(CheckCode::B021, Path::new("B021.py"); "B021")]
     #[test_case(CheckCode::B022, Path::new("B022.py"); "B022")]
     #[test_case(CheckCode::B024, Path::new("B024.py"); "B024")]

src/snapshots/ruff__linter__tests__B020_B020.py.snap

@@ -0,0 +1,32 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    LoopVariableOverridesIterator: items
+  location:
+    row: 8
+    column: 4
+  end_location:
+    row: 8
+    column: 9
+  fix: ~
+- kind:
+    LoopVariableOverridesIterator: values
+  location:
+    row: 21
+    column: 9
+  end_location:
+    row: 21
+    column: 15
+  fix: ~
+- kind:
+    LoopVariableOverridesIterator: vars
+  location:
+    row: 36
+    column: 4
+  end_location:
+    row: 36
+    column: 8
+  fix: ~
+