diff --git a/src/ast/operations.rs b/src/ast/operations.rs index 9fcf5f5f01..798cfa07cd 100644 --- a/src/ast/operations.rs +++ b/src/ast/operations.rs @@ -3,10 +3,10 @@ use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; use rustpython_parser::lexer; use rustpython_parser::lexer::Tok; -use crate::ast::types::{BindingKind, Scope}; +use crate::ast::types::{Binding, BindingKind, Scope}; /// Extract the names bound to a given __all__ assignment. -pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec { +pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Vec { fn add_to_names(names: &mut Vec, elts: &[Expr]) { for elt in elts { if let ExprKind::Constant { @@ -23,8 +23,8 @@ pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec { // Grab the existing bound __all__ values. if let StmtKind::AugAssign { .. } = &stmt.node { - if let Some(binding) = scope.values.get("__all__") { - if let BindingKind::Export(existing) = &binding.kind { + if let Some(index) = scope.values.get("__all__") { + if let BindingKind::Export(existing) = &bindings[*index].kind { names.extend_from_slice(existing); } } diff --git a/src/ast/types.rs b/src/ast/types.rs index 2de88c5ce8..2bcbace0af 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -73,7 +73,7 @@ pub struct Scope<'a> { pub kind: ScopeKind<'a>, pub import_starred: bool, pub uses_locals: bool, - pub values: FxHashMap<&'a str, Binding>, + pub values: FxHashMap<&'a str, usize>, } impl<'a> Scope<'a> { diff --git a/src/check_ast.rs b/src/check_ast.rs index 7881897a45..a0d7aac201 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -66,6 +66,7 @@ pub struct Checker<'a> { // at various points in time. pub(crate) parents: Vec<&'a Stmt>, pub(crate) parent_stack: Vec, + pub(crate) bindings: Vec, scopes: Vec>, scope_stack: Vec, dead_scopes: Vec, @@ -110,6 +111,7 @@ impl<'a> Checker<'a> { import_aliases: FxHashMap::default(), parents: vec![], parent_stack: vec![], + bindings: vec![], scopes: vec![], scope_stack: vec![], dead_scopes: vec![], @@ -188,8 +190,8 @@ impl<'a> Checker<'a> { pub fn is_builtin(&self, member: &str) -> bool { self.current_scopes() .find_map(|scope| scope.values.get(member)) - .map_or(false, |binding| { - matches!(binding.kind, BindingKind::Builtin) + .map_or(false, |index| { + matches!(self.bindings[*index].kind, BindingKind::Builtin) }) } } @@ -242,23 +244,21 @@ where let usage = Some((scope.id, Range::from_located(stmt))); for name in names { // Add a binding to the current scope. - scope.values.insert( - name, - Binding { - kind: BindingKind::Global, - used: usage, - range: Range::from_located(stmt), - }, - ); + let index = self.bindings.len(); + self.bindings.push(Binding { + kind: BindingKind::Global, + used: usage, + range: Range::from_located(stmt), + }); + scope.values.insert(name, index); } // Mark the binding in the global scope as used. for name in names { - if let Some(mut existing) = self.scopes[GLOBAL_SCOPE_INDEX] - .values - .get_mut(&name.as_str()) + if let Some(index) = + self.scopes[GLOBAL_SCOPE_INDEX].values.get(&name.as_str()) { - existing.used = usage; + self.bindings[*index].used = usage; } } } @@ -282,24 +282,21 @@ where let usage = Some((scope.id, Range::from_located(stmt))); for name in names { // Add a binding to the current scope. - scope.values.insert( - name, - Binding { - kind: BindingKind::Global, - used: usage, - range: Range::from_located(stmt), - }, - ); + let index = self.bindings.len(); + self.bindings.push(Binding { + kind: BindingKind::Nonlocal, + used: usage, + range: Range::from_located(stmt), + }); + scope.values.insert(name, index); } // Mark the binding in the defining scopes as used too. (Skip the global scope // and the current scope.) for name in names { for index in self.scope_stack.iter().skip(1).rev().skip(1) { - if let Some(mut existing) = - self.scopes[*index].values.get_mut(&name.as_str()) - { - existing.used = usage; + if let Some(index) = self.scopes[*index].values.get(&name.as_str()) { + self.bindings[*index].used = usage; } } } @@ -2394,7 +2391,7 @@ where ); } - let definition = self.current_scope().values.get(&name.as_str()).cloned(); + let definition = self.current_scope().values.get(&name.as_str()).copied(); self.handle_node_store( name, &Expr::new( @@ -2410,12 +2407,12 @@ where walk_excepthandler(self, excepthandler); - if let Some(binding) = { + if let Some(index) = { let scope = &mut self.scopes [*(self.scope_stack.last().expect("No current scope found"))]; &scope.values.remove(&name.as_str()) } { - if binding.used.is_none() { + if self.bindings[*index].used.is_none() { if self.settings.enabled.contains(&CheckCode::F841) { self.add_check(Check::new( CheckKind::UnusedVariable(name.to_string()), @@ -2425,10 +2422,10 @@ where } } - if let Some(binding) = definition { + if let Some(index) = definition { let scope = &mut self.scopes [*(self.scope_stack.last().expect("No current scope found"))]; - scope.values.insert(name, binding); + scope.values.insert(name, index); } } None => walk_excepthandler(self, excepthandler), @@ -2511,48 +2508,6 @@ where } } -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. - binding.used = Some((scope_id, Range::from_located(expr))); - - // If the name of the sub-importation is the same as an alias of another - // importation and the alias is used, that sub-importation should be - // marked as used too. - // - // This handles code like: - // import pyarrow as pa - // import pyarrow.csv - // print(pa.csv.read_csv("test.csv")) - if let BindingKind::Importation(name, full_name, _) - | BindingKind::FromImportation(name, full_name, _) - | BindingKind::SubmoduleImportation(name, full_name, _) = &binding.kind - { - let has_alias = full_name - .split('.') - .last() - .map(|segment| segment != name) - .unwrap_or_default(); - if has_alias { - // Clone the alias. (We'll mutate it below.) - full_name.to_string() - } else { - return true; - } - } else { - return true; - } - } else { - return false; - }; - - // Mark the sub-importation as used. - if let Some(binding) = scope.values.get_mut(alias.as_str()) { - binding.used = Some((scope_id, Range::from_located(expr))); - } - true -} - impl<'a> Checker<'a> { fn push_parent(&mut self, parent: &'a Stmt) { self.parent_stack.push(self.parents.len()); @@ -2580,26 +2535,14 @@ impl<'a> Checker<'a> { fn bind_builtins(&mut self) { let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; - - for builtin in BUILTINS { - scope.values.insert( - builtin, - Binding { - kind: BindingKind::Builtin, - range: Range::default(), - used: None, - }, - ); - } - for builtin in MAGIC_GLOBALS { - scope.values.insert( - builtin, - Binding { - kind: BindingKind::Builtin, - range: Range::default(), - used: None, - }, - ); + for builtin in BUILTINS.iter().chain(MAGIC_GLOBALS.iter()) { + let index = self.bindings.len(); + self.bindings.push(Binding { + kind: BindingKind::Builtin, + range: Range::default(), + used: None, + }); + scope.values.insert(builtin, index); } } @@ -2631,7 +2574,8 @@ impl<'a> Checker<'a> { { if self.settings.enabled.contains(&CheckCode::F402) { let scope = self.current_scope(); - if let Some(existing) = scope.values.get(&name) { + if let Some(index) = scope.values.get(&name) { + let existing = &self.bindings[*index]; if matches!(binding.kind, BindingKind::LoopVar) && matches!( existing.kind, @@ -2658,15 +2602,18 @@ impl<'a> Checker<'a> { let scope = self.current_scope(); let binding = match scope.values.get(&name) { None => binding, - Some(existing) => Binding { + Some(index) => Binding { kind: binding.kind, range: binding.range, - used: existing.used, + used: self.bindings[*index].used, }, }; + let index = self.bindings.len(); + self.bindings.push(binding); + let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; - scope.values.insert(name, binding); + scope.values.insert(name, index); } fn handle_node_load(&mut self, expr: &Expr) { @@ -2677,7 +2624,7 @@ impl<'a> Checker<'a> { let mut in_generator = false; let mut import_starred = false; for scope_index in self.scope_stack.iter().rev() { - let scope = &mut self.scopes[*scope_index]; + let scope = &self.scopes[*scope_index]; if matches!(scope.kind, ScopeKind::Class(_)) { if id == "__class__" { @@ -2687,7 +2634,37 @@ impl<'a> Checker<'a> { } } - if try_mark_used(scope, scope_id, id, expr) { + if let Some(index) = scope.values.get(&id.as_str()) { + // Mark the binding as used. + self.bindings[*index].used = Some((scope_id, Range::from_located(expr))); + + // If the name of the sub-importation is the same as an alias of another + // importation and the alias is used, that sub-importation should be + // marked as used too. + // + // This handles code like: + // import pyarrow as pa + // import pyarrow.csv + // print(pa.csv.read_csv("test.csv")) + if let BindingKind::Importation(name, full_name, _) + | BindingKind::FromImportation(name, full_name, _) + | BindingKind::SubmoduleImportation(name, full_name, _) = + &self.bindings[*index].kind + { + let has_alias = full_name + .split('.') + .last() + .map(|segment| segment != name) + .unwrap_or_default(); + if has_alias { + // Mark the sub-importation as used. + if let Some(index) = scope.values.get(full_name.as_str()) { + self.bindings[*index].used = + Some((scope_id, Range::from_located(expr))); + } + } + } + return; } @@ -2701,7 +2678,7 @@ impl<'a> Checker<'a> { let mut from_list = vec![]; for scope_index in self.scope_stack.iter().rev() { let scope = &self.scopes[*scope_index]; - for binding in scope.values.values() { + for binding in scope.values.values().map(|index| &self.bindings[*index]) { if let BindingKind::StarImportation(level, module) = &binding.kind { from_list.push(helpers::format_import_from( level.as_ref(), @@ -2754,7 +2731,7 @@ impl<'a> Checker<'a> { .iter() .map(|index| &self.scopes[*index]) .collect(); - if let Some(check) = pyflakes::checks::undefined_local(&scopes, id) { + if let Some(check) = pyflakes::checks::undefined_local(id, &scopes, &self.bindings) { self.add_check(check); } } @@ -2762,12 +2739,9 @@ impl<'a> Checker<'a> { if self.settings.enabled.contains(&CheckCode::N806) { if matches!(self.current_scope().kind, ScopeKind::Function(..)) { // Ignore globals. - if !self - .current_scope() - .values - .get(id) - .map_or(false, |binding| matches!(binding.kind, BindingKind::Global)) - { + if !self.current_scope().values.get(id).map_or(false, |index| { + matches!(self.bindings[*index].kind, BindingKind::Global) + }) { pep8_naming::plugins::non_lowercase_variable_in_function( self, expr, parent, id, ); @@ -2838,7 +2812,7 @@ impl<'a> Checker<'a> { self.add_binding( id, Binding { - kind: BindingKind::Export(extract_all_names(parent, current)), + kind: BindingKind::Export(extract_all_names(parent, current, &self.bindings)), used: None, range: Range::from_located(expr), }, @@ -3027,6 +3001,7 @@ impl<'a> Checker<'a> { self.add_checks( pyflakes::checks::unused_variables( &self.scopes[index], + &self.bindings, &self.settings.dummy_variable_rgx, ) .into_iter(), @@ -3045,6 +3020,7 @@ impl<'a> Checker<'a> { .last() .expect("Expected parent scope above function scope")], &self.scopes[index], + &self.bindings, ) .into_iter(), ); @@ -3062,7 +3038,10 @@ impl<'a> Checker<'a> { let mut checks: Vec = vec![]; for scope in self.dead_scopes.iter().map(|index| &self.scopes[*index]) { - let all_binding: Option<&Binding> = scope.values.get("__all__"); + let all_binding: Option<&Binding> = scope + .values + .get("__all__") + .map(|index| &self.bindings[*index]); let all_names: Option> = all_binding.and_then(|binding| match &binding.kind { BindingKind::Export(names) => Some(names.iter().map(String::as_str).collect()), @@ -3091,7 +3070,8 @@ impl<'a> Checker<'a> { if let Some(all_binding) = all_binding { if let Some(names) = &all_names { let mut from_list = vec![]; - for binding in scope.values.values() { + for binding in scope.values.values().map(|index| &self.bindings[*index]) + { if let BindingKind::StarImportation(level, module) = &binding.kind { from_list.push(helpers::format_import_from( level.as_ref(), @@ -3125,7 +3105,8 @@ impl<'a> Checker<'a> { let mut unused: BTreeMap<(usize, Option), Vec> = BTreeMap::new(); - for (name, binding) in &scope.values { + for (name, index) in &scope.values { + let binding = &self.bindings[*index]; let (BindingKind::Importation(_, full_name, context) | BindingKind::SubmoduleImportation(_, full_name, context) | BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue; }; diff --git a/src/flake8_unused_arguments/plugins.rs b/src/flake8_unused_arguments/plugins.rs index 0ecfba0eb0..912bb9f161 100644 --- a/src/flake8_unused_arguments/plugins.rs +++ b/src/flake8_unused_arguments/plugins.rs @@ -17,12 +17,13 @@ use crate::{visibility, Check}; fn function( argumentable: &Argumentable, args: &Arguments, - bindings: &FxHashMap<&str, Binding>, + values: &FxHashMap<&str, usize>, + bindings: &[Binding], dummy_variable_rgx: &Regex, ) -> Vec { let mut checks: Vec = vec![]; for arg_name in collect_arg_names(args) { - if let Some(binding) = bindings.get(arg_name) { + if let Some(binding) = values.get(arg_name).map(|index| &bindings[*index]) { if binding.used.is_none() && matches!(binding.kind, BindingKind::Argument) && !dummy_variable_rgx.is_match(arg_name) @@ -41,7 +42,8 @@ fn function( fn method( argumentable: &Argumentable, args: &Arguments, - bindings: &FxHashMap<&str, Binding>, + values: &FxHashMap<&str, usize>, + bindings: &[Binding], dummy_variable_rgx: &Regex, ) -> Vec { let mut checks: Vec = vec![]; @@ -54,7 +56,10 @@ fn method( .chain(iter::once::>(args.vararg.as_deref()).flatten()) .chain(iter::once::>(args.kwarg.as_deref()).flatten()) { - if let Some(binding) = bindings.get(&arg.node.arg.as_str()) { + if let Some(binding) = values + .get(&arg.node.arg.as_str()) + .map(|index| &bindings[*index]) + { if binding.used.is_none() && matches!(binding.kind, BindingKind::Argument) && !dummy_variable_rgx.is_match(arg.node.arg.as_str()) @@ -70,7 +75,12 @@ fn method( } /// ARG001, ARG002, ARG003, ARG004, ARG005 -pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec { +pub fn unused_arguments( + checker: &Checker, + parent: &Scope, + scope: &Scope, + bindings: &[Binding], +) -> Vec { match &scope.kind { ScopeKind::Function(FunctionDef { name, @@ -98,6 +108,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec &Argumentable::Function, args, &scope.values, + bindings, &checker.settings.dummy_variable_rgx, ) } else { @@ -117,6 +128,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec &Argumentable::Method, args, &scope.values, + bindings, &checker.settings.dummy_variable_rgx, ) } else { @@ -136,6 +148,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec &Argumentable::ClassMethod, args, &scope.values, + bindings, &checker.settings.dummy_variable_rgx, ) } else { @@ -155,6 +168,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec &Argumentable::StaticMethod, args, &scope.values, + bindings, &checker.settings.dummy_variable_rgx, ) } else { @@ -173,6 +187,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec &Argumentable::Lambda, args, &scope.values, + bindings, &checker.settings.dummy_variable_rgx, ) } else { diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index eea18e7804..a3aac7ad13 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -7,7 +7,7 @@ use rustpython_parser::ast::{ Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind, }; -use crate::ast::types::{BindingKind, Range, Scope, ScopeKind}; +use crate::ast::types::{Binding, BindingKind, Range, Scope, ScopeKind}; use crate::checks::{Check, CheckKind}; use crate::pyflakes::cformat::CFormatSummary; use crate::pyflakes::format::FormatSummary; @@ -366,12 +366,12 @@ pub fn if_tuple(test: &Expr, location: Range) -> Option { } /// F821 -pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option { +pub fn undefined_local(name: &str, scopes: &[&Scope], bindings: &[Binding]) -> 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(binding) = scope.values.get(name).map(|index| &bindings[*index]) { if let Some((scope_id, location)) = binding.used { if scope_id == current.id { return Some(Check::new( @@ -388,23 +388,31 @@ pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option { } /// F841 -pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec { +pub fn unused_variables( + scope: &Scope, + bindings: &[Binding], + dummy_variable_rgx: &Regex, +) -> Vec { let mut checks: Vec = vec![]; if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) { return checks; } - for (&name, binding) in &scope.values { + for (name, binding) in scope + .values + .iter() + .map(|(name, index)| (name, &bindings[*index])) + { if binding.used.is_none() && matches!(binding.kind, BindingKind::Assignment) && !dummy_variable_rgx.is_match(name) - && name != "__tracebackhide__" - && name != "__traceback_info__" - && name != "__traceback_supplement__" + && name != &"__tracebackhide__" + && name != &"__traceback_info__" + && name != &"__traceback_supplement__" { checks.push(Check::new( - CheckKind::UnusedVariable(name.to_string()), + CheckKind::UnusedVariable((*name).to_string()), binding.range, )); } diff --git a/src/pyflakes/plugins/invalid_print_syntax.rs b/src/pyflakes/plugins/invalid_print_syntax.rs index f05968983a..6c8ef5483f 100644 --- a/src/pyflakes/plugins/invalid_print_syntax.rs +++ b/src/pyflakes/plugins/invalid_print_syntax.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::types::{Binding, BindingKind, Range}; +use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -12,12 +12,7 @@ pub fn invalid_print_syntax(checker: &mut Checker, left: &Expr) { if id != "print" { return; } - let scope = checker.current_scope(); - let Some(Binding { - kind: BindingKind::Builtin, - .. - }) = scope.values.get("print") else - { + if !checker.is_builtin("print") { return; }; checker.add_check(Check::new( diff --git a/src/pylint/plugins/use_sys_exit.rs b/src/pylint/plugins/use_sys_exit.rs index 29ede5576b..688661e14e 100644 --- a/src/pylint/plugins/use_sys_exit.rs +++ b/src/pylint/plugins/use_sys_exit.rs @@ -9,8 +9,8 @@ use crate::checks::{Check, CheckKind}; /// sys import *`). fn is_module_star_imported(checker: &Checker, module: &str) -> bool { checker.current_scopes().any(|scope| { - scope.values.values().any(|binding| { - if let BindingKind::StarImportation(_, name) = &binding.kind { + scope.values.values().any(|index| { + if let BindingKind::StarImportation(_, name) = &checker.bindings[*index].kind { name.as_ref().map(|name| name == module).unwrap_or_default() } else { false @@ -26,7 +26,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) - scope .values .values() - .find_map(|binding| match &binding.kind { + .find_map(|index| match &checker.bindings[*index].kind { // e.g. module=sys object=exit // `import sys` -> `sys.exit` // `import sys as sys2` -> `sys2.exit` diff --git a/src/pyupgrade/checks.rs b/src/pyupgrade/checks.rs index 0a504704cd..251838e39d 100644 --- a/src/pyupgrade/checks.rs +++ b/src/pyupgrade/checks.rs @@ -101,7 +101,12 @@ pub fn useless_metaclass_type(targets: &[Expr], value: &Expr, location: Range) - } /// UP004 -pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) -> Option { +pub fn useless_object_inheritance( + name: &str, + bases: &[Expr], + scope: &Scope, + bindings: &[Binding], +) -> Option { for expr in bases { let ExprKind::Name { id, .. } = &expr.node else { continue; @@ -110,7 +115,10 @@ pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) -> continue; } if !matches!( - scope.values.get(&id.as_str()), + scope + .values + .get(&id.as_str()) + .map(|index| &bindings[*index]), None | Some(Binding { kind: BindingKind::Builtin, .. diff --git a/src/pyupgrade/plugins/useless_object_inheritance.rs b/src/pyupgrade/plugins/useless_object_inheritance.rs index 81ec20697b..9880e83a25 100644 --- a/src/pyupgrade/plugins/useless_object_inheritance.rs +++ b/src/pyupgrade/plugins/useless_object_inheritance.rs @@ -12,8 +12,7 @@ pub fn useless_object_inheritance( bases: &[Expr], keywords: &[Keyword], ) { - let scope = checker.current_scope(); - let Some(mut check) = checks::useless_object_inheritance(name, bases, scope) else { + let Some(mut check) = checks::useless_object_inheritance(name, bases, checker.current_scope(), &checker.bindings) else { return; }; if checker.patch(check.kind.code()) {