From 92179e6369f9b46e57242c56d60bb5d5ee05d8e7 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 17 Mar 2023 17:12:27 +0100 Subject: [PATCH] Scope and Binding IDs (#3572) --- crates/ruff/src/checkers/ast/deferred.rs | 3 +- crates/ruff/src/checkers/ast/mod.rs | 296 +++++++----------- .../rules/cached_instance_method.rs | 2 +- .../rules/unused_loop_control_variable.rs | 2 +- .../flake8_simplify/rules/ast_unary_op.rs | 4 +- .../src/rules/flake8_type_checking/helpers.rs | 4 +- .../rules/flake8_unused_arguments/rules.rs | 17 +- .../pycodestyle/rules/lambda_assignment.rs | 2 +- .../pyflakes/rules/return_outside_function.rs | 2 +- .../rules/pyflakes/rules/undefined_local.rs | 5 +- .../rules/pyflakes/rules/unused_annotation.rs | 3 +- .../rules/pyflakes/rules/unused_variable.rs | 4 +- .../pyflakes/rules/yield_outside_function.rs | 2 +- crates/ruff/src/rules/pylint/helpers.rs | 4 +- .../rules/pylint/rules/await_outside_async.rs | 2 +- .../pylint/rules/consider_using_sys_exit.rs | 4 +- .../rules/pylint/rules/global_statement.rs | 2 +- .../rules/used_prior_global_declaration.rs | 2 +- .../rules/super_call_with_parameters.rs | 2 +- .../rules/useless_object_inheritance.rs | 5 +- crates/ruff_python_ast/src/context.rs | 199 ++++++++++-- crates/ruff_python_ast/src/types.rs | 88 +++++- 22 files changed, 404 insertions(+), 250 deletions(-) diff --git a/crates/ruff/src/checkers/ast/deferred.rs b/crates/ruff/src/checkers/ast/deferred.rs index 5e643499ed974..ccef124bd3282 100644 --- a/crates/ruff/src/checkers/ast/deferred.rs +++ b/crates/ruff/src/checkers/ast/deferred.rs @@ -1,3 +1,4 @@ +use ruff_python_ast::context::ScopeStack; use rustpython_parser::ast::{Expr, Stmt}; use ruff_python_ast::types::Range; @@ -7,7 +8,7 @@ use ruff_python_ast::visibility::{Visibility, VisibleScope}; use crate::checkers::ast::AnnotationContext; use crate::docstrings::definition::Definition; -type Context<'a> = (Vec, Vec>); +type Context<'a> = (ScopeStack, Vec>); /// A collection of AST nodes that are deferred for later analysis. /// Used to, e.g., store functions, whose bodies shouldn't be analyzed until all diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 279387f783cca..80830531c9e1d 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -14,7 +14,7 @@ use rustpython_parser::ast::{ }; use ruff_diagnostics::Diagnostic; -use ruff_python_ast::context::Context; +use ruff_python_ast::context::{Context, ScopeStack}; use ruff_python_ast::helpers::{ binding_range, extract_handled_exceptions, to_module_path, Exceptions, }; @@ -22,8 +22,8 @@ use ruff_python_ast::operations::{extract_all_names, AllNamesFlags}; use ruff_python_ast::relocate::relocate_expr; use ruff_python_ast::source_code::{Indexer, Locator, Stylist}; use ruff_python_ast::types::{ - Binding, BindingKind, ClassDef, ExecutionContext, FunctionDef, Lambda, Node, Range, - RefEquality, Scope, ScopeKind, + Binding, BindingId, BindingKind, ClassDef, ExecutionContext, FunctionDef, Lambda, Node, Range, + RefEquality, Scope, ScopeId, ScopeKind, }; use ruff_python_ast::typing::{match_annotated_subscript, Callable, SubscriptKind}; use ruff_python_ast::visitor::{walk_excepthandler, walk_pattern, Visitor}; @@ -53,8 +53,6 @@ use crate::{autofix, docstrings, noqa}; mod deferred; -const GLOBAL_SCOPE_INDEX: usize = 0; - type AnnotationContext = (bool, bool); #[allow(clippy::struct_excessive_bools)] @@ -193,16 +191,15 @@ where // Pre-visit. match &stmt.node { StmtKind::Global { names } => { - let scope_index = *self.ctx.scope_stack.last().expect("No current scope found"); + let scope_index = self.ctx.scope_id(); let ranges: Vec = helpers::find_names(stmt, self.locator).collect(); - if scope_index != GLOBAL_SCOPE_INDEX { + if !scope_index.is_global() { // Add the binding to the current scope. let context = self.ctx.execution_context(); let scope = &mut self.ctx.scopes[scope_index]; let usage = Some((scope.id, Range::from(stmt))); for (name, range) in names.iter().zip(ranges.iter()) { - let index = self.ctx.bindings.len(); - self.ctx.bindings.push(Binding { + let id = self.ctx.bindings.push(Binding { kind: BindingKind::Global, runtime_usage: None, synthetic_usage: usage, @@ -211,7 +208,7 @@ where source: Some(RefEquality(stmt)), context, }); - scope.bindings.insert(name, index); + scope.bindings.insert(name, id); } } @@ -223,16 +220,15 @@ where } } StmtKind::Nonlocal { names } => { - let scope_index = *self.ctx.scope_stack.last().expect("No current scope found"); + let scope_index = self.ctx.scope_id(); let ranges: Vec = helpers::find_names(stmt, self.locator).collect(); - if scope_index != GLOBAL_SCOPE_INDEX { + if !scope_index.is_global() { let context = self.ctx.execution_context(); let scope = &mut self.ctx.scopes[scope_index]; let usage = Some((scope.id, Range::from(stmt))); for (name, range) in names.iter().zip(ranges.iter()) { // Add a binding to the current scope. - let index = self.ctx.bindings.len(); - self.ctx.bindings.push(Binding { + let id = self.ctx.bindings.push(Binding { kind: BindingKind::Nonlocal, runtime_usage: None, synthetic_usage: usage, @@ -241,14 +237,18 @@ where source: Some(RefEquality(stmt)), context, }); - scope.bindings.insert(name, index); + scope.bindings.insert(name, id); } // Mark the binding in the defining scopes as used too. (Skip the global scope // and the current scope.) for (name, range) in names.iter().zip(ranges.iter()) { let mut exists = false; - for index in self.ctx.scope_stack.iter().skip(1).rev().skip(1) { + let mut scopes_iter = self.ctx.scope_stack.iter(); + // Skip the global scope + scopes_iter.next_back(); + + for index in scopes_iter.skip(1) { if let Some(index) = self.ctx.scopes[*index].bindings.get(&name.as_str()) { @@ -354,7 +354,7 @@ where if let Some(diagnostic) = pep8_naming::rules::invalid_first_argument_name_for_class_method( self, - self.ctx.current_scope(), + self.ctx.scope(), name, decorator_list, args, @@ -372,7 +372,7 @@ where if let Some(diagnostic) = pep8_naming::rules::invalid_first_argument_name_for_method( self, - self.ctx.current_scope(), + self.ctx.scope(), name, decorator_list, args, @@ -393,7 +393,7 @@ where if self.settings.rules.enabled(Rule::DunderFunctionName) { if let Some(diagnostic) = pep8_naming::rules::dunder_function_name( - self.ctx.current_scope(), + self.ctx.scope(), stmt, name, self.locator, @@ -592,7 +592,7 @@ where // See: https://docs.python.org/3/reference/simple_stmts.html#annotated-assignment-statements let runtime_annotation = !self.ctx.annotations_future_enabled && matches!( - self.ctx.current_scope().kind, + self.ctx.scope().kind, ScopeKind::Class(..) | ScopeKind::Module ); @@ -849,15 +849,7 @@ where kind: BindingKind::FutureImportation, runtime_usage: None, // Always mark `__future__` imports as used. - synthetic_usage: Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )), + synthetic_usage: Some((self.ctx.scope_id(), Range::from(alias))), typing_usage: None, range: Range::from(alias), source: Some(self.ctx.current_stmt().clone()), @@ -884,15 +876,7 @@ where kind: BindingKind::SubmoduleImportation(name, full_name), runtime_usage: None, synthetic_usage: if is_handled { - Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )) + Some((self.ctx.scope_id(), Range::from(alias))) } else { None }, @@ -922,15 +906,7 @@ where kind: BindingKind::Importation(name, full_name), runtime_usage: None, synthetic_usage: if is_handled || is_explicit_reexport { - Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )) + Some((self.ctx.scope_id(), Range::from(alias))) } else { None }, @@ -1191,15 +1167,7 @@ where kind: BindingKind::FutureImportation, runtime_usage: None, // Always mark `__future__` imports as used. - synthetic_usage: Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )), + synthetic_usage: Some((self.ctx.scope_id(), Range::from(alias))), typing_usage: None, range: Range::from(alias), source: Some(self.ctx.current_stmt().clone()), @@ -1230,15 +1198,7 @@ where kind: BindingKind::StarImportation(*level, module.clone()), runtime_usage: None, synthetic_usage: if is_handled { - Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )) + Some((self.ctx.scope_id(), Range::from(alias))) } else { None }, @@ -1250,8 +1210,7 @@ where ); if self.settings.rules.enabled(Rule::ImportStarNotPermitted) { - let scope = &self.ctx.scopes - [*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope(); if !matches!(scope.kind, ScopeKind::Module) { self.diagnostics.push(Diagnostic::new( pyflakes::rules::ImportStarNotPermitted { @@ -1277,8 +1236,7 @@ where )); } - let scope = &mut self.ctx.scopes - [*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); scope.import_starred = true; } else { if let Some(asname) = &alias.node.asname { @@ -1308,15 +1266,7 @@ where kind: BindingKind::FromImportation(name, full_name), runtime_usage: None, synthetic_usage: if is_handled || is_explicit_reexport { - Some(( - self.ctx.scopes[*(self - .ctx - .scope_stack - .last() - .expect("No current scope found"))] - .id, - Range::from(alias), - )) + Some((self.ctx.scope_id(), Range::from(alias))) } else { None }, @@ -1921,13 +1871,14 @@ where // If any global bindings don't already exist in the global scope, add it. let globals = operations::extract_globals(body); for (name, stmt) in operations::extract_globals(body) { - if self.ctx.scopes[GLOBAL_SCOPE_INDEX] + if self + .ctx + .global_scope() .bindings .get(name) .map_or(true, |index| self.ctx.bindings[*index].kind.is_annotation()) { - let index = self.ctx.bindings.len(); - self.ctx.bindings.push(Binding { + let id = self.ctx.bindings.push(Binding { kind: BindingKind::Assignment, runtime_usage: None, synthetic_usage: None, @@ -1936,21 +1887,18 @@ where source: Some(RefEquality(stmt)), context: self.ctx.execution_context(), }); - self.ctx.scopes[GLOBAL_SCOPE_INDEX] - .bindings - .insert(name, index); + self.ctx.global_scope_mut().bindings.insert(name, id); } } - self.ctx - .push_scope(Scope::new(ScopeKind::Function(FunctionDef { - name, - body, - args, - decorator_list, - async_: matches!(stmt.node, StmtKind::AsyncFunctionDef { .. }), - globals, - }))); + self.ctx.push_scope(ScopeKind::Function(FunctionDef { + name, + body, + args, + decorator_list, + async_: matches!(stmt.node, StmtKind::AsyncFunctionDef { .. }), + globals, + })); self.deferred.functions.push(( stmt, @@ -1986,13 +1934,14 @@ where // If any global bindings don't already exist in the global scope, add it. let globals = operations::extract_globals(body); for (name, stmt) in &globals { - if self.ctx.scopes[GLOBAL_SCOPE_INDEX] + if self + .ctx + .global_scope() .bindings .get(name) .map_or(true, |index| self.ctx.bindings[*index].kind.is_annotation()) { - let index = self.ctx.bindings.len(); - self.ctx.bindings.push(Binding { + let id = self.ctx.bindings.push(Binding { kind: BindingKind::Assignment, runtime_usage: None, synthetic_usage: None, @@ -2001,19 +1950,17 @@ where source: Some(RefEquality(stmt)), context: self.ctx.execution_context(), }); - self.ctx.scopes[GLOBAL_SCOPE_INDEX] - .bindings - .insert(name, index); + self.ctx.global_scope_mut().bindings.insert(name, id); } } - self.ctx.push_scope(Scope::new(ScopeKind::Class(ClassDef { + self.ctx.push_scope(ScopeKind::Class(ClassDef { name, bases, keywords, decorator_list, globals, - }))); + })); self.visit_body(body); } @@ -2074,7 +2021,7 @@ where // available at runtime. // See: https://docs.python.org/3/reference/simple_stmts.html#annotated-assignment-statements let runtime_annotation = if self.ctx.annotations_future_enabled { - if matches!(self.ctx.current_scope().kind, ScopeKind::Class(..)) { + if matches!(self.ctx.scope().kind, ScopeKind::Class(..)) { let baseclasses = &self .settings .flake8_type_checking @@ -2093,7 +2040,7 @@ where } } else { matches!( - self.ctx.current_scope().kind, + self.ctx.scope().kind, ScopeKind::Class(..) | ScopeKind::Module ) }; @@ -2718,8 +2665,7 @@ where } if let ExprKind::Name { id, ctx } = &func.node { if id == "locals" && matches!(ctx, ExprContext::Load) { - let scope = &mut self.ctx.scopes - [*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); scope.uses_locals = true; } } @@ -3365,7 +3311,7 @@ where self.visit_expr(expr); } self.ctx - .push_scope(Scope::new(ScopeKind::Lambda(Lambda { args, body }))); + .push_scope(ScopeKind::Lambda(Lambda { args, body })); } ExprKind::IfExp { test, body, orelse } => { if self.settings.rules.enabled(Rule::IfExprWithTrueFalse) { @@ -3391,13 +3337,13 @@ where if self.settings.rules.enabled(Rule::FunctionUsesLoopVariable) { flake8_bugbear::rules::function_uses_loop_variable(self, &Node::Expr(expr)); } - self.ctx.push_scope(Scope::new(ScopeKind::Generator)); + self.ctx.push_scope(ScopeKind::Generator); } ExprKind::GeneratorExp { .. } | ExprKind::DictComp { .. } => { if self.settings.rules.enabled(Rule::FunctionUsesLoopVariable) { flake8_bugbear::rules::function_uses_loop_variable(self, &Node::Expr(expr)); } - self.ctx.push_scope(Scope::new(ScopeKind::Generator)); + self.ctx.push_scope(ScopeKind::Generator); } ExprKind::BoolOp { op, values } => { if self.settings.rules.enabled(Rule::ConsiderMergingIsinstance) { @@ -3767,12 +3713,7 @@ where let name_range = helpers::excepthandler_name_range(excepthandler, self.locator).unwrap(); - if self - .ctx - .current_scope() - .bindings - .contains_key(&name.as_str()) - { + if self.ctx.scope().bindings.contains_key(&name.as_str()) { self.handle_node_store( name, &Expr::new( @@ -3786,12 +3727,7 @@ where ); } - let definition = self - .ctx - .current_scope() - .bindings - .get(&name.as_str()) - .copied(); + let definition = self.ctx.scope().bindings.get(&name.as_str()).copied(); self.handle_node_store( name, &Expr::new( @@ -3807,8 +3743,7 @@ where walk_excepthandler(self, excepthandler); if let Some(index) = { - let scope = &mut self.ctx.scopes - [*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); &scope.bindings.remove(&name.as_str()) } { if !self.ctx.bindings[*index].used() { @@ -3842,8 +3777,7 @@ where } if let Some(index) = definition { - let scope = &mut self.ctx.scopes - [*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); scope.bindings.insert(name, index); } } @@ -3998,18 +3932,16 @@ impl<'a> Checker<'a> { where 'b: 'a, { - let binding_index = self.ctx.bindings.len(); - + let binding_id = self.ctx.bindings.next_id(); if let Some((stack_index, scope_index)) = self .ctx .scope_stack .iter() - .rev() .enumerate() .find(|(_, scope_index)| self.ctx.scopes[**scope_index].bindings.contains_key(&name)) { - let existing_binding_index = self.ctx.scopes[*scope_index].bindings.get(&name).unwrap(); - let existing = &self.ctx.bindings[*existing_binding_index]; + let existing_binding_index = self.ctx.scopes[*scope_index].bindings[&name]; + let existing = &self.ctx.bindings[existing_binding_index]; let in_current_scope = stack_index == 0; if !existing.kind.is_builtin() && existing.source.as_ref().map_or(true, |left| { @@ -4072,14 +4004,14 @@ impl<'a> Checker<'a> { } else if existing_is_import && binding.redefines(existing) { self.ctx .redefinitions - .entry(*existing_binding_index) + .entry(existing_binding_index) .or_insert_with(Vec::new) - .push(binding_index); + .push(binding_id); } } } - let scope = self.ctx.current_scope(); + let scope = self.ctx.scope(); let binding = if let Some(index) = scope.bindings.get(&name) { let existing = &self.ctx.bindings[*index]; match &existing.kind { @@ -4111,10 +4043,9 @@ impl<'a> Checker<'a> { // Don't treat annotations as assignments if there is an existing value // in scope. - let scope = - &mut self.ctx.scopes[*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); if !(binding.kind.is_annotation() && scope.bindings.contains_key(name)) { - if let Some(rebound_index) = scope.bindings.insert(name, binding_index) { + if let Some(rebound_index) = scope.bindings.insert(name, binding_id) { scope .rebounds .entry(name) @@ -4128,7 +4059,7 @@ impl<'a> Checker<'a> { fn bind_builtins(&mut self) { let scope = - &mut self.ctx.scopes[*(self.ctx.scope_stack.last().expect("No current scope found"))]; + &mut self.ctx.scopes[self.ctx.scope_stack.top().expect("No current scope found")]; for builtin in BUILTINS .iter() @@ -4136,17 +4067,16 @@ impl<'a> Checker<'a> { .copied() .chain(self.settings.builtins.iter().map(String::as_str)) { - let index = self.ctx.bindings.len(); - self.ctx.bindings.push(Binding { + let id = self.ctx.bindings.push(Binding { kind: BindingKind::Builtin, range: Range::default(), runtime_usage: None, - synthetic_usage: Some((0, Range::default())), + synthetic_usage: Some((ScopeId::global(), Range::default())), typing_usage: None, source: None, context: ExecutionContext::Runtime, }); - scope.bindings.insert(builtin, index); + scope.bindings.insert(builtin, id); } } @@ -4154,13 +4084,13 @@ impl<'a> Checker<'a> { let ExprKind::Name { id, .. } = &expr.node else { return; }; - let scope_id = self.ctx.current_scope().id; + let scope_id = self.ctx.scope_id(); let mut first_iter = true; let mut in_generator = false; let mut import_starred = false; - for scope_index in self.ctx.scope_stack.iter().rev() { + for scope_index in self.ctx.scope_stack.iter() { let scope = &self.ctx.scopes[*scope_index]; if matches!(scope.kind, ScopeKind::Class(_)) { @@ -4241,7 +4171,7 @@ impl<'a> Checker<'a> { if import_starred { if self.settings.rules.enabled(Rule::ImportStarUsage) { let mut from_list = vec![]; - for scope_index in self.ctx.scope_stack.iter().rev() { + for scope_index in self.ctx.scope_stack.iter() { let scope = &self.ctx.scopes[*scope_index]; for binding in scope .bindings @@ -4277,7 +4207,7 @@ impl<'a> Checker<'a> { // Allow "__module__" and "__qualname__" in class scopes. if (id == "__module__" || id == "__qualname__") - && matches!(self.ctx.current_scope().kind, ScopeKind::Class(..)) + && matches!(self.ctx.scope().kind, ScopeKind::Class(..)) { return; } @@ -4310,6 +4240,7 @@ impl<'a> Checker<'a> { .ctx .scope_stack .iter() + .rev() .map(|index| &self.ctx.scopes[*index]) .collect(); if let Some(diagnostic) = @@ -4324,11 +4255,11 @@ impl<'a> Checker<'a> { .rules .enabled(Rule::NonLowercaseVariableInFunction) { - if matches!(self.ctx.current_scope().kind, ScopeKind::Function(..)) { + if matches!(self.ctx.scope().kind, ScopeKind::Function(..)) { // Ignore globals. if !self .ctx - .current_scope() + .scope() .bindings .get(id) .map_or(false, |index| self.ctx.bindings[*index].kind.is_global()) @@ -4343,7 +4274,7 @@ impl<'a> Checker<'a> { .rules .enabled(Rule::MixedCaseVariableInClassScope) { - if matches!(self.ctx.current_scope().kind, ScopeKind::Class(..)) { + if matches!(self.ctx.scope().kind, ScopeKind::Class(..)) { pep8_naming::rules::mixed_case_variable_in_class_scope(self, expr, parent, id); } } @@ -4353,7 +4284,7 @@ impl<'a> Checker<'a> { .rules .enabled(Rule::MixedCaseVariableInGlobalScope) { - if matches!(self.ctx.current_scope().kind, ScopeKind::Module) { + if matches!(self.ctx.scope().kind, ScopeKind::Module) { pep8_naming::rules::mixed_case_variable_in_global_scope(self, expr, parent, id); } } @@ -4410,7 +4341,7 @@ impl<'a> Checker<'a> { return; } - let current = self.ctx.current_scope(); + let current = self.ctx.scope(); if id == "__all__" && matches!(current.kind, ScopeKind::Module) && matches!( @@ -4501,8 +4432,7 @@ impl<'a> Checker<'a> { return; } - let scope = - &mut self.ctx.scopes[*(self.ctx.scope_stack.last().expect("No current scope found"))]; + let scope = self.ctx.scope_mut(); if scope.bindings.remove(&id.as_str()).is_some() { return; } @@ -4646,8 +4576,9 @@ impl<'a> Checker<'a> { fn check_deferred_assignments(&mut self) { self.deferred.assignments.reverse(); while let Some((scopes, ..)) = self.deferred.assignments.pop() { - let scope_index = scopes[scopes.len() - 1]; - let parent_scope_index = scopes[scopes.len() - 2]; + let mut scopes_iter = scopes.iter(); + let scope_index = *scopes_iter.next().unwrap(); + let parent_scope_index = *scopes_iter.next().unwrap(); // pyflakes if self.settings.rules.enabled(Rule::UnusedVariable) { @@ -4728,28 +4659,33 @@ impl<'a> Checker<'a> { } // Mark anything referenced in `__all__` as used. - let global_scope = &self.ctx.scopes[GLOBAL_SCOPE_INDEX]; - let all_names: Option<(&Vec, Range)> = global_scope - .bindings - .get("__all__") - .map(|index| &self.ctx.bindings[*index]) - .and_then(|binding| match &binding.kind { - BindingKind::Export(names) => Some((names, binding.range)), - _ => None, - }); - let all_bindings: Option<(Vec, Range)> = all_names.map(|(names, range)| { - ( - names - .iter() - .filter_map(|name| global_scope.bindings.get(name.as_str()).copied()) - .collect(), - range, - ) - }); + + let all_bindings: Option<(Vec, Range)> = { + let global_scope = self.ctx.global_scope(); + let all_names: Option<(&Vec, Range)> = global_scope + .bindings + .get("__all__") + .map(|index| &self.ctx.bindings[*index]) + .and_then(|binding| match &binding.kind { + BindingKind::Export(names) => Some((names, binding.range)), + _ => None, + }); + + all_names.map(|(names, range)| { + ( + names + .iter() + .filter_map(|name| global_scope.bindings.get(name.as_str()).copied()) + .collect(), + range, + ) + }) + }; + if let Some((bindings, range)) = all_bindings { for index in bindings { self.ctx.bindings[index].mark_used( - GLOBAL_SCOPE_INDEX, + ScopeId::global(), range, ExecutionContext::Runtime, ); @@ -4757,7 +4693,9 @@ impl<'a> Checker<'a> { } // Extract `__all__` names from the global scope. - let all_names: Option<(Vec<&str>, Range)> = global_scope + let all_names: Option<(Vec<&str>, Range)> = self + .ctx + .global_scope() .bindings .get("__all__") .map(|index| &self.ctx.bindings[*index]) @@ -4786,7 +4724,7 @@ impl<'a> Checker<'a> { .filter(|binding| { flake8_type_checking::helpers::is_valid_runtime_import(binding) }) - .collect::>() + .collect() }) .collect::>() } @@ -4799,7 +4737,7 @@ impl<'a> Checker<'a> { let scope = &self.ctx.scopes[*index]; // F822 - if *index == GLOBAL_SCOPE_INDEX { + if index.is_global() { if self.settings.rules.enabled(Rule::UndefinedExport) { if let Some((names, range)) = &all_names { diagnostics.extend(pyflakes::rules::undefined_export( @@ -4915,8 +4853,9 @@ impl<'a> Checker<'a> { } else { stack .iter() + .rev() .chain(iter::once(index)) - .flat_map(|index| runtime_imports[*index].iter()) + .flat_map(|index| runtime_imports[usize::from(*index)].iter()) .copied() .collect() }; @@ -5356,7 +5295,7 @@ impl<'a> Checker<'a> { } fn check_builtin_shadowing(&mut self, name: &str, located: &Located, is_attribute: bool) { - if is_attribute && matches!(self.ctx.current_scope().kind, ScopeKind::Class(_)) { + if is_attribute && matches!(self.ctx.scope().kind, ScopeKind::Class(_)) { if self.settings.rules.enabled(Rule::BuiltinAttributeShadowing) { if let Some(diagnostic) = flake8_builtins::rules::builtin_shadowing( name, @@ -5420,7 +5359,6 @@ pub fn check_ast( stylist, indexer, ); - checker.ctx.push_scope(Scope::new(ScopeKind::Module)); checker.bind_builtins(); // Check for module docstring. @@ -5445,7 +5383,7 @@ pub fn check_ast( checker.check_definitions(); // Reset the scope to module-level, and check all consumed scopes. - checker.ctx.scope_stack = vec![GLOBAL_SCOPE_INDEX]; + checker.ctx.scope_stack = ScopeStack::default(); checker.ctx.pop_scope(); checker.check_dead_scopes(); diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/cached_instance_method.rs b/crates/ruff/src/rules/flake8_bugbear/rules/cached_instance_method.rs index bc0f5d40efa21..6aea88d051ebc 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/cached_instance_method.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/cached_instance_method.rs @@ -30,7 +30,7 @@ fn is_cache_func(checker: &Checker, expr: &Expr) -> bool { /// B019 pub fn cached_instance_method(checker: &mut Checker, decorator_list: &[Expr]) { - if !matches!(checker.ctx.current_scope().kind, ScopeKind::Class(_)) { + if !matches!(checker.ctx.scope().kind, ScopeKind::Class(_)) { return; } for decorator in decorator_list { diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index 556b542de0c77..2f83bbc2fb12a 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -163,7 +163,7 @@ pub fn unused_loop_control_variable( if let Some(rename) = rename { if certainty.into() && checker.patch(diagnostic.kind.rule()) { // Find the `BindingKind::LoopVar` corresponding to the name. - let scope = checker.ctx.current_scope(); + let scope = checker.ctx.scope(); let binding = scope .bindings .get(name) diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs index a512777929f9d..b6c9989ed932a 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs @@ -93,7 +93,7 @@ pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop, } // Avoid flagging issues in dunder implementations. - if let ScopeKind::Function(def) = &checker.ctx.current_scope().kind { + if let ScopeKind::Function(def) = &checker.ctx.scope().kind { if DUNDER_METHODS.contains(&def.name) { return; } @@ -144,7 +144,7 @@ pub fn negation_with_not_equal_op( } // Avoid flagging issues in dunder implementations. - if let ScopeKind::Function(def) = &checker.ctx.current_scope().kind { + if let ScopeKind::Function(def) = &checker.ctx.scope().kind { if DUNDER_METHODS.contains(&def.name) { return; } diff --git a/crates/ruff/src/rules/flake8_type_checking/helpers.rs b/crates/ruff/src/rules/flake8_type_checking/helpers.rs index f24c6a8214cf9..6b520fee7c769 100644 --- a/crates/ruff/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff/src/rules/flake8_type_checking/helpers.rs @@ -71,7 +71,7 @@ pub fn runtime_evaluated( } fn runtime_evaluated_base_class(context: &Context, base_classes: &[String]) -> bool { - if let ScopeKind::Class(class_def) = &context.current_scope().kind { + if let ScopeKind::Class(class_def) = &context.scope().kind { for base in class_def.bases.iter() { if let Some(call_path) = context.resolve_call_path(base) { if base_classes @@ -87,7 +87,7 @@ fn runtime_evaluated_base_class(context: &Context, base_classes: &[String]) -> b } fn runtime_evaluated_decorators(context: &Context, decorators: &[String]) -> bool { - if let ScopeKind::Class(class_def) = &context.current_scope().kind { + if let ScopeKind::Class(class_def) = &context.scope().kind { for decorator in class_def.decorator_list.iter() { if let Some(call_path) = context.resolve_call_path(map_callable(decorator)) { if decorators diff --git a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs b/crates/ruff/src/rules/flake8_unused_arguments/rules.rs index 54cb212f758a8..c4f94eacfd045 100644 --- a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs +++ b/crates/ruff/src/rules/flake8_unused_arguments/rules.rs @@ -6,9 +6,10 @@ use rustpython_parser::ast::{Arg, Arguments}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::context::Bindings; use ruff_python_ast::function_type; use ruff_python_ast::function_type::FunctionType; -use ruff_python_ast::types::{Binding, FunctionDef, Lambda, Scope, ScopeKind}; +use ruff_python_ast::types::{BindingId, FunctionDef, Lambda, Scope, ScopeKind}; use ruff_python_ast::visibility; use crate::checkers::ast::Checker; @@ -85,8 +86,8 @@ impl Violation for UnusedLambdaArgument { fn function( argumentable: &Argumentable, args: &Arguments, - values: &FxHashMap<&str, usize>, - bindings: &[Binding], + values: &FxHashMap<&str, BindingId>, + bindings: &Bindings, dummy_variable_rgx: &Regex, ignore_variadic_names: bool, ) -> Vec { @@ -112,8 +113,8 @@ fn function( fn method( argumentable: &Argumentable, args: &Arguments, - values: &FxHashMap<&str, usize>, - bindings: &[Binding], + values: &FxHashMap<&str, BindingId>, + bindings: &Bindings, dummy_variable_rgx: &Regex, ignore_variadic_names: bool, ) -> Vec { @@ -139,8 +140,8 @@ fn method( fn call<'a>( argumentable: &Argumentable, args: impl Iterator, - values: &FxHashMap<&str, usize>, - bindings: &[Binding], + values: &FxHashMap<&str, BindingId>, + bindings: &Bindings, dummy_variable_rgx: &Regex, ) -> Vec { let mut diagnostics: Vec = vec![]; @@ -168,7 +169,7 @@ pub fn unused_arguments( checker: &Checker, parent: &Scope, scope: &Scope, - bindings: &[Binding], + bindings: &Bindings, ) -> Vec { match &scope.kind { ScopeKind::Function(FunctionDef { diff --git a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs index 021b80467edd0..10cda5f65c9e7 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs @@ -65,7 +65,7 @@ pub fn lambda_assignment(checker: &mut Checker, target: &Expr, value: &Expr, stm // package like dataclasses, which wouldn't consider the // rewritten function definition to be equivalent. // See https://github.com/charliermarsh/ruff/issues/3046 - let fixable = !matches!(checker.ctx.current_scope().kind, ScopeKind::Class(_)); + let fixable = !matches!(checker.ctx.scope().kind, ScopeKind::Class(_)); let mut diagnostic = Diagnostic::new( LambdaAssignment { diff --git a/crates/ruff/src/rules/pyflakes/rules/return_outside_function.rs b/crates/ruff/src/rules/pyflakes/rules/return_outside_function.rs index 0224455f8ccf4..2ec768426a9f4 100644 --- a/crates/ruff/src/rules/pyflakes/rules/return_outside_function.rs +++ b/crates/ruff/src/rules/pyflakes/rules/return_outside_function.rs @@ -17,7 +17,7 @@ impl Violation for ReturnOutsideFunction { } pub fn return_outside_function(checker: &mut Checker, stmt: &Stmt) { - if let Some(&index) = checker.ctx.scope_stack.last() { + if let Some(index) = checker.ctx.scope_stack.top() { if matches!( checker.ctx.scopes[index].kind, ScopeKind::Class(_) | ScopeKind::Module diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs index 02b2f8e2e6800..e63af15388228 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs @@ -2,7 +2,8 @@ use std::string::ToString; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::types::{Binding, Scope, ScopeKind}; +use ruff_python_ast::context::Bindings; +use ruff_python_ast::types::{Scope, ScopeKind}; #[violation] pub struct UndefinedLocal { @@ -18,7 +19,7 @@ impl Violation for UndefinedLocal { } /// F821 -pub fn undefined_local(name: &str, scopes: &[&Scope], bindings: &[Binding]) -> Option { +pub fn undefined_local(name: &str, scopes: &[&Scope], bindings: &Bindings) -> Option { let current = &scopes.last().expect("No current scope found"); if matches!(current.kind, ScopeKind::Function(_)) && !current.bindings.contains_key(name) { for scope in scopes.iter().rev().skip(1) { diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs b/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs index b82258389363f..f7a994b7e76ed 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs @@ -1,5 +1,6 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::types::ScopeId; use crate::checkers::ast::Checker; @@ -17,7 +18,7 @@ impl Violation for UnusedAnnotation { } /// F842 -pub fn unused_annotation(checker: &mut Checker, scope: usize) { +pub fn unused_annotation(checker: &mut Checker, scope: ScopeId) { let scope = &checker.ctx.scopes[scope]; for (name, binding) in scope .bindings diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs index f99cacb527d3d..109b875ca83f7 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs @@ -7,7 +7,7 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::contains_effect; use ruff_python_ast::source_code::Locator; -use ruff_python_ast::types::{Range, RefEquality, ScopeKind}; +use ruff_python_ast::types::{Range, RefEquality, ScopeId, ScopeKind}; use crate::autofix::helpers::delete_stmt; use crate::checkers::ast::Checker; @@ -312,7 +312,7 @@ fn remove_unused_variable( } /// F841 -pub fn unused_variable(checker: &mut Checker, scope: usize) { +pub fn unused_variable(checker: &mut Checker, scope: ScopeId) { let scope = &checker.ctx.scopes[scope]; if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) { return; diff --git a/crates/ruff/src/rules/pyflakes/rules/yield_outside_function.rs b/crates/ruff/src/rules/pyflakes/rules/yield_outside_function.rs index fcf83bceee881..96133bc936f52 100644 --- a/crates/ruff/src/rules/pyflakes/rules/yield_outside_function.rs +++ b/crates/ruff/src/rules/pyflakes/rules/yield_outside_function.rs @@ -40,7 +40,7 @@ impl Violation for YieldOutsideFunction { pub fn yield_outside_function(checker: &mut Checker, expr: &Expr) { if matches!( - checker.ctx.current_scope().kind, + checker.ctx.scope().kind, ScopeKind::Class(_) | ScopeKind::Module ) { let keyword = match expr.node { diff --git a/crates/ruff/src/rules/pylint/helpers.rs b/crates/ruff/src/rules/pylint/helpers.rs index 64f31492a4eb3..e1fb0b413a6bd 100644 --- a/crates/ruff/src/rules/pylint/helpers.rs +++ b/crates/ruff/src/rules/pylint/helpers.rs @@ -5,7 +5,7 @@ use ruff_python_ast::types::{FunctionDef, ScopeKind}; use crate::checkers::ast::Checker; pub fn in_dunder_init(checker: &Checker) -> bool { - let scope = checker.ctx.current_scope(); + let scope = checker.ctx.scope(); let ScopeKind::Function(FunctionDef { name, decorator_list, @@ -16,7 +16,7 @@ pub fn in_dunder_init(checker: &Checker) -> bool { if name != "__init__" { return false; } - let Some(parent) = checker.ctx.current_scope_parent() else { + let Some(parent) = checker.ctx.parent_scope() else { return false; }; diff --git a/crates/ruff/src/rules/pylint/rules/await_outside_async.rs b/crates/ruff/src/rules/pylint/rules/await_outside_async.rs index 4748e2777dc3b..4c1728ad43d87 100644 --- a/crates/ruff/src/rules/pylint/rules/await_outside_async.rs +++ b/crates/ruff/src/rules/pylint/rules/await_outside_async.rs @@ -20,7 +20,7 @@ impl Violation for AwaitOutsideAsync { pub fn await_outside_async(checker: &mut Checker, expr: &Expr) { if !checker .ctx - .current_scopes() + .scopes() .find_map(|scope| { if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { Some(*async_) diff --git a/crates/ruff/src/rules/pylint/rules/consider_using_sys_exit.rs b/crates/ruff/src/rules/pylint/rules/consider_using_sys_exit.rs index 6878110030d7d..1822a9dc95ae8 100644 --- a/crates/ruff/src/rules/pylint/rules/consider_using_sys_exit.rs +++ b/crates/ruff/src/rules/pylint/rules/consider_using_sys_exit.rs @@ -28,7 +28,7 @@ impl Violation for ConsiderUsingSysExit { /// Return `true` if the `module` was imported using a star import (e.g., `from /// sys import *`). fn is_module_star_imported(checker: &Checker, module: &str) -> bool { - checker.ctx.current_scopes().any(|scope| { + checker.ctx.scopes().any(|scope| { scope.bindings.values().any(|index| { if let BindingKind::StarImportation(_, name) = &checker.ctx.bindings[*index].kind { name.as_ref().map(|name| name == module).unwrap_or_default() @@ -42,7 +42,7 @@ fn is_module_star_imported(checker: &Checker, module: &str) -> bool { /// Return the appropriate `sys.exit` reference based on the current set of /// imports, or `None` is `sys.exit` hasn't been imported. fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -> Option { - checker.ctx.current_scopes().find_map(|scope| { + checker.ctx.scopes().find_map(|scope| { scope .bindings .values() diff --git a/crates/ruff/src/rules/pylint/rules/global_statement.rs b/crates/ruff/src/rules/pylint/rules/global_statement.rs index 2ff492ca59ed3..530510feab58d 100644 --- a/crates/ruff/src/rules/pylint/rules/global_statement.rs +++ b/crates/ruff/src/rules/pylint/rules/global_statement.rs @@ -52,7 +52,7 @@ impl Violation for GlobalStatement { /// PLW0603 pub fn global_statement(checker: &mut Checker, name: &str) { - let scope = checker.ctx.current_scope(); + let scope = checker.ctx.scope(); if let Some(index) = scope.bindings.get(name) { let binding = &checker.ctx.bindings[*index]; if binding.kind.is_global() { diff --git a/crates/ruff/src/rules/pylint/rules/used_prior_global_declaration.rs b/crates/ruff/src/rules/pylint/rules/used_prior_global_declaration.rs index 22126b046810a..46beebe2d0211 100644 --- a/crates/ruff/src/rules/pylint/rules/used_prior_global_declaration.rs +++ b/crates/ruff/src/rules/pylint/rules/used_prior_global_declaration.rs @@ -21,7 +21,7 @@ impl Violation for UsedPriorGlobalDeclaration { } /// PLE0118 pub fn used_prior_global_declaration(checker: &mut Checker, name: &str, expr: &Expr) { - let globals = match &checker.ctx.current_scope().kind { + let globals = match &checker.ctx.scope().kind { ScopeKind::Class(class_def) => &class_def.globals, ScopeKind::Function(function_def) => &function_def.globals, _ => return, diff --git a/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs b/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs index d6a7a099e728c..4fdab4b178676 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs @@ -38,7 +38,7 @@ pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Exp if !is_super_call_with_arguments(func, args) { return; } - let scope = checker.ctx.current_scope(); + let scope = checker.ctx.scope(); let parents: Vec<&Stmt> = checker.ctx.parents.iter().map(Into::into).collect(); // Check: are we in a Function scope? diff --git a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs index fec76bcfaf3ba..e2bf86f1c45d1 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs @@ -2,6 +2,7 @@ use rustpython_parser::ast::{Expr, ExprKind, Keyword, Stmt}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::context::Bindings; use ruff_python_ast::types::{Binding, BindingKind, Range, Scope}; use crate::checkers::ast::Checker; @@ -26,7 +27,7 @@ impl AlwaysAutofixableViolation for UselessObjectInheritance { } } -fn rule(name: &str, bases: &[Expr], scope: &Scope, bindings: &[Binding]) -> Option { +fn rule(name: &str, bases: &[Expr], scope: &Scope, bindings: &Bindings) -> Option { for expr in bases { let ExprKind::Name { id, .. } = &expr.node else { continue; @@ -65,7 +66,7 @@ pub fn useless_object_inheritance( bases: &[Expr], keywords: &[Keyword], ) { - let Some(mut diagnostic) = rule(name, bases, checker.ctx.current_scope(), &checker.ctx.bindings) else { + let Some(mut diagnostic) = rule(name, bases, checker.ctx.scope(), &checker.ctx.bindings) else { return; }; if checker.patch(diagnostic.kind.rule()) { diff --git a/crates/ruff_python_ast/src/context.rs b/crates/ruff_python_ast/src/context.rs index 341ac047b823a..947d3b08a6b61 100644 --- a/crates/ruff_python_ast/src/context.rs +++ b/crates/ruff_python_ast/src/context.rs @@ -1,6 +1,7 @@ +use std::ops::{Deref, Index, IndexMut}; use std::path::Path; -use nohash_hasher::IntMap; +use nohash_hasher::{BuildNoHashHasher, IntMap}; use rustc_hash::FxHashMap; use rustpython_parser::ast::{Expr, Stmt}; use smallvec::smallvec; @@ -9,7 +10,10 @@ use ruff_python_stdlib::path::is_python_stub_file; use ruff_python_stdlib::typing::TYPING_EXTENSIONS; use crate::helpers::{collect_call_path, from_relative_import, Exceptions}; -use crate::types::{Binding, BindingKind, CallPath, ExecutionContext, RefEquality, Scope}; +use crate::types::{ + Binding, BindingId, BindingKind, CallPath, ExecutionContext, RefEquality, Scope, ScopeId, + ScopeKind, +}; use crate::visibility::{module_visibility, Modifier, VisibleScope}; #[allow(clippy::struct_excessive_bools)] @@ -22,13 +26,14 @@ pub struct Context<'a> { pub depths: FxHashMap, usize>, pub child_to_parent: FxHashMap, RefEquality<'a, Stmt>>, // A stack of all bindings created in any scope, at any point in execution. - pub bindings: Vec>, + pub bindings: Bindings<'a>, // Map from binding index to indexes of bindings that redefine it in other scopes. - pub redefinitions: IntMap>, + pub redefinitions: + std::collections::HashMap, BuildNoHashHasher>, pub exprs: Vec>, - pub scopes: Vec>, - pub scope_stack: Vec, - pub dead_scopes: Vec<(usize, Vec)>, + pub scopes: Scopes<'a>, + pub scope_stack: ScopeStack, + pub dead_scopes: Vec<(ScopeId, ScopeStack)>, // Body iteration; used to peek at siblings. pub body: &'a [Stmt], pub body_index: usize, @@ -60,11 +65,11 @@ impl<'a> Context<'a> { parents: Vec::default(), depths: FxHashMap::default(), child_to_parent: FxHashMap::default(), - bindings: Vec::default(), + bindings: Bindings::default(), redefinitions: IntMap::default(), exprs: Vec::default(), - scopes: Vec::default(), - scope_stack: Vec::default(), + scopes: Scopes::default(), + scope_stack: ScopeStack::default(), dead_scopes: Vec::default(), body: &[], body_index: 0, @@ -119,7 +124,7 @@ impl<'a> Context<'a> { /// Return the current `Binding` for a given `name`. pub fn find_binding(&self, member: &str) -> Option<&Binding> { - self.current_scopes() + self.scopes() .find_map(|scope| scope.bindings.get(member)) .map(|index| &self.bindings[*index]) } @@ -217,9 +222,10 @@ impl<'a> Context<'a> { .expect("Attempted to pop without expression"); } - pub fn push_scope(&mut self, scope: Scope<'a>) { - self.scope_stack.push(self.scopes.len()); - self.scopes.push(scope); + pub fn push_scope(&mut self, kind: ScopeKind<'a>) -> ScopeId { + let id = self.scopes.push_scope(kind); + self.scope_stack.push(id); + id } pub fn pop_scope(&mut self) { @@ -261,23 +267,41 @@ impl<'a> Context<'a> { self.body.get(self.body_index + 1) } - pub fn current_scope(&self) -> &Scope { - &self.scopes[*(self.scope_stack.last().expect("No current scope found"))] + /// Returns a reference to the global scope + pub fn global_scope(&self) -> &Scope<'a> { + self.scopes.global() } - pub fn current_scope_parent(&self) -> Option<&Scope> { + /// Returns a mutable reference to the global scope + pub fn global_scope_mut(&mut self) -> &mut Scope<'a> { + self.scopes.global_mut() + } + + /// Returns the current top most scope. + pub fn scope(&self) -> &Scope<'a> { + &self.scopes[self.scope_stack.top().expect("No current scope found")] + } + + /// Returns the id of the top-most scope + pub fn scope_id(&self) -> ScopeId { + self.scope_stack.top().expect("No current scope found") + } + + /// Returns a mutable reference to the current top most scope. + pub fn scope_mut(&mut self) -> &mut Scope<'a> { + let top_id = self.scope_stack.top().expect("No current scope found"); + &mut self.scopes[top_id] + } + + pub fn parent_scope(&self) -> Option<&Scope> { self.scope_stack .iter() - .rev() .nth(1) .map(|index| &self.scopes[*index]) } - pub fn current_scopes(&self) -> impl Iterator { - self.scope_stack - .iter() - .rev() - .map(|index| &self.scopes[*index]) + pub fn scopes(&self) -> impl Iterator { + self.scope_stack.iter().map(|index| &self.scopes[*index]) } pub const fn in_exception_handler(&self) -> bool { @@ -295,3 +319,132 @@ impl<'a> Context<'a> { } } } + +/// The scopes of a program indexed by [`ScopeId`] +#[derive(Debug)] +pub struct Scopes<'a>(Vec>); + +impl<'a> Scopes<'a> { + /// Returns a reference to the global scope + pub fn global(&self) -> &Scope<'a> { + &self[ScopeId::global()] + } + + /// Returns a mutable reference to the global scope + pub fn global_mut(&mut self) -> &mut Scope<'a> { + &mut self[ScopeId::global()] + } + + /// Pushes a new scope and returns its unique id + fn push_scope(&mut self, kind: ScopeKind<'a>) -> ScopeId { + let next_id = ScopeId::try_from(self.0.len()).unwrap(); + self.0.push(Scope::local(next_id, kind)); + next_id + } +} + +impl Default for Scopes<'_> { + fn default() -> Self { + Self(vec![Scope::global(ScopeKind::Module)]) + } +} + +impl<'a> Index for Scopes<'a> { + type Output = Scope<'a>; + + fn index(&self, index: ScopeId) -> &Self::Output { + &self.0[usize::from(index)] + } +} + +impl<'a> IndexMut for Scopes<'a> { + fn index_mut(&mut self, index: ScopeId) -> &mut Self::Output { + &mut self.0[usize::from(index)] + } +} + +impl<'a> Deref for Scopes<'a> { + type Target = [Scope<'a>]; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[derive(Debug, Clone)] +pub struct ScopeStack(Vec); + +impl ScopeStack { + /// Pushes a new scope on the stack + pub fn push(&mut self, id: ScopeId) { + self.0.push(id); + } + + /// Pops the top most scope + pub fn pop(&mut self) -> Option { + self.0.pop() + } + + /// Returns the id of the top-most + pub fn top(&self) -> Option { + self.0.last().copied() + } + + /// Returns an iterator from the current scope to the top scope (reverse iterator) + pub fn iter(&self) -> std::iter::Rev> { + self.0.iter().rev() + } +} + +impl Default for ScopeStack { + fn default() -> Self { + Self(vec![ScopeId::global()]) + } +} + +/// The bindings in a program. +/// +/// Bindings are indexed by [`BindingId`] +#[derive(Debug, Clone, Default)] +pub struct Bindings<'a>(Vec>); + +impl<'a> Bindings<'a> { + /// Pushes a new binding and returns its id + pub fn push(&mut self, binding: Binding<'a>) -> BindingId { + let id = self.next_id(); + self.0.push(binding); + id + } + + /// Returns the id that will be assigned when pushing the next binding + pub fn next_id(&self) -> BindingId { + BindingId::try_from(self.0.len()).unwrap() + } +} + +impl<'a> Index for Bindings<'a> { + type Output = Binding<'a>; + + fn index(&self, index: BindingId) -> &Self::Output { + &self.0[usize::from(index)] + } +} + +impl<'a> IndexMut for Bindings<'a> { + fn index_mut(&mut self, index: BindingId) -> &mut Self::Output { + &mut self.0[usize::from(index)] + } +} + +impl<'a> Deref for Bindings<'a> { + type Target = [Binding<'a>]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl<'a> FromIterator> for Bindings<'a> { + fn from_iter>>(iter: T) -> Self { + Self(Vec::from_iter(iter)) + } +} diff --git a/crates/ruff_python_ast/src/types.rs b/crates/ruff_python_ast/src/types.rs index 0e158889d7b5b..1c27a7bf581d3 100644 --- a/crates/ruff_python_ast/src/types.rs +++ b/crates/ruff_python_ast/src/types.rs @@ -1,14 +1,9 @@ +use std::num::TryFromIntError; use std::ops::Deref; -use std::sync::atomic::{AtomicUsize, Ordering}; use rustc_hash::FxHashMap; use rustpython_parser::ast::{Arguments, Expr, Keyword, Located, Location, Stmt}; -fn id() -> usize { - static COUNTER: AtomicUsize = AtomicUsize::new(1); - COUNTER.fetch_add(1, Ordering::Relaxed) -} - #[derive(Clone)] pub enum Node<'a> { Stmt(&'a Stmt), @@ -84,24 +79,63 @@ pub enum ScopeKind<'a> { Lambda(Lambda<'a>), } +/// Id uniquely identifying a scope in a program. +/// +/// Using a `u32` is sufficient because Ruff only supports parsing documents with a size of max `u32::max` +/// and it is impossible to have more scopes than characters in the file (because defining a function or class +/// requires more than one character). +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct ScopeId(u32); + +impl ScopeId { + /// Returns the ID for the global scope + #[inline] + pub const fn global() -> Self { + ScopeId(0) + } + + /// Returns `true` if this is the id of the global scope + pub const fn is_global(&self) -> bool { + self.0 == 0 + } +} + +impl TryFrom for ScopeId { + type Error = TryFromIntError; + + fn try_from(value: usize) -> Result { + Ok(Self(u32::try_from(value)?)) + } +} + +impl From for usize { + fn from(value: ScopeId) -> Self { + value.0 as usize + } +} + #[derive(Debug)] pub struct Scope<'a> { - pub id: usize, + pub id: ScopeId, pub kind: ScopeKind<'a>, pub import_starred: bool, pub uses_locals: bool, /// A map from bound name to binding index, for live bindings. - pub bindings: FxHashMap<&'a str, usize>, + pub bindings: FxHashMap<&'a str, BindingId>, /// A map from bound name to binding index, for bindings that were created /// in the scope but rebound (and thus overridden) later on in the same /// scope. - pub rebounds: FxHashMap<&'a str, Vec>, + pub rebounds: FxHashMap<&'a str, Vec>, } impl<'a> Scope<'a> { - pub fn new(kind: ScopeKind<'a>) -> Self { + pub fn global(kind: ScopeKind<'a>) -> Self { + Self::local(ScopeId::global(), kind) + } + + pub fn local(id: ScopeId, kind: ScopeKind<'a>) -> Self { Scope { - id: id(), + id, kind, import_starred: false, uses_locals: false, @@ -148,6 +182,30 @@ pub enum BindingKind<'a> { SubmoduleImportation(&'a str, &'a str), } +/// ID uniquely identifying a [Binding] in a program. +/// +/// Using a `u32` to identify [Binding]s should is sufficient because Ruff only supports documents with a +/// size smaller than or equal to `u32::max`. A document with the size of `u32::max` must have fewer than `u32::max` +/// bindings because bindings must be separated by whitespace (and have an assignment). +#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub struct BindingId(u32); + +impl From for usize { + fn from(value: BindingId) -> Self { + value.0 as usize + } +} + +impl TryFrom for BindingId { + type Error = TryFromIntError; + + fn try_from(value: usize) -> Result { + Ok(Self(u32::try_from(value)?)) + } +} + +impl nohash_hasher::IsEnabled for BindingId {} + #[derive(Debug, Clone)] pub struct Binding<'a> { pub kind: BindingKind<'a>, @@ -158,15 +216,15 @@ pub struct Binding<'a> { pub source: Option>, /// Tuple of (scope index, range) indicating the scope and range at which /// the binding was last used in a runtime context. - pub runtime_usage: Option<(usize, Range)>, + pub runtime_usage: Option<(ScopeId, Range)>, /// Tuple of (scope index, range) indicating the scope and range at which /// the binding was last used in a typing-time context. - pub typing_usage: Option<(usize, Range)>, + pub typing_usage: Option<(ScopeId, Range)>, /// Tuple of (scope index, range) indicating the scope and range at which /// the binding was last used in a synthetic context. This is used for /// (e.g.) `__future__` imports, explicit re-exports, and other bindings /// that should be considered used even if they're never referenced. - pub synthetic_usage: Option<(usize, Range)>, + pub synthetic_usage: Option<(ScopeId, Range)>, } #[derive(Copy, Debug, Clone)] @@ -176,7 +234,7 @@ pub enum ExecutionContext { } impl<'a> Binding<'a> { - pub fn mark_used(&mut self, scope: usize, range: Range, context: ExecutionContext) { + pub fn mark_used(&mut self, scope: ScopeId, range: Range, context: ExecutionContext) { match context { ExecutionContext::Runtime => self.runtime_usage = Some((scope, range)), ExecutionContext::Typing => self.typing_usage = Some((scope, range)),