Skip to content

Commit

Permalink
Avoid flagging unused loop variable (B007) with locals()
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 25, 2023
1 parent f6fd702 commit 821e025
Show file tree
Hide file tree
Showing 3 changed files with 208 additions and 4 deletions.
5 changes: 5 additions & 0 deletions resources/test/fixtures/flake8_bugbear/B007.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ def strange_generator():

for i, (j, (k, l)) in strange_generator(): # i, k not used
print(j, l)

FMT = "{foo} {bar}"
for foo, bar in [(1, 2)]:
if foo:
print(FMT.format(**locals()))
199 changes: 197 additions & 2 deletions src/ast/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, ExprKind, Keyword, KeywordData,
Located, Location, Stmt, StmtKind,
Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Keyword,
KeywordData, Located, Location, Stmt, StmtKind,
};
use rustpython_parser::lexer;
use rustpython_parser::lexer::Tok;
Expand Down Expand Up @@ -239,6 +239,186 @@ where
}
}

pub fn any_over_stmt<F>(stmt: &Stmt, func: &F) -> bool
where
F: Fn(&Expr) -> bool,
{
match &stmt.node {
StmtKind::FunctionDef {
args,
body,
decorator_list,
returns,
..
}
| StmtKind::AsyncFunctionDef {
args,
body,
decorator_list,
returns,
..
} => {
args.defaults.iter().any(|expr| any_over_expr(expr, func))
|| args
.kw_defaults
.iter()
.any(|expr| any_over_expr(expr, func))
|| args.args.iter().any(|arg| {
arg.node
.annotation
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
})
|| args.kwonlyargs.iter().any(|arg| {
arg.node
.annotation
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
})
|| args.posonlyargs.iter().any(|arg| {
arg.node
.annotation
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
})
|| args.vararg.as_ref().map_or(false, |arg| {
arg.node
.annotation
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
})
|| args.kwarg.as_ref().map_or(false, |arg| {
arg.node
.annotation
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
})
|| body.iter().any(|stmt| any_over_stmt(stmt, func))
|| decorator_list.iter().any(|expr| any_over_expr(expr, func))
|| returns
.as_ref()
.map_or(false, |value| any_over_expr(value, func))
}
StmtKind::ClassDef {
bases,
keywords,
body,
decorator_list,
..
} => {
bases.iter().any(|expr| any_over_expr(expr, func))
|| keywords
.iter()
.any(|keyword| any_over_expr(&keyword.node.value, func))
|| body.iter().any(|stmt| any_over_stmt(stmt, func))
|| decorator_list.iter().any(|expr| any_over_expr(expr, func))
}
StmtKind::Return { value } => value
.as_ref()
.map_or(false, |value| any_over_expr(value, func)),
StmtKind::Delete { targets } => targets.iter().any(|expr| any_over_expr(expr, func)),
StmtKind::Assign { targets, value, .. } => {
targets.iter().any(|expr| any_over_expr(expr, func)) || any_over_expr(value, func)
}
StmtKind::AugAssign { target, value, .. } => {
any_over_expr(target, func) || any_over_expr(value, func)
}
StmtKind::AnnAssign {
target,
annotation,
value,
..
} => {
any_over_expr(target, func)
|| any_over_expr(annotation, func)
|| value
.as_ref()
.map_or(false, |value| any_over_expr(value, func))
}
StmtKind::For {
target,
iter,
body,
orelse,
..
}
| StmtKind::AsyncFor {
target,
iter,
body,
orelse,
..
} => {
any_over_expr(target, func)
|| any_over_expr(iter, func)
|| any_over_body(body, func)
|| any_over_body(orelse, func)
}
StmtKind::While { test, body, orelse } => {
any_over_expr(test, func) || any_over_body(body, func) || any_over_body(orelse, func)
}
StmtKind::If { test, body, orelse } => {
any_over_expr(test, func) || any_over_body(body, func) || any_over_body(orelse, func)
}
StmtKind::With { items, body, .. } | StmtKind::AsyncWith { items, body, .. } => {
items.iter().any(|withitem| {
any_over_expr(&withitem.context_expr, func)
|| withitem
.optional_vars
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
}) || any_over_body(body, func)
}
StmtKind::Raise { exc, cause } => {
exc.as_ref()
.map_or(false, |value| any_over_expr(value, func))
|| cause
.as_ref()
.map_or(false, |value| any_over_expr(value, func))
}
StmtKind::Try {
body,
handlers,
orelse,
finalbody,
} => {
any_over_body(body, func)
|| handlers.iter().any(|handler| {
let ExcepthandlerKind::ExceptHandler { type_, body, .. } = &handler.node;
type_
.as_ref()
.map_or(false, |expr| any_over_expr(expr, func))
|| any_over_body(body, func)
})
|| any_over_body(orelse, func)
|| any_over_body(finalbody, func)
}
StmtKind::Assert { test, msg } => {
any_over_expr(test, func)
|| msg
.as_ref()
.map_or(false, |value| any_over_expr(value, func))
}
// TODO(charlie): Handle match statements.
StmtKind::Match { .. } => false,
StmtKind::Import { .. } => false,
StmtKind::ImportFrom { .. } => false,
StmtKind::Global { .. } => false,
StmtKind::Nonlocal { .. } => false,
StmtKind::Expr { value } => any_over_expr(value, func),
StmtKind::Pass => false,
StmtKind::Break => false,
StmtKind::Continue => false,
}
}

pub fn any_over_body<F>(body: &[Stmt], func: &F) -> bool
where
F: Fn(&Expr) -> bool,
{
body.iter().any(|stmt| any_over_stmt(stmt, func))
}

static DUNDER_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap());

/// Return `true` if the [`Stmt`] is an assignment to a dunder (like `__all__`).
Expand Down Expand Up @@ -382,6 +562,21 @@ 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 {
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
}
} else {
false
}
})
}

/// Format the module name for a relative import.
pub fn format_import_from(level: Option<&usize>, module: Option<&str>) -> String {
let mut module_name = String::with_capacity(16);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use rustc_hash::FxHashMap;
use rustpython_ast::{Expr, ExprKind, Stmt};

use crate::ast::types::Range;
use crate::ast::visitor;
use crate::ast::visitor::Visitor;
use crate::ast::{helpers, visitor};
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::registry::Diagnostic;
Expand Down Expand Up @@ -57,6 +57,10 @@ 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);
Expand All @@ -73,7 +77,7 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body:

for (name, expr) in control_names {
// Ignore names that are already underscore-prefixed.
if name.starts_with('_') {
if checker.settings.dummy_variable_rgx.is_match(name) {
continue;
}

Expand Down

0 comments on commit 821e025

Please sign in to comment.