Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid flagging unused loop variable (B007) with locals() #2161

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Avoid flagging unused loop variable (B007) with locals() #2161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions resources/test/fixtures/flake8_bugbear/B007.py
View file
Open in desktop
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
View file
Open in desktop
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
8 changes: 6 additions & 2 deletions src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs
View file
Open in desktop
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