From 85b882fc54b28fefeeb834b5c09b51caf4244644 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 6 Nov 2022 17:42:10 -0500 Subject: [PATCH] Remove CheckLocator abstraction (#627) --- src/ast/types.rs | 4 - src/check_ast.rs | 507 +++++++++--------- src/flake8_bugbear/plugins/assert_false.rs | 7 +- .../plugins/assert_raises_exception.rs | 4 +- .../plugins/assignment_to_os_environ.rs | 4 +- .../plugins/cannot_raise_literal.rs | 4 +- .../plugins/duplicate_exceptions.rs | 6 +- .../plugins/function_call_argument_default.rs | 4 +- .../plugins/mutable_argument_default.rs | 6 +- .../redundant_tuple_in_exception_handler.rs | 4 +- .../plugins/unary_prefix_increment.rs | 4 +- .../plugins/unused_loop_control_variable.rs | 4 +- .../plugins/useless_comparison.rs | 4 +- .../plugins/useless_expression.rs | 6 +- src/flake8_print/plugins/print_call.rs | 4 +- src/pycodestyle/checks.rs | 24 +- src/pyflakes/checks.rs | 49 +- src/pyflakes/plugins/assert_tuple.rs | 5 +- src/pyflakes/plugins/if_tuple.rs | 4 +- src/pyflakes/plugins/raise_not_implemented.rs | 7 +- src/pyupgrade/plugins/type_of_primitive.rs | 6 +- src/pyupgrade/plugins/unnecessary_abspath.rs | 6 +- .../plugins/useless_metaclass_type.rs | 10 +- 23 files changed, 348 insertions(+), 335 deletions(-) diff --git a/src/ast/types.rs b/src/ast/types.rs index 9dbaf632fd..bb4a9c558c 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -100,10 +100,6 @@ pub struct Binding { pub used: Option<(usize, Range)>, } -pub trait CheckLocator { - fn locate_check(&self, default: Range) -> Range; -} - #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum ImportKind { Import, diff --git a/src/check_ast.rs b/src/check_ast.rs index cfa266b251..5b44e4ae80 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -15,8 +15,8 @@ use crate::ast::helpers::{extract_handler_names, match_name_or_attr_from_module} use crate::ast::operations::extract_all_names; use crate::ast::relocate::relocate_expr; use crate::ast::types::{ - Binding, BindingContext, BindingKind, CheckLocator, ClassScope, FunctionScope, ImportKind, - Range, Scope, ScopeKind, + Binding, BindingContext, BindingKind, ClassScope, FunctionScope, ImportKind, Range, Scope, + ScopeKind, }; use crate::ast::visitor::{walk_excepthandler, Visitor}; use crate::ast::{helpers, operations, visitor}; @@ -128,6 +128,31 @@ impl<'a> Checker<'a> { } } + /// Add a `Check` to the `Checker`. + #[inline(always)] + pub(crate) fn add_check(&mut self, check: Check) { + // If we're in an f-string, override the location. RustPython doesn't produce + // reliable locations for expressions within f-strings, so we use the + // span of the f-string itself as a best-effort default. + if let Some(range) = self.in_f_string { + self.checks.push(Check { + location: range.location, + end_location: range.end_location, + ..check + }); + } else { + self.checks.push(check); + } + } + + /// Add multiple `Check` items to the `Checker`. + #[inline(always)] + pub(crate) fn add_checks(&mut self, checks: impl Iterator) { + for check in checks { + self.add_check(check); + } + } + /// Return `true` if a patch should be generated under the given autofix /// `Mode`. pub fn patch(&self) -> bool { @@ -205,10 +230,15 @@ where } if self.settings.enabled.contains(&CheckCode::E741) { - let location = self.locate_check(Range::from_located(stmt)); - self.checks.extend(names.iter().filter_map(|name| { - pycodestyle::checks::ambiguous_variable_name(name, location) - })); + let location = Range::from_located(stmt); + self.add_checks( + names + .iter() + .filter_map(|name| { + pycodestyle::checks::ambiguous_variable_name(name, location) + }) + .into_iter(), + ); } } StmtKind::Break => { @@ -217,9 +247,8 @@ where stmt, &self.parents, &self.parent_stack, - self, ) { - self.checks.push(check); + self.add_check(check); } } } @@ -229,9 +258,8 @@ where stmt, &self.parents, &self.parent_stack, - self, ) { - self.checks.push(check); + self.add_check(check); } } } @@ -254,9 +282,9 @@ where if self.settings.enabled.contains(&CheckCode::E743) { if let Some(check) = pycodestyle::checks::ambiguous_function_name( name, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), ) { - self.checks.push(check); + self.add_check(check); } } @@ -266,7 +294,7 @@ where name, &self.settings.pep8_naming, ) { - self.checks.push(check); + self.add_check(check); } } @@ -280,7 +308,7 @@ where &self.settings.pep8_naming, ) { - self.checks.push(check); + self.add_check(check); } } @@ -292,7 +320,7 @@ where args, &self.settings.pep8_naming, ) { - self.checks.push(check); + self.add_check(check); } } @@ -300,7 +328,7 @@ where if let Some(check) = pep8_naming::checks::dunder_function_name(self.current_scope(), stmt, name) { - self.checks.push(check); + self.add_check(check); } } @@ -360,12 +388,12 @@ where } StmtKind::Return { .. } => { if self.settings.enabled.contains(&CheckCode::F706) { - if let Some(scope_index) = self.scope_stack.last().cloned() { - match self.scopes[scope_index].kind { + if let Some(index) = self.scope_stack.last().cloned() { + match self.scopes[index].kind { ScopeKind::Class(_) | ScopeKind::Module => { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ReturnOutsideFunction, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } _ => {} @@ -387,17 +415,16 @@ where } if self.settings.enabled.contains(&CheckCode::E742) { - if let Some(check) = pycodestyle::checks::ambiguous_class_name( - name, - self.locate_check(Range::from_located(stmt)), - ) { - self.checks.push(check); + if let Some(check) = + pycodestyle::checks::ambiguous_class_name(name, Range::from_located(stmt)) + { + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::N801) { if let Some(check) = pep8_naming::checks::invalid_class_name(stmt, name) { - self.checks.push(check); + self.add_check(check); } } @@ -405,7 +432,7 @@ where if let Some(check) = pep8_naming::checks::error_suffix_on_exception_name(stmt, bases, name) { - self.checks.push(check); + self.add_check(check); } } @@ -413,11 +440,7 @@ where flake8_bugbear::plugins::useless_expression(self, body); } - self.check_builtin_shadowing( - name, - self.locate_check(Range::from_located(stmt)), - false, - ); + self.check_builtin_shadowing(name, Range::from_located(stmt), false); for expr in bases { self.visit_expr(expr) @@ -438,9 +461,9 @@ where StmtKind::Import { names } => { if self.settings.enabled.contains(&CheckCode::E402) { if self.seen_import_boundary && stmt.location.column() == 0 { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ModuleImportNotAtTopOfFile, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } @@ -495,7 +518,7 @@ where stmt, name, asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -505,7 +528,7 @@ where stmt, name, asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -515,7 +538,7 @@ where stmt, name, asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -523,7 +546,7 @@ where if let Some(check) = pep8_naming::checks::camelcase_imported_as_constant( stmt, name, asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -531,7 +554,7 @@ where if let Some(check) = pep8_naming::checks::camelcase_imported_as_acronym( stmt, name, asname, ) { - self.checks.push(check); + self.add_check(check); } } } @@ -562,9 +585,9 @@ where if self.settings.enabled.contains(&CheckCode::E402) { if self.seen_import_boundary && stmt.location.column() == 0 { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ModuleImportNotAtTopOfFile, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } @@ -594,18 +617,18 @@ where if self.settings.enabled.contains(&CheckCode::F407) { if !ALL_FEATURE_NAMES.contains(&alias.node.name.deref()) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::FutureFeatureNotDefined(alias.node.name.to_string()), - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } if self.settings.enabled.contains(&CheckCode::F404) && !self.futures_allowed { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::LateFutureImport, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } else if alias.node.name == "*" { @@ -628,17 +651,17 @@ where let scope = &self.scopes [*(self.scope_stack.last().expect("No current scope found."))]; if !matches!(scope.kind, ScopeKind::Module) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ImportStarNotPermitted(module_name.to_string()), - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } if self.settings.enabled.contains(&CheckCode::F403) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ImportStarUsed(module_name.to_string()), - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } @@ -683,7 +706,7 @@ where asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -695,7 +718,7 @@ where asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -707,7 +730,7 @@ where asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -717,7 +740,7 @@ where &alias.node.name, asname, ) { - self.checks.push(check); + self.add_check(check); } } @@ -727,7 +750,7 @@ where &alias.node.name, asname, ) { - self.checks.push(check); + self.add_check(check); } } } @@ -774,7 +797,7 @@ where StmtKind::Try { handlers, .. } => { if self.settings.enabled.contains(&CheckCode::F707) { if let Some(check) = pyflakes::checks::default_except_not_last(handlers) { - self.checks.push(check); + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::B014) @@ -788,11 +811,10 @@ where } StmtKind::Assign { targets, value, .. } => { if self.settings.enabled.contains(&CheckCode::E731) { - if let Some(check) = pycodestyle::checks::do_not_assign_lambda( - value, - self.locate_check(Range::from_located(stmt)), - ) { - self.checks.push(check); + if let Some(check) = + pycodestyle::checks::do_not_assign_lambda(value, Range::from_located(stmt)) + { + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::U001) { @@ -807,9 +829,9 @@ where if let Some(value) = value { if let Some(check) = pycodestyle::checks::do_not_assign_lambda( value, - self.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), ) { - self.checks.push(check); + self.add_check(check); } } } @@ -956,9 +978,9 @@ where elts, check_too_many_expressions, check_two_starred_expressions, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); } } } @@ -978,9 +1000,9 @@ where if self.settings.enabled.contains(&CheckCode::E741) { if let Some(check) = pycodestyle::checks::ambiguous_variable_name( id, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); } } @@ -1029,9 +1051,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1043,9 +1065,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1057,9 +1079,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1072,10 +1094,10 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1085,10 +1107,10 @@ where func, args, keywords, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1100,9 +1122,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1114,9 +1136,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1128,9 +1150,9 @@ where keywords, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1142,10 +1164,10 @@ where args, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1157,10 +1179,10 @@ where args, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1171,9 +1193,9 @@ where args, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1182,10 +1204,10 @@ where flake8_comprehensions::checks::unnecessary_call_around_sorted( func, args, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1194,10 +1216,10 @@ where flake8_comprehensions::checks::unnecessary_double_cast_or_process( func, args, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1206,10 +1228,10 @@ where flake8_comprehensions::checks::unnecessary_subscript_reversal( func, args, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1217,9 +1239,9 @@ where if let Some(check) = flake8_comprehensions::checks::unnecessary_map( func, args, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } @@ -1253,21 +1275,23 @@ where let check_repeated_literals = self.settings.enabled.contains(&CheckCode::F601); let check_repeated_variables = self.settings.enabled.contains(&CheckCode::F602); if check_repeated_literals || check_repeated_variables { - self.checks.extend(pyflakes::checks::repeated_keys( - keys, - check_repeated_literals, - check_repeated_variables, - self, - )); + self.add_checks( + pyflakes::checks::repeated_keys( + keys, + check_repeated_literals, + check_repeated_variables, + ) + .into_iter(), + ); } } ExprKind::Yield { .. } | ExprKind::YieldFrom { .. } | ExprKind::Await { .. } => { let scope = self.current_scope(); if self.settings.enabled.contains(&CheckCode::F704) { if matches!(scope.kind, ScopeKind::Class(_) | ScopeKind::Module) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::YieldOutsideFunction, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } } @@ -1279,9 +1303,9 @@ where .iter() .any(|value| matches!(value.node, ExprKind::FormattedValue { .. })) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::FStringMissingPlaceholders, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } } @@ -1300,13 +1324,10 @@ where let check_not_in = self.settings.enabled.contains(&CheckCode::E713); let check_not_is = self.settings.enabled.contains(&CheckCode::E714); if check_not_in || check_not_is { - self.checks.extend(pycodestyle::checks::not_tests( - op, - operand, - check_not_in, - check_not_is, - self, - )); + self.add_checks( + pycodestyle::checks::not_tests(op, operand, check_not_in, check_not_is) + .into_iter(), + ); } if self.settings.enabled.contains(&CheckCode::B002) { @@ -1321,14 +1342,16 @@ where let check_none_comparisons = self.settings.enabled.contains(&CheckCode::E711); let check_true_false_comparisons = self.settings.enabled.contains(&CheckCode::E712); if check_none_comparisons || check_true_false_comparisons { - self.checks.extend(pycodestyle::checks::literal_comparisons( - left, - ops, - comparators, - check_none_comparisons, - check_true_false_comparisons, - self, - )); + self.add_checks( + pycodestyle::checks::literal_comparisons( + left, + ops, + comparators, + check_none_comparisons, + check_true_false_comparisons, + ) + .into_iter(), + ); } if self.settings.enabled.contains(&CheckCode::F632) { @@ -1337,16 +1360,19 @@ where left, ops, comparators, - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ); } if self.settings.enabled.contains(&CheckCode::E721) { - self.checks.extend(pycodestyle::checks::type_comparison( - ops, - comparators, - self.locate_check(Range::from_located(expr)), - )); + self.add_checks( + pycodestyle::checks::type_comparison( + ops, + comparators, + Range::from_located(expr), + ) + .into_iter(), + ); } } ExprKind::Constant { @@ -1401,9 +1427,9 @@ where generators, self.locator, self.patch(), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { - self.checks.push(check); + self.add_check(check); }; } self.push_scope(Scope::new(ScopeKind::Generator)) @@ -1597,7 +1623,7 @@ where match &excepthandler.node { ExcepthandlerKind::ExceptHandler { type_, name, .. } => { if self.settings.enabled.contains(&CheckCode::E722) && type_.is_none() { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::DoNotUseBareExcept, Range::from_located(excepthandler), )); @@ -1607,9 +1633,9 @@ where if self.settings.enabled.contains(&CheckCode::E741) { if let Some(check) = pycodestyle::checks::ambiguous_variable_name( name, - self.locate_check(Range::from_located(excepthandler)), + Range::from_located(excepthandler), ) { - self.checks.push(check); + self.add_check(check); } } @@ -1648,12 +1674,14 @@ where walk_excepthandler(self, excepthandler); - let scope = &mut self.scopes - [*(self.scope_stack.last().expect("No current scope found."))]; - if let Some(binding) = &scope.values.remove(name) { + if let Some(binding) = { + let scope = &mut self.scopes + [*(self.scope_stack.last().expect("No current scope found."))]; + &scope.values.remove(name) + } { if binding.used.is_none() { if self.settings.enabled.contains(&CheckCode::F841) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::UnusedVariable(name.to_string()), Range::from_located(excepthandler), )); @@ -1662,6 +1690,8 @@ where } if let Some(binding) = definition { + let scope = &mut self.scopes + [*(self.scope_stack.last().expect("No current scope found."))]; scope.values.insert(name.to_string(), binding); } } @@ -1717,9 +1747,9 @@ where if self.settings.enabled.contains(&CheckCode::E741) { if let Some(check) = pycodestyle::checks::ambiguous_variable_name( &arg.node.arg, - self.locate_check(Range::from_located(arg)), + Range::from_located(arg), ) { - self.checks.push(check); + self.add_check(check); } } @@ -1727,7 +1757,7 @@ where if let Some(check) = pep8_naming::checks::invalid_argument_name(&arg.node.arg, Range::from_located(arg)) { - self.checks.push(check); + self.add_check(check); } } @@ -1735,12 +1765,6 @@ where } } -impl CheckLocator for Checker<'_> { - fn locate_check(&self, default: Range) -> Range { - self.in_f_string.unwrap_or(default) - } -} - fn try_mark_used(scope: &mut Scope, scope_id: usize, id: &str, expr: &Expr) -> bool { let alias = if let Some(binding) = scope.values.get_mut(id) { // Mark the binding as used. @@ -1784,10 +1808,6 @@ fn try_mark_used(scope: &mut Scope, scope_id: usize, id: &str, expr: &Expr) -> b } impl<'a> Checker<'a> { - pub fn add_check(&mut self, check: Check) { - self.checks.push(check); - } - fn push_parent(&mut self, parent: &'a Stmt) { self.parent_stack.push(self.parents.len()); self.parents.push(parent); @@ -1856,41 +1876,43 @@ impl<'a> Checker<'a> { } fn add_binding(&mut self, name: String, binding: Binding) { - let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + if self.settings.enabled.contains(&CheckCode::F402) { + let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + if let Some(existing) = scope.values.get(&name) { + if matches!(binding.kind, BindingKind::LoopVar) + && matches!( + existing.kind, + BindingKind::Importation(_, _, _) + | BindingKind::FromImportation(_, _, _) + | BindingKind::SubmoduleImportation(_, _, _) + | BindingKind::StarImportation + | BindingKind::FutureImportation + ) + { + self.add_check(Check::new( + CheckKind::ImportShadowedByLoopVar( + name.clone(), + existing.range.location.row(), + ), + binding.range, + )); + } + } + } // TODO(charlie): Don't treat annotations as assignments if there is an existing // value. + let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; let binding = match scope.values.get(&name) { None => binding, - Some(existing) => { - if self.settings.enabled.contains(&CheckCode::F402) { - if matches!(binding.kind, BindingKind::LoopVar) - && matches!( - existing.kind, - BindingKind::Importation(_, _, _) - | BindingKind::FromImportation(_, _, _) - | BindingKind::SubmoduleImportation(_, _, _) - | BindingKind::StarImportation - | BindingKind::FutureImportation - ) - { - self.checks.push(Check::new( - CheckKind::ImportShadowedByLoopVar( - name.clone(), - existing.range.location.row(), - ), - binding.range, - )); - } - } - Binding { - kind: binding.kind, - range: binding.range, - used: existing.used, - } - } + Some(existing) => Binding { + kind: binding.kind, + range: binding.range, + used: existing.used, + }, }; + let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; scope.values.insert(name, binding); } @@ -1934,9 +1956,9 @@ impl<'a> Checker<'a> { } from_list.sort(); - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ImportStarUsage(id.clone(), from_list), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } return; @@ -1955,9 +1977,9 @@ impl<'a> Checker<'a> { } } - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::UndefinedName(id.clone()), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), )) } } @@ -1965,51 +1987,44 @@ impl<'a> Checker<'a> { fn handle_node_store(&mut self, expr: &Expr, parent: &Stmt) { if let ExprKind::Name { id, .. } = &expr.node { - let current = - &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; - if self.settings.enabled.contains(&CheckCode::F823) { - if matches!(current.kind, ScopeKind::Function(_)) - && !current.values.contains_key(id) - { - for scope in self.scopes.iter().rev().skip(1) { - if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) { - if let Some(binding) = scope.values.get(id) { - if let Some((scope_id, location)) = binding.used { - if scope_id == current.id { - self.checks.push(Check::new( - CheckKind::UndefinedLocal(id.clone()), - self.locate_check(location), - )); - } - } - } - } - } + let scopes: Vec<&Scope> = self + .scope_stack + .iter() + .map(|index| &self.scopes[*index]) + .collect(); + if let Some(check) = pyflakes::checks::undefined_local(&scopes, id) { + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::N806) { + let current = + &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; if let Some(check) = pep8_naming::checks::non_lowercase_variable_in_function(current, expr, id) { - self.checks.push(check); + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::N815) { + let current = + &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; if let Some(check) = pep8_naming::checks::mixed_case_variable_in_class_scope(current, expr, id) { - self.checks.push(check); + self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::N816) { + let current = + &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; if let Some(check) = pep8_naming::checks::mixed_case_variable_in_global_scope(current, expr, id) { - self.checks.push(check); + self.add_check(check); } } @@ -2053,6 +2068,8 @@ impl<'a> Checker<'a> { return; } + let current = + &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; if id == "__all__" && matches!(current.kind, ScopeKind::Module) && matches!( @@ -2094,9 +2111,9 @@ impl<'a> Checker<'a> { &mut self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; if scope.values.remove(id).is_none() && self.settings.enabled.contains(&CheckCode::F821) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::UndefinedName(id.clone()), - self.locate_check(Range::from_located(expr)), + Range::from_located(expr), )) } } @@ -2139,9 +2156,9 @@ impl<'a> Checker<'a> { allocator.push(expr); } else { if self.settings.enabled.contains(&CheckCode::F722) { - self.checks.push(Check::new( + self.add_check(Check::new( CheckKind::ForwardAnnotationSyntaxError(expression.to_string()), - self.locate_check(range), + range, )); } } @@ -2197,11 +2214,13 @@ impl<'a> Checker<'a> { fn check_deferred_assignments(&mut self) { if self.settings.enabled.contains(&CheckCode::F841) { while let Some(index) = self.deferred_assignments.pop() { - self.checks.extend(pyflakes::checks::unused_variables( - &self.scopes[index], - self, - &self.settings.dummy_variable_rgx, - )); + self.add_checks( + pyflakes::checks::unused_variables( + &self.scopes[index], + &self.settings.dummy_variable_rgx, + ) + .into_iter(), + ); } } } @@ -2214,9 +2233,8 @@ impl<'a> Checker<'a> { return; } - for index in self.dead_scopes.iter().copied() { - let scope = &self.scopes[index]; - + let mut checks: Vec = vec![]; + for scope in self.dead_scopes.iter().map(|index| &self.scopes[*index]) { let all_binding = scope.values.get("__all__"); let all_names = all_binding.and_then(|binding| match &binding.kind { BindingKind::Export(names) => Some(names), @@ -2229,9 +2247,9 @@ impl<'a> Checker<'a> { if let Some(names) = all_names { for name in names { if !scope.values.contains_key(name) { - self.checks.push(Check::new( + checks.push(Check::new( CheckKind::UndefinedExport(name.to_string()), - self.locate_check(all_binding.range), + all_binding.range, )); } } @@ -2240,23 +2258,25 @@ impl<'a> Checker<'a> { } } - if self.settings.enabled.contains(&CheckCode::F405) && scope.import_starred { - if let Some(all_binding) = all_binding { - if let Some(names) = all_names { - let mut from_list = vec![]; - for (name, binding) in scope.values.iter() { - if matches!(binding.kind, BindingKind::StarImportation) { - from_list.push(name.to_string()); + if self.settings.enabled.contains(&CheckCode::F405) { + if scope.import_starred { + if let Some(all_binding) = all_binding { + if let Some(names) = all_names { + let mut from_list = vec![]; + for (name, binding) in scope.values.iter() { + if matches!(binding.kind, BindingKind::StarImportation) { + from_list.push(name.to_string()); + } } - } - from_list.sort(); + from_list.sort(); - for name in names { - if !scope.values.contains_key(name) { - self.checks.push(Check::new( - CheckKind::ImportStarUsage(name.clone(), from_list.clone()), - self.locate_check(all_binding.range), - )); + for name in names { + if !scope.values.contains_key(name) { + checks.push(Check::new( + CheckKind::ImportStarUsage(name.clone(), from_list.clone()), + all_binding.range, + )); + } } } } @@ -2331,12 +2351,12 @@ impl<'a> Checker<'a> { }; if self.path.ends_with("__init__.py") { - self.checks.push(Check::new( + checks.push(Check::new( CheckKind::UnusedImport( full_names.into_iter().map(String::from).collect(), true, ), - self.locate_check(Range::from_located(child)), + Range::from_located(child), )); } else { let mut check = Check::new( @@ -2344,16 +2364,17 @@ impl<'a> Checker<'a> { full_names.into_iter().map(String::from).collect(), false, ), - self.locate_check(Range::from_located(child)), + Range::from_located(child), ); if let Some(fix) = fix { check.amend(fix); } - self.checks.push(check); + checks.push(check); } } } } + self.add_checks(checks.into_iter()); } fn check_definitions(&mut self) { @@ -2463,20 +2484,20 @@ impl<'a> Checker<'a> { if self.settings.enabled.contains(&CheckCode::A003) { if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, - self.locate_check(location), + location, flake8_builtins::types::ShadowingType::Attribute, ) { - self.checks.push(check); + self.add_check(check); } } } else { if self.settings.enabled.contains(&CheckCode::A001) { if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, - self.locate_check(location), + location, flake8_builtins::types::ShadowingType::Variable, ) { - self.checks.push(check); + self.add_check(check); } } } @@ -2486,10 +2507,10 @@ impl<'a> Checker<'a> { if self.settings.enabled.contains(&CheckCode::A002) { if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, - self.locate_check(location), + location, flake8_builtins::types::ShadowingType::Argument, ) { - self.checks.push(check); + self.add_check(check); } } } diff --git a/src/flake8_bugbear/plugins/assert_false.rs b/src/flake8_bugbear/plugins/assert_false.rs index 95976f192b..ebb25d9f08 100644 --- a/src/flake8_bugbear/plugins/assert_false.rs +++ b/src/flake8_bugbear/plugins/assert_false.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -42,10 +42,7 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: &Optio .. } = &test.node { - let mut check = Check::new( - CheckKind::DoNotAssertFalse, - checker.locate_check(Range::from_located(test)), - ); + let mut check = Check::new(CheckKind::DoNotAssertFalse, Range::from_located(test)); if checker.patch() { let mut generator = SourceGenerator::new(); if let Ok(()) = generator.unparse_stmt(&assertion_error(msg)) { diff --git a/src/flake8_bugbear/plugins/assert_raises_exception.rs b/src/flake8_bugbear/plugins/assert_raises_exception.rs index a3278df157..c408f4b7b9 100644 --- a/src/flake8_bugbear/plugins/assert_raises_exception.rs +++ b/src/flake8_bugbear/plugins/assert_raises_exception.rs @@ -1,7 +1,7 @@ use rustpython_ast::{ExprKind, Stmt, Withitem}; use crate::ast::helpers::match_name_or_attr; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -17,7 +17,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With { checker.add_check(Check::new( CheckKind::NoAssertRaisesException, - checker.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } diff --git a/src/flake8_bugbear/plugins/assignment_to_os_environ.rs b/src/flake8_bugbear/plugins/assignment_to_os_environ.rs index 5b2c1e312e..0bc538a6c8 100644 --- a/src/flake8_bugbear/plugins/assignment_to_os_environ.rs +++ b/src/flake8_bugbear/plugins/assignment_to_os_environ.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -14,7 +14,7 @@ pub fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr]) { if id == "os" { checker.add_check(Check::new( CheckKind::AssignmentToOsEnviron, - checker.locate_check(Range::from_located(target)), + Range::from_located(target), )); } } diff --git a/src/flake8_bugbear/plugins/cannot_raise_literal.rs b/src/flake8_bugbear/plugins/cannot_raise_literal.rs index 9908e32590..09b02bd5df 100644 --- a/src/flake8_bugbear/plugins/cannot_raise_literal.rs +++ b/src/flake8_bugbear/plugins/cannot_raise_literal.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -9,7 +9,7 @@ pub fn cannot_raise_literal(checker: &mut Checker, expr: &Expr) { if let ExprKind::Constant { .. } = &expr.node { checker.add_check(Check::new( CheckKind::CannotRaiseLiteral, - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } } diff --git a/src/flake8_bugbear/plugins/duplicate_exceptions.rs b/src/flake8_bugbear/plugins/duplicate_exceptions.rs index f4e098adc6..2d1acfeafe 100644 --- a/src/flake8_bugbear/plugins/duplicate_exceptions.rs +++ b/src/flake8_bugbear/plugins/duplicate_exceptions.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt}; use crate::ast::helpers; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckCode, CheckKind}; @@ -47,7 +47,7 @@ pub fn duplicate_handler_exceptions( CheckKind::DuplicateHandlerException( duplicates.into_iter().sorted().collect::>(), ), - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), ); if checker.patch() { // TODO(charlie): If we have a single element, remove the tuple. @@ -106,7 +106,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce for duplicate in duplicates.into_iter().sorted() { checker.add_check(Check::new( CheckKind::DuplicateTryBlockException(duplicate), - checker.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )); } } diff --git a/src/flake8_bugbear/plugins/function_call_argument_default.rs b/src/flake8_bugbear/plugins/function_call_argument_default.rs index 290dee3e58..035b99c98d 100644 --- a/src/flake8_bugbear/plugins/function_call_argument_default.rs +++ b/src/flake8_bugbear/plugins/function_call_argument_default.rs @@ -1,7 +1,7 @@ use rustpython_ast::{Arguments, Constant, Expr, ExprKind}; use crate::ast::helpers::compose_call_path; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::check_ast::Checker; @@ -91,6 +91,6 @@ pub fn function_call_argument_default(checker: &mut Checker, arguments: &Argumen visitor.visit_expr(expr); } for (check, range) in visitor.checks { - checker.add_check(Check::new(check, checker.locate_check(range))); + checker.add_check(Check::new(check, range)); } } diff --git a/src/flake8_bugbear/plugins/mutable_argument_default.rs b/src/flake8_bugbear/plugins/mutable_argument_default.rs index 3a443d40f2..7b7da70ff5 100644 --- a/src/flake8_bugbear/plugins/mutable_argument_default.rs +++ b/src/flake8_bugbear/plugins/mutable_argument_default.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Arguments, Expr, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -45,14 +45,14 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { | ExprKind::SetComp { .. } => { checker.add_check(Check::new( CheckKind::MutableArgumentDefault, - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } ExprKind::Call { func, .. } => { if is_mutable_func(func) { checker.add_check(Check::new( CheckKind::MutableArgumentDefault, - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), )); } } diff --git a/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs b/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs index bfb507187c..50de5a1fd1 100644 --- a/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs +++ b/src/flake8_bugbear/plugins/redundant_tuple_in_exception_handler.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -13,7 +13,7 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E if elts.len() == 1 { checker.add_check(Check::new( CheckKind::RedundantTupleInExceptionHandler(elts[0].to_string()), - checker.locate_check(Range::from_located(type_)), + Range::from_located(type_), )); } } diff --git a/src/flake8_bugbear/plugins/unary_prefix_increment.rs b/src/flake8_bugbear/plugins/unary_prefix_increment.rs index 487412fb81..16e22a1cfe 100644 --- a/src/flake8_bugbear/plugins/unary_prefix_increment.rs +++ b/src/flake8_bugbear/plugins/unary_prefix_increment.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind, Unaryop}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -11,7 +11,7 @@ pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop, if matches!(op, Unaryop::UAdd) { checker.add_check(Check::new( CheckKind::UnaryPrefixIncrement, - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), )) } } diff --git a/src/flake8_bugbear/plugins/unused_loop_control_variable.rs b/src/flake8_bugbear/plugins/unused_loop_control_variable.rs index 4937a3c97b..9eb0cd84dd 100644 --- a/src/flake8_bugbear/plugins/unused_loop_control_variable.rs +++ b/src/flake8_bugbear/plugins/unused_loop_control_variable.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use rustpython_ast::{Expr, ExprKind, Stmt}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::autofix::Fix; @@ -64,7 +64,7 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body: let mut check = Check::new( CheckKind::UnusedLoopControlVariable(name.to_string()), - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), ); if checker.patch() { // Prefix the variable name with an underscore. diff --git a/src/flake8_bugbear/plugins/useless_comparison.rs b/src/flake8_bugbear/plugins/useless_comparison.rs index 5eb4fbc002..80d6cd8ee8 100644 --- a/src/flake8_bugbear/plugins/useless_comparison.rs +++ b/src/flake8_bugbear/plugins/useless_comparison.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -8,7 +8,7 @@ pub fn useless_comparison(checker: &mut Checker, expr: &Expr) { if let ExprKind::Compare { left, .. } = &expr.node { checker.add_check(Check::new( CheckKind::UselessComparison, - checker.locate_check(Range::from_located(left)), + Range::from_located(left), )); } } diff --git a/src/flake8_bugbear/plugins/useless_expression.rs b/src/flake8_bugbear/plugins/useless_expression.rs index d98ce1c3fc..850195e0f8 100644 --- a/src/flake8_bugbear/plugins/useless_expression.rs +++ b/src/flake8_bugbear/plugins/useless_expression.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Constant, ExprKind, Stmt, StmtKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -12,7 +12,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) { ExprKind::List { .. } | ExprKind::Dict { .. } | ExprKind::Set { .. } => { checker.add_check(Check::new( CheckKind::UselessExpression, - checker.locate_check(Range::from_located(value)), + Range::from_located(value), )); } ExprKind::Constant { value: val, .. } => match &val { @@ -20,7 +20,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) { _ => { checker.add_check(Check::new( CheckKind::UselessExpression, - checker.locate_check(Range::from_located(value)), + Range::from_located(value), )); } }, diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 35ea9bd5b4..435621c942 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -1,7 +1,7 @@ use log::error; use rustpython_ast::{Expr, Stmt, StmtKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::helpers; use crate::check_ast::Checker; use crate::checks::CheckCode; @@ -13,7 +13,7 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { func, checker.settings.enabled.contains(&CheckCode::T201), checker.settings.enabled.contains(&CheckCode::T203), - checker.locate_check(Range::from_located(expr)), + Range::from_located(expr), ) { if checker.patch() { let context = checker.binding_context(); diff --git a/src/pycodestyle/checks.rs b/src/pycodestyle/checks.rs index 865d617069..864ae2d5be 100644 --- a/src/pycodestyle/checks.rs +++ b/src/pycodestyle/checks.rs @@ -2,7 +2,7 @@ use itertools::izip; use rustpython_ast::Location; use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Unaryop}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::checks::{Check, CheckKind, RejectedCmpop}; use crate::source_code_locator::SourceCodeLocator; @@ -61,7 +61,6 @@ pub fn not_tests( operand: &Expr, check_not_in: bool, check_not_is: bool, - locator: &dyn CheckLocator, ) -> Vec { let mut checks: Vec = vec![]; @@ -73,7 +72,7 @@ pub fn not_tests( if check_not_in { checks.push(Check::new( CheckKind::NotInTest, - locator.locate_check(Range::from_located(operand)), + Range::from_located(operand), )); } } @@ -81,7 +80,7 @@ pub fn not_tests( if check_not_is { checks.push(Check::new( CheckKind::NotIsTest, - locator.locate_check(Range::from_located(operand)), + Range::from_located(operand), )); } } @@ -101,7 +100,6 @@ pub fn literal_comparisons( comparators: &[Expr], check_none_comparisons: bool, check_true_false_comparisons: bool, - locator: &dyn CheckLocator, ) -> Vec { let mut checks: Vec = vec![]; @@ -121,13 +119,13 @@ pub fn literal_comparisons( if matches!(op, Cmpop::Eq) { checks.push(Check::new( CheckKind::NoneComparison(RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } if matches!(op, Cmpop::NotEq) { checks.push(Check::new( CheckKind::NoneComparison(RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } } @@ -141,13 +139,13 @@ pub fn literal_comparisons( if matches!(op, Cmpop::Eq) { checks.push(Check::new( CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } if matches!(op, Cmpop::NotEq) { checks.push(Check::new( CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } } @@ -167,13 +165,13 @@ pub fn literal_comparisons( if matches!(op, Cmpop::Eq) { checks.push(Check::new( CheckKind::NoneComparison(RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } if matches!(op, Cmpop::NotEq) { checks.push(Check::new( CheckKind::NoneComparison(RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } } @@ -187,13 +185,13 @@ pub fn literal_comparisons( if matches!(op, Cmpop::Eq) { checks.push(Check::new( CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } if matches!(op, Cmpop::NotEq) { checks.push(Check::new( CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), + Range::from_located(comparator), )); } } diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index 4871b57d86..29bc7e8ff3 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -5,7 +5,7 @@ use rustpython_parser::ast::{ Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind, }; -use crate::ast::types::{BindingKind, CheckLocator, FunctionScope, Range, Scope, ScopeKind}; +use crate::ast::types::{BindingKind, FunctionScope, Range, Scope, ScopeKind}; use crate::checks::{Check, CheckKind}; /// F631 @@ -28,12 +28,30 @@ pub fn if_tuple(test: &Expr, location: Range) -> Option { None } +/// F821 +pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option { + let current = &scopes.last().expect("No current scope found."); + if matches!(current.kind, ScopeKind::Function(_)) && !current.values.contains_key(name) { + for scope in scopes.iter().rev().skip(1) { + if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) { + if let Some(binding) = scope.values.get(name) { + if let Some((scope_id, location)) = binding.used { + if scope_id == current.id { + return Some(Check::new( + CheckKind::UndefinedLocal(name.to_string()), + location, + )); + } + } + } + } + } + } + None +} + /// F841 -pub fn unused_variables( - scope: &Scope, - locator: &dyn CheckLocator, - dummy_variable_rgx: &Regex, -) -> Vec { +pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec { let mut checks: Vec = vec![]; if matches!( @@ -53,7 +71,7 @@ pub fn unused_variables( { checks.push(Check::new( CheckKind::UnusedVariable(name.to_string()), - locator.locate_check(binding.range), + binding.range, )); } } @@ -129,7 +147,6 @@ pub fn repeated_keys( keys: &[Expr], check_repeated_literals: bool, check_repeated_variables: bool, - locator: &dyn CheckLocator, ) -> Vec { let mut checks: Vec = vec![]; @@ -144,7 +161,7 @@ pub fn repeated_keys( if check_repeated_literals && v1 == v2 { checks.push(Check::new( CheckKind::MultiValueRepeatedKeyLiteral, - locator.locate_check(Range::from_located(k2)), + Range::from_located(k2), )) } } @@ -152,7 +169,7 @@ pub fn repeated_keys( if check_repeated_variables && v1 == v2 { checks.push(Check::new( CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()), - locator.locate_check(Range::from_located(k2)), + Range::from_located(k2), )) } } @@ -195,12 +212,7 @@ pub fn starred_expressions( } /// F701 -pub fn break_outside_loop( - stmt: &Stmt, - parents: &[&Stmt], - parent_stack: &[usize], - locator: &dyn CheckLocator, -) -> Option { +pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize]) -> Option { let mut allowed: bool = false; let mut parent = stmt; for index in parent_stack.iter().rev() { @@ -228,7 +240,7 @@ pub fn break_outside_loop( if !allowed { Some(Check::new( CheckKind::BreakOutsideLoop, - locator.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )) } else { None @@ -240,7 +252,6 @@ pub fn continue_outside_loop( stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize], - locator: &dyn CheckLocator, ) -> Option { let mut allowed: bool = false; let mut parent = stmt; @@ -269,7 +280,7 @@ pub fn continue_outside_loop( if !allowed { Some(Check::new( CheckKind::ContinueOutsideLoop, - locator.locate_check(Range::from_located(stmt)), + Range::from_located(stmt), )) } else { None diff --git a/src/pyflakes/plugins/assert_tuple.rs b/src/pyflakes/plugins/assert_tuple.rs index b624a144f1..6cff56348a 100644 --- a/src/pyflakes/plugins/assert_tuple.rs +++ b/src/pyflakes/plugins/assert_tuple.rs @@ -1,13 +1,12 @@ use rustpython_ast::{Expr, Stmt}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::pyflakes::checks; /// F631 pub fn assert_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) { - if let Some(check) = checks::assert_tuple(test, checker.locate_check(Range::from_located(stmt))) - { + if let Some(check) = checks::assert_tuple(test, Range::from_located(stmt)) { checker.add_check(check); } } diff --git a/src/pyflakes/plugins/if_tuple.rs b/src/pyflakes/plugins/if_tuple.rs index e63bf9aab5..b272a65155 100644 --- a/src/pyflakes/plugins/if_tuple.rs +++ b/src/pyflakes/plugins/if_tuple.rs @@ -1,12 +1,12 @@ use rustpython_ast::{Expr, Stmt}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::pyflakes::checks; /// F634 pub fn if_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) { - if let Some(check) = checks::if_tuple(test, checker.locate_check(Range::from_located(stmt))) { + if let Some(check) = checks::if_tuple(test, Range::from_located(stmt)) { checker.add_check(check); } } diff --git a/src/pyflakes/plugins/raise_not_implemented.rs b/src/pyflakes/plugins/raise_not_implemented.rs index becc5ccf16..0c329e17b9 100644 --- a/src/pyflakes/plugins/raise_not_implemented.rs +++ b/src/pyflakes/plugins/raise_not_implemented.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -27,10 +27,7 @@ fn match_not_implemented(expr: &Expr) -> Option<&Expr> { /// F901 pub fn raise_not_implemented(checker: &mut Checker, expr: &Expr) { if let Some(expr) = match_not_implemented(expr) { - let mut check = Check::new( - CheckKind::RaiseNotImplemented, - checker.locate_check(Range::from_located(expr)), - ); + let mut check = Check::new(CheckKind::RaiseNotImplemented, Range::from_located(expr)); if checker.patch() { check.amend(Fix::replacement( "NotImplementedError".to_string(), diff --git a/src/pyupgrade/plugins/type_of_primitive.rs b/src/pyupgrade/plugins/type_of_primitive.rs index 06ae400d1d..723fcd7183 100644 --- a/src/pyupgrade/plugins/type_of_primitive.rs +++ b/src/pyupgrade/plugins/type_of_primitive.rs @@ -1,15 +1,13 @@ use rustpython_ast::Expr; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::pyupgrade::checks; pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { - if let Some(mut check) = - checks::type_of_primitive(func, args, checker.locate_check(Range::from_located(expr))) - { + if let Some(mut check) = checks::type_of_primitive(func, args, Range::from_located(expr)) { if checker.patch() { if let CheckKind::TypeOfPrimitive(primitive) = &check.kind { check.amend(Fix::replacement( diff --git a/src/pyupgrade/plugins/unnecessary_abspath.rs b/src/pyupgrade/plugins/unnecessary_abspath.rs index ede508bc10..9ccc90f5ac 100644 --- a/src/pyupgrade/plugins/unnecessary_abspath.rs +++ b/src/pyupgrade/plugins/unnecessary_abspath.rs @@ -1,14 +1,12 @@ use rustpython_ast::Expr; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::pyupgrade::checks; pub fn unnecessary_abspath(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { - if let Some(mut check) = - checks::unnecessary_abspath(func, args, checker.locate_check(Range::from_located(expr))) - { + if let Some(mut check) = checks::unnecessary_abspath(func, args, Range::from_located(expr)) { if checker.patch() { check.amend(Fix::replacement( "__file__".to_string(), diff --git a/src/pyupgrade/plugins/useless_metaclass_type.rs b/src/pyupgrade/plugins/useless_metaclass_type.rs index f65a748806..bcff798545 100644 --- a/src/pyupgrade/plugins/useless_metaclass_type.rs +++ b/src/pyupgrade/plugins/useless_metaclass_type.rs @@ -1,17 +1,15 @@ use log::error; use rustpython_ast::{Expr, Stmt}; -use crate::ast::types::{CheckLocator, Range}; +use crate::ast::types::Range; use crate::autofix::helpers; use crate::check_ast::Checker; use crate::pyupgrade::checks; pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, targets: &[Expr]) { - if let Some(mut check) = checks::useless_metaclass_type( - targets, - value, - checker.locate_check(Range::from_located(stmt)), - ) { + if let Some(mut check) = + checks::useless_metaclass_type(targets, value, Range::from_located(stmt)) + { if checker.patch() { let context = checker.binding_context(); let deleted: Vec<&Stmt> = checker