mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-26 20:10:09 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Implement B023 (function uses loop variable) (#907)

Browse source
This commit is contained in:
Charlie Marsh 2022-11-25 18:29:54 -05:00 • committed by GitHub
parent 7c78d4e103
commit 7445d00b88
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 600 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

9
README.md
View file

@ -575,6 +575,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/
| 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 | |
| B023 | FunctionUsesLoopVariable | Function definition does not bind loop variable `...` | |
| B024 | AbstractBaseClassWithoutAbstractMethod | `...` is an abstract base class, but it has no abstract methods | |
| B025 | DuplicateTryBlockException | try-except block with duplicate exception `Exception` | |
| B026 | StarArgUnpackingAfterKeywordArg | Star-arg unpacking after a keyword argument is strongly discouraged | |
@ -820,6 +821,8 @@ including:
- [`pydocstyle`](https://pypi.org/project/pydocstyle/)
- [`pep8-naming`](https://pypi.org/project/pep8-naming/)
- [`yesqa`](https://github.com/asottile/yesqa)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/)
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
@ -827,9 +830,7 @@ including:
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-bandit`](https://pypi.org/project/flake8-bandit/) (6/40)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (27/32)
- [`flake8-2020`](https://pypi.org/project/flake8-2020/)
- [`flake8-blind-except`](https://pypi.org/project/flake8-blind-except/)
- [`flake8-boolean-trap`](https://pypi.org/project/flake8-boolean-trap/)
@ -851,6 +852,8 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`pydocstyle`](https://pypi.org/project/pydocstyle/)
- [`pep8-naming`](https://pypi.org/project/pep8-naming/)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/)
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
@ -859,8 +862,6 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`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/) (27/32)
- [`flake8-2020`](https://pypi.org/project/flake8-2020/)
- [`flake8-blind-except`](https://pypi.org/project/flake8-blind-except/)
- [`flake8-boolean-trap`](https://pypi.org/project/flake8-boolean-trap/)

78
resources/test/fixtures/B023.py vendored Normal file
View file

@ -0,0 +1,78 @@
"""
Should emit:
B023 - on lines 12, 13, 16, 28, 29, 30, 31, 40, 42, 50, 51, 52, 53, 61, 68.
"""
functions = []
z = 0
for x in range(3):
y = x + 1
# Subject to late-binding problems
functions.append(lambda: x)
functions.append(lambda: y) # not just the loop var
def f_bad_1():
return x
# Actually OK
functions.append(lambda x: x * 2)
functions.append(lambda x=x: x)
functions.append(lambda: z) # OK because not assigned in the loop
def f_ok_1(x):
return x * 2
def check_inside_functions_too():
ls = [lambda: x for x in range(2)]
st = {lambda: x for x in range(2)}
gn = (lambda: x for x in range(2))
dt = {x: lambda: x for x in range(2)}
async def pointless_async_iterable():
yield 1
async def container_for_problems():
async for x in pointless_async_iterable():
functions.append(lambda: x)
[lambda: x async for x in pointless_async_iterable()]
a = 10
b = 0
while True:
a = a_ = a - 1
b += 1
functions.append(lambda: a)
functions.append(lambda: a_)
functions.append(lambda: b)
functions.append(lambda: c) # not a name error because of late binding!
c: bool = a > 3
if not c:
break
# Nested loops should not duplicate reports
for j in range(2):
for k in range(3):
lambda: j * k
for j, k, l in [(1, 2, 3)]:
def f():
j = None # OK because it's an assignment
[l for k in range(2)] # error for l, not for k
assert a and functions
a.attribute = 1 # modifying an attribute doesn't make it a loop variable
functions[0] = lambda: None # same for an element
for var in range(2):
def explicit_capture(captured=var):
return captured

3
ruff_dev/src/generate_check_code_prefix.rs
View file

@ -117,7 +117,8 @@ pub fn main(cli: &Cli) -> Result<()> {
// Construct the output contents.
let mut output = String::new();
output.push_str("//! File automatically generated by examples/generate_check_code_prefix.rs.");
output
.push_str("//! File automatically generated by `examples/generate_check_code_prefix.rs`.");
output.push('\n');
output.push('\n');
output.push_str("use serde::{{Serialize, Deserialize}};");

25
src/ast/helpers.rs
View file

@ -1,7 +1,9 @@
use once_cell::sync::Lazy;
use regex::Regex;
use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Location, Stmt, StmtKind};
use rustpython_ast::{
Arguments, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Location, Stmt, StmtKind,
};
use crate::ast::types::Range;
use crate::SourceCodeLocator;
@ -213,6 +215,27 @@ pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<Vec<&str>> {
handler_names
}
/// Return the set of all bound argument names.
pub fn collect_arg_names<'a>(arguments: &'a Arguments) -> FxHashSet<&'a str> {
let mut arg_names: FxHashSet<&'a str> = FxHashSet::default();
for arg in &arguments.posonlyargs {
arg_names.insert(arg.node.arg.as_str());
}
for arg in &arguments.args {
arg_names.insert(arg.node.arg.as_str());
}
if let Some(arg) = &arguments.vararg {
arg_names.insert(arg.node.arg.as_str());
}
for arg in &arguments.kwonlyargs {
arg_names.insert(arg.node.arg.as_str());
}
if let Some(arg) = &arguments.kwarg {
arg_names.insert(arg.node.arg.as_str());
}
arg_names
}
/// Returns `true` if a call is an argumented `super` invocation.
pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool {
// Check: is this a `super` call?

8
src/ast/types.rs
View file

@ -1,7 +1,7 @@
use std::sync::atomic::{AtomicUsize, Ordering};
use rustc_hash::FxHashMap;
use rustpython_ast::{Expr, Keyword};
use rustpython_ast::{Expr, Keyword, Stmt};
use rustpython_parser::ast::{Located, Location};
fn id() -> usize {
@ -9,6 +9,12 @@ fn id() -> usize {
COUNTER.fetch_add(1, Ordering::Relaxed)
}
#[derive(Clone)]
pub enum Node<'a> {
Stmt(&'a Stmt),
Expr(&'a Expr),
}
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
pub struct Range {
pub location: Location,

25
src/check_ast.rs
View file

@ -19,8 +19,8 @@ use crate::ast::helpers::{
use crate::ast::operations::extract_all_names;
use crate::ast::relocate::relocate_expr;
use crate::ast::types::{
Binding, BindingContext, BindingKind, ClassScope, FunctionScope, ImportKind, Range, Scope,
ScopeKind,
Binding, BindingContext, BindingKind, ClassScope, FunctionScope, ImportKind, Node, Range,
Scope, ScopeKind,
};
use crate::ast::visitor::{walk_excepthandler, walk_withitem, Visitor};
use crate::ast::{helpers, operations, visitor};
@ -83,6 +83,8 @@ pub struct Checker<'a> {
futures_allowed: bool,
annotations_future_enabled: bool,
except_handlers: Vec<Vec<Vec<&'a str>>>,
// Check-specific state.
pub(crate) seen_b023: Vec<&'a Expr>,
}
impl<'a> Checker<'a> {
@ -127,6 +129,8 @@ impl<'a> Checker<'a> {
futures_allowed: true,
annotations_future_enabled: false,
except_handlers: vec![],
// Check-specific state.
seen_b023: vec![],
}
}
@ -951,8 +955,16 @@ where
flake8_bugbear::plugins::assert_raises_exception(self, stmt, items);
}
}
StmtKind::While { .. } => {
if self.settings.enabled.contains(&CheckCode::B023) {
flake8_bugbear::plugins::function_uses_loop_variable(self, &Node::Stmt(stmt));
}
}
StmtKind::For {
target, body, iter, ..
}
| StmtKind::AsyncFor {
target, body, iter, ..
} => {
if self.settings.enabled.contains(&CheckCode::B007) {
flake8_bugbear::plugins::unused_loop_control_variable(self, target, body);
@ -960,6 +972,9 @@ where
if self.settings.enabled.contains(&CheckCode::B020) {
flake8_bugbear::plugins::loop_variable_overrides_iterator(self, target, iter);
}
if self.settings.enabled.contains(&CheckCode::B023) {
flake8_bugbear::plugins::function_uses_loop_variable(self, &Node::Stmt(stmt));
}
}
StmtKind::Try { handlers, .. } => {
if self.settings.enabled.contains(&CheckCode::F707) {
@ -1810,9 +1825,15 @@ where
self.add_check(check);
};
}
if self.settings.enabled.contains(&CheckCode::B023) {
flake8_bugbear::plugins::function_uses_loop_variable(self, &Node::Expr(expr));
}
self.push_scope(Scope::new(ScopeKind::Generator));
}
ExprKind::GeneratorExp { .. } | ExprKind::DictComp { .. } => {
if self.settings.enabled.contains(&CheckCode::B023) {
flake8_bugbear::plugins::function_uses_loop_variable(self, &Node::Expr(expr));
}
self.push_scope(Scope::new(ScopeKind::Generator));
}
_ => {}

96
src/checks.rs
View file

@ -104,6 +104,7 @@ pub enum CheckCode {
B020,
B021,
B022,
B023,
B024,
B025,
B026,
@ -427,32 +428,33 @@ pub enum CheckKind {
// flake8-blind-except
BlindExcept,
// flake8-bugbear
UnaryPrefixIncrement,
AssignmentToOsEnviron,
UnreliableCallableCheck,
StripWithMultiCharacters,
MutableArgumentDefault,
UnusedLoopControlVariable(String),
FunctionCallArgumentDefault(Option<String>),
GetAttrWithConstant,
SetAttrWithConstant,
DoNotAssertFalse,
JumpStatementInFinally(String),
RedundantTupleInExceptionHandler(String),
DuplicateHandlerException(Vec<String>),
UselessComparison,
CannotRaiseLiteral,
NoAssertRaisesException,
UselessExpression,
CachedInstanceMethod,
LoopVariableOverridesIterator(String),
FStringDocstring,
UselessContextlibSuppress,
AbstractBaseClassWithoutAbstractMethod(String),
AssignmentToOsEnviron,
CachedInstanceMethod,
CannotRaiseLiteral,
DoNotAssertFalse,
DuplicateHandlerException(Vec<String>),
DuplicateTryBlockException(String),
StarArgUnpackingAfterKeywordArg,
EmptyMethodWithoutAbstractDecorator(String),
FStringDocstring,
FunctionCallArgumentDefault(Option<String>),
FunctionUsesLoopVariable(String),
GetAttrWithConstant,
JumpStatementInFinally(String),
LoopVariableOverridesIterator(String),
MutableArgumentDefault,
NoAssertRaisesException,
RaiseWithoutFromInsideExcept,
RedundantTupleInExceptionHandler(String),
SetAttrWithConstant,
StarArgUnpackingAfterKeywordArg,
StripWithMultiCharacters,
UnaryPrefixIncrement,
UnreliableCallableCheck,
UnusedLoopControlVariable(String),
UselessComparison,
UselessContextlibSuppress,
UselessExpression,
// flake8-comprehensions
UnnecessaryGeneratorList,
UnnecessaryGeneratorSet,
@ -716,6 +718,7 @@ impl CheckCode {
CheckCode::B020 => CheckKind::LoopVariableOverridesIterator("...".to_string()),
CheckCode::B021 => CheckKind::FStringDocstring,
CheckCode::B022 => CheckKind::UselessContextlibSuppress,
CheckCode::B023 => CheckKind::FunctionUsesLoopVariable("...".to_string()),
CheckCode::B024 => CheckKind::AbstractBaseClassWithoutAbstractMethod("...".to_string()),
CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()),
CheckCode::B026 => CheckKind::StarArgUnpackingAfterKeywordArg,
@ -981,6 +984,7 @@ impl CheckCode {
CheckCode::B020 => CheckCategory::Flake8Bugbear,
CheckCode::B021 => CheckCategory::Flake8Bugbear,
CheckCode::B022 => CheckCategory::Flake8Bugbear,
CheckCode::B023 => CheckCategory::Flake8Bugbear,
CheckCode::B024 => CheckCategory::Flake8Bugbear,
CheckCode::B025 => CheckCategory::Flake8Bugbear,
CheckCode::B026 => CheckCategory::Flake8Bugbear,
@ -1184,32 +1188,33 @@ impl CheckKind {
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
// flake8-bugbear
CheckKind::UnaryPrefixIncrement => &CheckCode::B002,
CheckKind::AssignmentToOsEnviron => &CheckCode::B003,
CheckKind::UnreliableCallableCheck => &CheckCode::B004,
CheckKind::StripWithMultiCharacters => &CheckCode::B005,
CheckKind::MutableArgumentDefault => &CheckCode::B006,
CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007,
CheckKind::FunctionCallArgumentDefault(_) => &CheckCode::B008,
CheckKind::GetAttrWithConstant => &CheckCode::B009,
CheckKind::SetAttrWithConstant => &CheckCode::B010,
CheckKind::DoNotAssertFalse => &CheckCode::B011,
CheckKind::JumpStatementInFinally(_) => &CheckCode::B012,
CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013,
CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
CheckKind::UselessComparison => &CheckCode::B015,
CheckKind::CannotRaiseLiteral => &CheckCode::B016,
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,
CheckKind::AssignmentToOsEnviron => &CheckCode::B003,
CheckKind::CachedInstanceMethod => &CheckCode::B019,
CheckKind::CannotRaiseLiteral => &CheckCode::B016,
CheckKind::DoNotAssertFalse => &CheckCode::B011,
CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025,
CheckKind::StarArgUnpackingAfterKeywordArg => &CheckCode::B026,
CheckKind::EmptyMethodWithoutAbstractDecorator(_) => &CheckCode::B027,
CheckKind::FStringDocstring => &CheckCode::B021,
CheckKind::FunctionCallArgumentDefault(_) => &CheckCode::B008,
CheckKind::FunctionUsesLoopVariable(_) => &CheckCode::B023,
CheckKind::GetAttrWithConstant => &CheckCode::B009,
CheckKind::JumpStatementInFinally(_) => &CheckCode::B012,
CheckKind::LoopVariableOverridesIterator(_) => &CheckCode::B020,
CheckKind::MutableArgumentDefault => &CheckCode::B006,
CheckKind::NoAssertRaisesException => &CheckCode::B017,
CheckKind::RaiseWithoutFromInsideExcept => &CheckCode::B904,
CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013,
CheckKind::SetAttrWithConstant => &CheckCode::B010,
CheckKind::StarArgUnpackingAfterKeywordArg => &CheckCode::B026,
CheckKind::StripWithMultiCharacters => &CheckCode::B005,
CheckKind::UnaryPrefixIncrement => &CheckCode::B002,
CheckKind::UnreliableCallableCheck => &CheckCode::B004,
CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007,
CheckKind::UselessComparison => &CheckCode::B015,
CheckKind::UselessContextlibSuppress => &CheckCode::B022,
CheckKind::UselessExpression => &CheckCode::B018,
// flake8-blind-except
CheckKind::BlindExcept => &CheckCode::BLE001,
// flake8-comprehensions
@ -1554,6 +1559,9 @@ impl CheckKind {
"Do not perform function call in argument defaults".to_string()
}
}
CheckKind::FunctionUsesLoopVariable(name) => {
format!("Function definition does not bind loop variable `{name}`")
}
CheckKind::GetAttrWithConstant => "Do not call `getattr` with a constant attribute \
value. It is not any safer than normal property \
access."

39
src/checks_gen.rs
View file

@ -59,6 +59,7 @@ pub enum CheckCodePrefix {
B020,
B021,
B022,
B023,
B024,
B025,
B026,
@ -186,6 +187,12 @@ pub enum CheckCodePrefix {
F406,
F407,
F5,
F52,
F521,
F522,
F523,
F524,
F525,
F54,
F541,
F6,
@ -415,6 +422,7 @@ impl CheckCodePrefix {
CheckCode::B020,
CheckCode::B021,
CheckCode::B022,
CheckCode::B023,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
@ -443,6 +451,7 @@ impl CheckCodePrefix {
CheckCode::B020,
CheckCode::B021,
CheckCode::B022,
CheckCode::B023,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
@ -492,6 +501,7 @@ impl CheckCodePrefix {
CheckCode::B020,
CheckCode::B021,
CheckCode::B022,
CheckCode::B023,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
@ -500,6 +510,7 @@ impl CheckCodePrefix {
CheckCodePrefix::B020 => vec![CheckCode::B020],
CheckCodePrefix::B021 => vec![CheckCode::B021],
CheckCodePrefix::B022 => vec![CheckCode::B022],
CheckCodePrefix::B023 => vec![CheckCode::B023],
CheckCodePrefix::B024 => vec![CheckCode::B024],
CheckCodePrefix::B025 => vec![CheckCode::B025],
CheckCodePrefix::B026 => vec![CheckCode::B026],
@ -899,7 +910,26 @@ impl CheckCodePrefix {
CheckCodePrefix::F405 => vec![CheckCode::F405],
CheckCodePrefix::F406 => vec![CheckCode::F406],
CheckCodePrefix::F407 => vec![CheckCode::F407],
CheckCodePrefix::F5 => vec![CheckCode::F541],
CheckCodePrefix::F5 => vec![
CheckCode::F521,
CheckCode::F522,
CheckCode::F523,
CheckCode::F524,
CheckCode::F525,
CheckCode::F541,
],
CheckCodePrefix::F52 => vec![
CheckCode::F521,
CheckCode::F522,
CheckCode::F523,
CheckCode::F524,
CheckCode::F525,
],
CheckCodePrefix::F521 => vec![CheckCode::F521],
CheckCodePrefix::F522 => vec![CheckCode::F522],
CheckCodePrefix::F523 => vec![CheckCode::F523],
CheckCodePrefix::F524 => vec![CheckCode::F524],
CheckCodePrefix::F525 => vec![CheckCode::F525],
CheckCodePrefix::F54 => vec![CheckCode::F541],
CheckCodePrefix::F541 => vec![CheckCode::F541],
CheckCodePrefix::F6 => vec![
@ -1293,6 +1323,7 @@ impl CheckCodePrefix {
CheckCodePrefix::B020 => PrefixSpecificity::Explicit,
CheckCodePrefix::B021 => PrefixSpecificity::Explicit,
CheckCodePrefix::B022 => PrefixSpecificity::Explicit,
CheckCodePrefix::B023 => PrefixSpecificity::Explicit,
CheckCodePrefix::B024 => PrefixSpecificity::Explicit,
CheckCodePrefix::B025 => PrefixSpecificity::Explicit,
CheckCodePrefix::B026 => PrefixSpecificity::Explicit,
@ -1420,6 +1451,12 @@ impl CheckCodePrefix {
CheckCodePrefix::F406 => PrefixSpecificity::Explicit,
CheckCodePrefix::F407 => PrefixSpecificity::Explicit,
CheckCodePrefix::F5 => PrefixSpecificity::Hundreds,
CheckCodePrefix::F52 => PrefixSpecificity::Tens,
CheckCodePrefix::F521 => PrefixSpecificity::Explicit,
CheckCodePrefix::F522 => PrefixSpecificity::Explicit,
CheckCodePrefix::F523 => PrefixSpecificity::Explicit,
CheckCodePrefix::F524 => PrefixSpecificity::Explicit,
CheckCodePrefix::F525 => PrefixSpecificity::Explicit,
CheckCodePrefix::F54 => PrefixSpecificity::Tens,
CheckCodePrefix::F541 => PrefixSpecificity::Explicit,
CheckCodePrefix::F6 => PrefixSpecificity::Hundreds,

2
src/flake8_annotations/plugins.rs
View file

@ -21,7 +21,7 @@ where
fn visit_stmt(&mut self, stmt: &'b Stmt) {
match &stmt.node {
StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => {
// No recurse.
// Don't recurse.
}
StmtKind::Return { value } => self.returns.push(value.as_ref().map(|expr| &**expr)),
_ => visitor::walk_stmt(self, stmt),

218
src/flake8_bugbear/plugins/function_uses_loop_variable.rs Normal file
View file

@ -0,0 +1,218 @@
use rustc_hash::FxHashSet;
use rustpython_ast::{Comprehension, Expr, ExprContext, ExprKind, Stmt, StmtKind};
use crate::ast::helpers::collect_arg_names;
use crate::ast::types::{Node, Range};
use crate::ast::visitor;
use crate::ast::visitor::Visitor;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
#[derive(Default)]
struct LoadedNamesVisitor<'a> {
names: Vec<(&'a str, &'a Expr)>,
}
/// `Visitor` to collect all used identifiers in a statement.
impl<'a, 'b> Visitor<'b> for LoadedNamesVisitor<'a>
where
'b: 'a,
{
fn visit_expr(&mut self, expr: &'b Expr) {
match &expr.node {
ExprKind::Name { id, ctx } if matches!(ctx, ExprContext::Load) => {
self.names.push((id, expr));
}
_ => visitor::walk_expr(self, expr),
}
}
}
#[derive(Default)]
struct SuspiciousVariablesVisitor<'a> {
names: Vec<(&'a str, &'a Expr)>,
}
/// `Visitor` to collect all suspicious variables (those referenced in
/// functions, but not bound as arguments).
impl<'a, 'b> Visitor<'b> for SuspiciousVariablesVisitor<'a>
where
'b: 'a,
{
fn visit_stmt(&mut self, stmt: &'b Stmt) {
match &stmt.node {
StmtKind::FunctionDef { args, body, .. }
| StmtKind::AsyncFunctionDef { args, body, .. } => {
// Collect all loaded variable names.
let mut visitor = LoadedNamesVisitor::default();
for stmt in body {
visitor.visit_stmt(stmt);
}
// Collect all argument names.
let arg_names = collect_arg_names(args);
// Treat any non-arguments as "suspicious".
self.names.extend(
visitor
.names
.into_iter()
.filter(|(id, _)| !arg_names.contains(id)),
);
}
_ => visitor::walk_stmt(self, stmt),
}
}
fn visit_expr(&mut self, expr: &'b Expr) {
match &expr.node {
ExprKind::Lambda { args, body } => {
// Collect all loaded variable names.
let mut visitor = LoadedNamesVisitor::default();
visitor.visit_expr(body);
// Collect all argument names.
let arg_names = collect_arg_names(args);
// Treat any non-arguments as "suspicious".
self.names.extend(
visitor
.names
.into_iter()
.filter(|(id, _)| !arg_names.contains(id)),
);
}
_ => visitor::walk_expr(self, expr),
}
}
}
#[derive(Default)]
struct NamesFromAssignmentsVisitor<'a> {
names: FxHashSet<&'a str>,
}
/// `Visitor` to collect all names used in an assignment expression.
impl<'a, 'b> Visitor<'b> for NamesFromAssignmentsVisitor<'a>
where
'b: 'a,
{
fn visit_expr(&mut self, expr: &'b Expr) {
match &expr.node {
ExprKind::Name { id, .. } => {
self.names.insert(id.as_str());
}
ExprKind::Starred { value, .. } => {
self.visit_expr(value);
}
ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } => {
for expr in elts {
self.visit_expr(expr);
}
}
_ => {}
}
}
}
#[derive(Default)]
struct AssignedNamesVisitor<'a> {
names: FxHashSet<&'a str>,
}
/// `Visitor` to collect all used identifiers in a statement.
impl<'a, 'b> Visitor<'b> for AssignedNamesVisitor<'a>
where
'b: 'a,
{
fn visit_stmt(&mut self, stmt: &'b Stmt) {
if matches!(
&stmt.node,
StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. }
) {
// Don't recurse.
return;
}
match &stmt.node {
StmtKind::Assign { targets, .. } => {
let mut visitor = NamesFromAssignmentsVisitor::default();
for expr in targets {
visitor.visit_expr(expr);
}
self.names.extend(visitor.names);
}
StmtKind::AugAssign { target, .. }
| StmtKind::AnnAssign { target, .. }
| StmtKind::For { target, .. }
| StmtKind::AsyncFor { target, .. } => {
let mut visitor = NamesFromAssignmentsVisitor::default();
visitor.visit_expr(target);
self.names.extend(visitor.names);
}
_ => {}
}
visitor::walk_stmt(self, stmt);
}
fn visit_expr(&mut self, expr: &'b Expr) {
if matches!(&expr.node, ExprKind::Lambda { .. }) {
// Don't recurse.
return;
}
visitor::walk_expr(self, expr);
}
fn visit_comprehension(&mut self, comprehension: &'b Comprehension) {
let mut visitor = NamesFromAssignmentsVisitor::default();
visitor.visit_expr(&comprehension.target);
self.names.extend(visitor.names);
visitor::walk_comprehension(self, comprehension);
}
}
/// B023
pub fn function_uses_loop_variable<'a, 'b>(checker: &'a mut Checker<'b>, node: &Node<'b>)
where
'b: 'a,
{
// Identify any "suspicious" variables. These are defined as variables that are
// referenced in a function or lambda body, but aren't bound as arguments.
let suspicious_variables = {
let mut visitor = SuspiciousVariablesVisitor::<'b>::default();
match node {
Node::Stmt(stmt) => visitor.visit_stmt(stmt),
Node::Expr(expr) => visitor.visit_expr(expr),
}
visitor.names
};
if !suspicious_variables.is_empty() {
// Identify any variables that are assigned in the loop (ignoring functions).
let reassigned_in_loop = {
let mut visitor = AssignedNamesVisitor::<'b>::default();
match node {
Node::Stmt(stmt) => visitor.visit_stmt(stmt),
Node::Expr(expr) => visitor.visit_expr(expr),
}
visitor.names
};
// If a variable was used in a function or lambda body, and assigned in the
// loop, flag it.
for (name, expr) in suspicious_variables {
if reassigned_in_loop.contains(name) {
if !checker.seen_b023.contains(&expr) {
checker.seen_b023.push(expr);
checker.add_check(Check::new(
CheckKind::FunctionUsesLoopVariable(name.to_string()),
Range::from_located(expr),
));
}
}
}
}
}

2
src/flake8_bugbear/plugins/mod.rs
View file

@ -7,6 +7,7 @@ pub use cannot_raise_literal::cannot_raise_literal;
pub use duplicate_exceptions::duplicate_exceptions;
pub use f_string_docstring::f_string_docstring;
pub use function_call_argument_default::function_call_argument_default;
pub use function_uses_loop_variable::function_uses_loop_variable;
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;
@ -32,6 +33,7 @@ mod cannot_raise_literal;
mod duplicate_exceptions;
mod f_string_docstring;
mod function_call_argument_default;
mod function_uses_loop_variable;
mod getattr_with_constant;
mod jump_statement_in_finally;
mod loop_variable_overrides_iterator;

1
src/linter.rs
View file

@ -413,6 +413,7 @@ mod tests {
#[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::B023, Path::new("B023.py"); "B023")]
#[test_case(CheckCode::B024, Path::new("B024.py"); "B024")]
#[test_case(CheckCode::B025, Path::new("B025.py"); "B025")]
#[test_case(CheckCode::B026, Path::new("B026.py"); "B026")]

149
src/snapshots/ruff__linter__tests__B023_B023.py.snap Normal file
View file

@ -0,0 +1,149 @@
---
source: src/linter.rs
expression: checks
---
- kind:
FunctionUsesLoopVariable: x
location:
row: 12
column: 29
end_location:
row: 12
column: 30
fix: ~
- kind:
FunctionUsesLoopVariable: y
location:
row: 13
column: 29
end_location:
row: 13
column: 30
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 16
column: 15
end_location:
row: 16
column: 16
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 28
column: 18
end_location:
row: 28
column: 19
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 29
column: 18
end_location:
row: 29
column: 19
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 30
column: 18
end_location:
row: 30
column: 19
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 31
column: 21
end_location:
row: 31
column: 22
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 40
column: 33
end_location:
row: 40
column: 34
fix: ~
- kind:
FunctionUsesLoopVariable: x
location:
row: 42
column: 13
end_location:
row: 42
column: 14
fix: ~
- kind:
FunctionUsesLoopVariable: a
location:
row: 50
column: 29
end_location:
row: 50
column: 30
fix: ~
- kind:
FunctionUsesLoopVariable: a_
location:
row: 51
column: 29
end_location:
row: 51
column: 31
fix: ~
- kind:
FunctionUsesLoopVariable: b
location:
row: 52
column: 29
end_location:
row: 52
column: 30
fix: ~
- kind:
FunctionUsesLoopVariable: c
location:
row: 53
column: 29
end_location:
row: 53
column: 30
fix: ~
- kind:
FunctionUsesLoopVariable: j
location:
row: 61
column: 16
end_location:
row: 61
column: 17
fix: ~
- kind:
FunctionUsesLoopVariable: k
location:
row: 61
column: 20
end_location:
row: 61
column: 21
fix: ~
- kind:
FunctionUsesLoopVariable: l
location:
row: 68
column: 9
end_location:
row: 68
column: 10
fix: ~