diff --git a/resources/test/fixtures/flake8_bugbear/B007.py b/resources/test/fixtures/flake8_bugbear/B007.py index be623afa8e..569b5ef0d1 100644 --- a/resources/test/fixtures/flake8_bugbear/B007.py +++ b/resources/test/fixtures/flake8_bugbear/B007.py @@ -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())) diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index fb414b4db8..2237205c76 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -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; @@ -239,6 +239,186 @@ where } } +pub fn any_over_stmt(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(body: &[Stmt], func: &F) -> bool +where + F: Fn(&Expr) -> bool, +{ + body.iter().any(|stmt| any_over_stmt(stmt, func)) +} + static DUNDER_REGEX: Lazy = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap()); /// Return `true` if the [`Stmt`] is an assignment to a dunder (like `__all__`). @@ -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); diff --git a/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index 7fd0fc8113..c1808847a1 100644 --- a/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -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; @@ -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); @@ -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; }