Move bindings to an arena (#1147)

This commit is contained in:
Charlie Marsh 2022-12-08 19:48:00 -05:00 committed by GitHub
parent c1b1ac069e
commit 84fa64d98c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 150 additions and 144 deletions

View file

@ -3,10 +3,10 @@ use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
use rustpython_parser::lexer; use rustpython_parser::lexer;
use rustpython_parser::lexer::Tok; 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. /// Extract the names bound to a given __all__ assignment.
pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec<String> { pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Vec<String> {
fn add_to_names(names: &mut Vec<String>, elts: &[Expr]) { fn add_to_names(names: &mut Vec<String>, elts: &[Expr]) {
for elt in elts { for elt in elts {
if let ExprKind::Constant { if let ExprKind::Constant {
@ -23,8 +23,8 @@ pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec<String> {
// Grab the existing bound __all__ values. // Grab the existing bound __all__ values.
if let StmtKind::AugAssign { .. } = &stmt.node { if let StmtKind::AugAssign { .. } = &stmt.node {
if let Some(binding) = scope.values.get("__all__") { if let Some(index) = scope.values.get("__all__") {
if let BindingKind::Export(existing) = &binding.kind { if let BindingKind::Export(existing) = &bindings[*index].kind {
names.extend_from_slice(existing); names.extend_from_slice(existing);
} }
} }

View file

@ -73,7 +73,7 @@ pub struct Scope<'a> {
pub kind: ScopeKind<'a>, pub kind: ScopeKind<'a>,
pub import_starred: bool, pub import_starred: bool,
pub uses_locals: bool, pub uses_locals: bool,
pub values: FxHashMap<&'a str, Binding>, pub values: FxHashMap<&'a str, usize>,
} }
impl<'a> Scope<'a> { impl<'a> Scope<'a> {

View file

@ -66,6 +66,7 @@ pub struct Checker<'a> {
// at various points in time. // at various points in time.
pub(crate) parents: Vec<&'a Stmt>, pub(crate) parents: Vec<&'a Stmt>,
pub(crate) parent_stack: Vec<usize>, pub(crate) parent_stack: Vec<usize>,
pub(crate) bindings: Vec<Binding>,
scopes: Vec<Scope<'a>>, scopes: Vec<Scope<'a>>,
scope_stack: Vec<usize>, scope_stack: Vec<usize>,
dead_scopes: Vec<usize>, dead_scopes: Vec<usize>,
@ -110,6 +111,7 @@ impl<'a> Checker<'a> {
import_aliases: FxHashMap::default(), import_aliases: FxHashMap::default(),
parents: vec![], parents: vec![],
parent_stack: vec![], parent_stack: vec![],
bindings: vec![],
scopes: vec![], scopes: vec![],
scope_stack: vec![], scope_stack: vec![],
dead_scopes: vec![], dead_scopes: vec![],
@ -188,8 +190,8 @@ impl<'a> Checker<'a> {
pub fn is_builtin(&self, member: &str) -> bool { pub fn is_builtin(&self, member: &str) -> bool {
self.current_scopes() self.current_scopes()
.find_map(|scope| scope.values.get(member)) .find_map(|scope| scope.values.get(member))
.map_or(false, |binding| { .map_or(false, |index| {
matches!(binding.kind, BindingKind::Builtin) matches!(self.bindings[*index].kind, BindingKind::Builtin)
}) })
} }
} }
@ -242,23 +244,21 @@ where
let usage = Some((scope.id, Range::from_located(stmt))); let usage = Some((scope.id, Range::from_located(stmt)));
for name in names { for name in names {
// Add a binding to the current scope. // Add a binding to the current scope.
scope.values.insert( let index = self.bindings.len();
name, self.bindings.push(Binding {
Binding { kind: BindingKind::Global,
kind: BindingKind::Global, used: usage,
used: usage, range: Range::from_located(stmt),
range: Range::from_located(stmt), });
}, scope.values.insert(name, index);
);
} }
// Mark the binding in the global scope as used. // Mark the binding in the global scope as used.
for name in names { for name in names {
if let Some(mut existing) = self.scopes[GLOBAL_SCOPE_INDEX] if let Some(index) =
.values self.scopes[GLOBAL_SCOPE_INDEX].values.get(&name.as_str())
.get_mut(&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))); let usage = Some((scope.id, Range::from_located(stmt)));
for name in names { for name in names {
// Add a binding to the current scope. // Add a binding to the current scope.
scope.values.insert( let index = self.bindings.len();
name, self.bindings.push(Binding {
Binding { kind: BindingKind::Nonlocal,
kind: BindingKind::Global, used: usage,
used: usage, range: Range::from_located(stmt),
range: Range::from_located(stmt), });
}, scope.values.insert(name, index);
);
} }
// Mark the binding in the defining scopes as used too. (Skip the global scope // Mark the binding in the defining scopes as used too. (Skip the global scope
// and the current scope.) // and the current scope.)
for name in names { for name in names {
for index in self.scope_stack.iter().skip(1).rev().skip(1) { for index in self.scope_stack.iter().skip(1).rev().skip(1) {
if let Some(mut existing) = if let Some(index) = self.scopes[*index].values.get(&name.as_str()) {
self.scopes[*index].values.get_mut(&name.as_str()) self.bindings[*index].used = usage;
{
existing.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( self.handle_node_store(
name, name,
&Expr::new( &Expr::new(
@ -2410,12 +2407,12 @@ where
walk_excepthandler(self, excepthandler); walk_excepthandler(self, excepthandler);
if let Some(binding) = { if let Some(index) = {
let scope = &mut self.scopes let scope = &mut self.scopes
[*(self.scope_stack.last().expect("No current scope found"))]; [*(self.scope_stack.last().expect("No current scope found"))];
&scope.values.remove(&name.as_str()) &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) { if self.settings.enabled.contains(&CheckCode::F841) {
self.add_check(Check::new( self.add_check(Check::new(
CheckKind::UnusedVariable(name.to_string()), 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 let scope = &mut self.scopes
[*(self.scope_stack.last().expect("No current scope found"))]; [*(self.scope_stack.last().expect("No current scope found"))];
scope.values.insert(name, binding); scope.values.insert(name, index);
} }
} }
None => walk_excepthandler(self, excepthandler), 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> { impl<'a> Checker<'a> {
fn push_parent(&mut self, parent: &'a Stmt) { fn push_parent(&mut self, parent: &'a Stmt) {
self.parent_stack.push(self.parents.len()); self.parent_stack.push(self.parents.len());
@ -2580,26 +2535,14 @@ impl<'a> Checker<'a> {
fn bind_builtins(&mut self) { fn bind_builtins(&mut self) {
let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))];
for builtin in BUILTINS.iter().chain(MAGIC_GLOBALS.iter()) {
for builtin in BUILTINS { let index = self.bindings.len();
scope.values.insert( self.bindings.push(Binding {
builtin, kind: BindingKind::Builtin,
Binding { range: Range::default(),
kind: BindingKind::Builtin, used: None,
range: Range::default(), });
used: None, scope.values.insert(builtin, index);
},
);
}
for builtin in MAGIC_GLOBALS {
scope.values.insert(
builtin,
Binding {
kind: BindingKind::Builtin,
range: Range::default(),
used: None,
},
);
} }
} }
@ -2631,7 +2574,8 @@ impl<'a> Checker<'a> {
{ {
if self.settings.enabled.contains(&CheckCode::F402) { if self.settings.enabled.contains(&CheckCode::F402) {
let scope = self.current_scope(); 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) if matches!(binding.kind, BindingKind::LoopVar)
&& matches!( && matches!(
existing.kind, existing.kind,
@ -2658,15 +2602,18 @@ impl<'a> Checker<'a> {
let scope = self.current_scope(); let scope = self.current_scope();
let binding = match scope.values.get(&name) { let binding = match scope.values.get(&name) {
None => binding, None => binding,
Some(existing) => Binding { Some(index) => Binding {
kind: binding.kind, kind: binding.kind,
range: binding.range, 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"))]; 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) { fn handle_node_load(&mut self, expr: &Expr) {
@ -2677,7 +2624,7 @@ impl<'a> Checker<'a> {
let mut in_generator = false; let mut in_generator = false;
let mut import_starred = false; let mut import_starred = false;
for scope_index in self.scope_stack.iter().rev() { 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 matches!(scope.kind, ScopeKind::Class(_)) {
if id == "__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; return;
} }
@ -2701,7 +2678,7 @@ impl<'a> Checker<'a> {
let mut from_list = vec![]; let mut from_list = vec![];
for scope_index in self.scope_stack.iter().rev() { for scope_index in self.scope_stack.iter().rev() {
let scope = &self.scopes[*scope_index]; 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 { if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from( from_list.push(helpers::format_import_from(
level.as_ref(), level.as_ref(),
@ -2754,7 +2731,7 @@ impl<'a> Checker<'a> {
.iter() .iter()
.map(|index| &self.scopes[*index]) .map(|index| &self.scopes[*index])
.collect(); .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); self.add_check(check);
} }
} }
@ -2762,12 +2739,9 @@ impl<'a> Checker<'a> {
if self.settings.enabled.contains(&CheckCode::N806) { if self.settings.enabled.contains(&CheckCode::N806) {
if matches!(self.current_scope().kind, ScopeKind::Function(..)) { if matches!(self.current_scope().kind, ScopeKind::Function(..)) {
// Ignore globals. // Ignore globals.
if !self if !self.current_scope().values.get(id).map_or(false, |index| {
.current_scope() matches!(self.bindings[*index].kind, BindingKind::Global)
.values }) {
.get(id)
.map_or(false, |binding| matches!(binding.kind, BindingKind::Global))
{
pep8_naming::plugins::non_lowercase_variable_in_function( pep8_naming::plugins::non_lowercase_variable_in_function(
self, expr, parent, id, self, expr, parent, id,
); );
@ -2838,7 +2812,7 @@ impl<'a> Checker<'a> {
self.add_binding( self.add_binding(
id, id,
Binding { Binding {
kind: BindingKind::Export(extract_all_names(parent, current)), kind: BindingKind::Export(extract_all_names(parent, current, &self.bindings)),
used: None, used: None,
range: Range::from_located(expr), range: Range::from_located(expr),
}, },
@ -3027,6 +3001,7 @@ impl<'a> Checker<'a> {
self.add_checks( self.add_checks(
pyflakes::checks::unused_variables( pyflakes::checks::unused_variables(
&self.scopes[index], &self.scopes[index],
&self.bindings,
&self.settings.dummy_variable_rgx, &self.settings.dummy_variable_rgx,
) )
.into_iter(), .into_iter(),
@ -3045,6 +3020,7 @@ impl<'a> Checker<'a> {
.last() .last()
.expect("Expected parent scope above function scope")], .expect("Expected parent scope above function scope")],
&self.scopes[index], &self.scopes[index],
&self.bindings,
) )
.into_iter(), .into_iter(),
); );
@ -3062,7 +3038,10 @@ impl<'a> Checker<'a> {
let mut checks: Vec<Check> = vec![]; let mut checks: Vec<Check> = vec![];
for scope in self.dead_scopes.iter().map(|index| &self.scopes[*index]) { 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<Vec<&str>> = let all_names: Option<Vec<&str>> =
all_binding.and_then(|binding| match &binding.kind { all_binding.and_then(|binding| match &binding.kind {
BindingKind::Export(names) => Some(names.iter().map(String::as_str).collect()), 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(all_binding) = all_binding {
if let Some(names) = &all_names { if let Some(names) = &all_names {
let mut from_list = vec![]; 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 { if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from( from_list.push(helpers::format_import_from(
level.as_ref(), level.as_ref(),
@ -3125,7 +3105,8 @@ impl<'a> Checker<'a> {
let mut unused: BTreeMap<(usize, Option<usize>), Vec<UnusedImport>> = let mut unused: BTreeMap<(usize, Option<usize>), Vec<UnusedImport>> =
BTreeMap::new(); 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) let (BindingKind::Importation(_, full_name, context)
| BindingKind::SubmoduleImportation(_, full_name, context) | BindingKind::SubmoduleImportation(_, full_name, context)
| BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue; }; | BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue; };

View file

@ -17,12 +17,13 @@ use crate::{visibility, Check};
fn function( fn function(
argumentable: &Argumentable, argumentable: &Argumentable,
args: &Arguments, args: &Arguments,
bindings: &FxHashMap<&str, Binding>, values: &FxHashMap<&str, usize>,
bindings: &[Binding],
dummy_variable_rgx: &Regex, dummy_variable_rgx: &Regex,
) -> Vec<Check> { ) -> Vec<Check> {
let mut checks: Vec<Check> = vec![]; let mut checks: Vec<Check> = vec![];
for arg_name in collect_arg_names(args) { 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() if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Argument) && matches!(binding.kind, BindingKind::Argument)
&& !dummy_variable_rgx.is_match(arg_name) && !dummy_variable_rgx.is_match(arg_name)
@ -41,7 +42,8 @@ fn function(
fn method( fn method(
argumentable: &Argumentable, argumentable: &Argumentable,
args: &Arguments, args: &Arguments,
bindings: &FxHashMap<&str, Binding>, values: &FxHashMap<&str, usize>,
bindings: &[Binding],
dummy_variable_rgx: &Regex, dummy_variable_rgx: &Regex,
) -> Vec<Check> { ) -> Vec<Check> {
let mut checks: Vec<Check> = vec![]; let mut checks: Vec<Check> = vec![];
@ -54,7 +56,10 @@ fn method(
.chain(iter::once::<Option<&Arg>>(args.vararg.as_deref()).flatten()) .chain(iter::once::<Option<&Arg>>(args.vararg.as_deref()).flatten())
.chain(iter::once::<Option<&Arg>>(args.kwarg.as_deref()).flatten()) .chain(iter::once::<Option<&Arg>>(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() if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Argument) && matches!(binding.kind, BindingKind::Argument)
&& !dummy_variable_rgx.is_match(arg.node.arg.as_str()) && !dummy_variable_rgx.is_match(arg.node.arg.as_str())
@ -70,7 +75,12 @@ fn method(
} }
/// ARG001, ARG002, ARG003, ARG004, ARG005 /// ARG001, ARG002, ARG003, ARG004, ARG005
pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec<Check> { pub fn unused_arguments(
checker: &Checker,
parent: &Scope,
scope: &Scope,
bindings: &[Binding],
) -> Vec<Check> {
match &scope.kind { match &scope.kind {
ScopeKind::Function(FunctionDef { ScopeKind::Function(FunctionDef {
name, name,
@ -98,6 +108,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec
&Argumentable::Function, &Argumentable::Function,
args, args,
&scope.values, &scope.values,
bindings,
&checker.settings.dummy_variable_rgx, &checker.settings.dummy_variable_rgx,
) )
} else { } else {
@ -117,6 +128,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec
&Argumentable::Method, &Argumentable::Method,
args, args,
&scope.values, &scope.values,
bindings,
&checker.settings.dummy_variable_rgx, &checker.settings.dummy_variable_rgx,
) )
} else { } else {
@ -136,6 +148,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec
&Argumentable::ClassMethod, &Argumentable::ClassMethod,
args, args,
&scope.values, &scope.values,
bindings,
&checker.settings.dummy_variable_rgx, &checker.settings.dummy_variable_rgx,
) )
} else { } else {
@ -155,6 +168,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec
&Argumentable::StaticMethod, &Argumentable::StaticMethod,
args, args,
&scope.values, &scope.values,
bindings,
&checker.settings.dummy_variable_rgx, &checker.settings.dummy_variable_rgx,
) )
} else { } else {
@ -173,6 +187,7 @@ pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec
&Argumentable::Lambda, &Argumentable::Lambda,
args, args,
&scope.values, &scope.values,
bindings,
&checker.settings.dummy_variable_rgx, &checker.settings.dummy_variable_rgx,
) )
} else { } else {

View file

@ -7,7 +7,7 @@ use rustpython_parser::ast::{
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind, 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::checks::{Check, CheckKind};
use crate::pyflakes::cformat::CFormatSummary; use crate::pyflakes::cformat::CFormatSummary;
use crate::pyflakes::format::FormatSummary; use crate::pyflakes::format::FormatSummary;
@ -366,12 +366,12 @@ pub fn if_tuple(test: &Expr, location: Range) -> Option<Check> {
} }
/// F821 /// F821
pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option<Check> { pub fn undefined_local(name: &str, scopes: &[&Scope], bindings: &[Binding]) -> Option<Check> {
let current = &scopes.last().expect("No current scope found"); let current = &scopes.last().expect("No current scope found");
if matches!(current.kind, ScopeKind::Function(_)) && !current.values.contains_key(name) { if matches!(current.kind, ScopeKind::Function(_)) && !current.values.contains_key(name) {
for scope in scopes.iter().rev().skip(1) { for scope in scopes.iter().rev().skip(1) {
if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) { 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 let Some((scope_id, location)) = binding.used {
if scope_id == current.id { if scope_id == current.id {
return Some(Check::new( return Some(Check::new(
@ -388,23 +388,31 @@ pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option<Check> {
} }
/// F841 /// F841
pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec<Check> { pub fn unused_variables(
scope: &Scope,
bindings: &[Binding],
dummy_variable_rgx: &Regex,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![]; let mut checks: Vec<Check> = vec![];
if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) { if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) {
return checks; 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() if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Assignment) && matches!(binding.kind, BindingKind::Assignment)
&& !dummy_variable_rgx.is_match(name) && !dummy_variable_rgx.is_match(name)
&& name != "__tracebackhide__" && name != &"__tracebackhide__"
&& name != "__traceback_info__" && name != &"__traceback_info__"
&& name != "__traceback_supplement__" && name != &"__traceback_supplement__"
{ {
checks.push(Check::new( checks.push(Check::new(
CheckKind::UnusedVariable(name.to_string()), CheckKind::UnusedVariable((*name).to_string()),
binding.range, binding.range,
)); ));
} }

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind}; use rustpython_ast::{Expr, ExprKind};
use crate::ast::types::{Binding, BindingKind, Range}; use crate::ast::types::Range;
use crate::check_ast::Checker; use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
@ -12,12 +12,7 @@ pub fn invalid_print_syntax(checker: &mut Checker, left: &Expr) {
if id != "print" { if id != "print" {
return; return;
} }
let scope = checker.current_scope(); if !checker.is_builtin("print") {
let Some(Binding {
kind: BindingKind::Builtin,
..
}) = scope.values.get("print") else
{
return; return;
}; };
checker.add_check(Check::new( checker.add_check(Check::new(

View file

@ -9,8 +9,8 @@ use crate::checks::{Check, CheckKind};
/// sys import *`). /// sys import *`).
fn is_module_star_imported(checker: &Checker, module: &str) -> bool { fn is_module_star_imported(checker: &Checker, module: &str) -> bool {
checker.current_scopes().any(|scope| { checker.current_scopes().any(|scope| {
scope.values.values().any(|binding| { scope.values.values().any(|index| {
if let BindingKind::StarImportation(_, name) = &binding.kind { if let BindingKind::StarImportation(_, name) = &checker.bindings[*index].kind {
name.as_ref().map(|name| name == module).unwrap_or_default() name.as_ref().map(|name| name == module).unwrap_or_default()
} else { } else {
false false
@ -26,7 +26,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -
scope scope
.values .values
.values() .values()
.find_map(|binding| match &binding.kind { .find_map(|index| match &checker.bindings[*index].kind {
// e.g. module=sys object=exit // e.g. module=sys object=exit
// `import sys` -> `sys.exit` // `import sys` -> `sys.exit`
// `import sys as sys2` -> `sys2.exit` // `import sys as sys2` -> `sys2.exit`

View file

@ -101,7 +101,12 @@ pub fn useless_metaclass_type(targets: &[Expr], value: &Expr, location: Range) -
} }
/// UP004 /// UP004
pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) -> Option<Check> { pub fn useless_object_inheritance(
name: &str,
bases: &[Expr],
scope: &Scope,
bindings: &[Binding],
) -> Option<Check> {
for expr in bases { for expr in bases {
let ExprKind::Name { id, .. } = &expr.node else { let ExprKind::Name { id, .. } = &expr.node else {
continue; continue;
@ -110,7 +115,10 @@ pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) ->
continue; continue;
} }
if !matches!( if !matches!(
scope.values.get(&id.as_str()), scope
.values
.get(&id.as_str())
.map(|index| &bindings[*index]),
None | Some(Binding { None | Some(Binding {
kind: BindingKind::Builtin, kind: BindingKind::Builtin,
.. ..

View file

@ -12,8 +12,7 @@ pub fn useless_object_inheritance(
bases: &[Expr], bases: &[Expr],
keywords: &[Keyword], keywords: &[Keyword],
) { ) {
let scope = checker.current_scope(); let Some(mut check) = checks::useless_object_inheritance(name, bases, checker.current_scope(), &checker.bindings) else {
let Some(mut check) = checks::useless_object_inheritance(name, bases, scope) else {
return; return;
}; };
if checker.patch(check.kind.code()) { if checker.patch(check.kind.code()) {