diff --git a/README.md b/README.md index 55ca9b50e8..98ab0e9439 100644 --- a/README.md +++ b/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), diff --git a/resources/test/fixtures/B020.py b/resources/test/fixtures/B020.py new file mode 100644 index 0000000000..9bdc4b4d38 --- /dev/null +++ b/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) diff --git a/src/check_ast.rs b/src/check_ast.rs index d91da72137..6d0281a550 100644 --- a/src/check_ast.rs +++ b/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) { diff --git a/src/checks.rs b/src/checks.rs index c6639b5dd4..417bff168a 100644 --- a/src/checks.rs +++ b/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(), diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 835441f619..dce9105915 100644 --- a/src/checks_gen.rs +++ b/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, diff --git a/src/flake8_bugbear/plugins/loop_variable_overrides_iterator.rs b/src/flake8_bugbear/plugins/loop_variable_overrides_iterator.rs new file mode 100644 index 0000000000..683afd24d7 --- /dev/null +++ b/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), + )); + } + } +} diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index a92a0501bb..5fe54e3c85 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/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; diff --git a/src/linter.rs b/src/linter.rs index 2f1098995a..cb8bea5839 100644 --- a/src/linter.rs +++ b/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")] diff --git a/src/snapshots/ruff__linter__tests__B020_B020.py.snap b/src/snapshots/ruff__linter__tests__B020_B020.py.snap new file mode 100644 index 0000000000..cce10096b4 --- /dev/null +++ b/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: ~ +