Skip to content

Commit

Permalink
Remove Scope.id
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 29, 2023
1 parent 4a05e5a commit 30b53b7
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 28 deletions.
21 changes: 9 additions & 12 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ where
let context = self.ctx.execution_context();
let exceptions = self.ctx.exceptions();
let scope = &mut self.ctx.scopes[self.ctx.scope_id];
let usage = Some((scope.id, stmt.range()));
let usage = Some((self.ctx.scope_id, stmt.range()));
for (name, range) in names.iter().zip(ranges.iter()) {
let id = self.ctx.bindings.push(Binding {
kind: BindingKind::Global,
Expand Down Expand Up @@ -251,7 +251,7 @@ where
let context = self.ctx.execution_context();
let exceptions = self.ctx.exceptions();
let scope = &mut self.ctx.scopes[self.ctx.scope_id];
let usage = Some((scope.id, stmt.range()));
let usage = Some((self.ctx.scope_id, stmt.range()));
for (name, range) in names.iter().zip(ranges.iter()) {
// Add a binding to the current scope.
let id = self.ctx.bindings.push(Binding {
Expand All @@ -275,11 +275,8 @@ where
.scopes
.ancestors(self.ctx.scope_id)
.skip(1)
.take_while(|scope_id| !scope_id.is_global())
.find_map(|scope_id| {
let scope = &self.ctx.scopes[scope_id];
scope.get(name.as_str())
});
.take_while(|scope| !scope.kind.is_module())
.find_map(|scope| scope.get(name.as_str()));

if let Some(binding_id) = binding_id {
self.ctx.bindings[*binding_id].runtime_usage = usage;
Expand Down Expand Up @@ -4179,7 +4176,7 @@ impl<'a> Checker<'a> {
if let Some((stack_index, existing_binding_id)) = self
.ctx
.scopes
.ancestor_scopes(self.ctx.scope_id)
.ancestors(self.ctx.scope_id)
.enumerate()
.find_map(|(stack_index, scope)| {
scope.get(name).map(|binding_id| (stack_index, *binding_id))
Expand Down Expand Up @@ -4341,7 +4338,7 @@ impl<'a> Checker<'a> {
let mut in_generator = false;
let mut import_starred = false;

for scope in self.ctx.scopes.ancestor_scopes(self.ctx.scope_id) {
for scope in self.ctx.scopes.ancestors(self.ctx.scope_id) {
if scope.kind.is_class() {
if id == "__class__" {
return;
Expand Down Expand Up @@ -4988,7 +4985,7 @@ impl<'a> Checker<'a> {
for scope_id in self.ctx.dead_scopes.iter().rev() {
let scope = &self.ctx.scopes[*scope_id];

if scope_id.is_global() {
if scope.kind.is_module() {
// F822
if self.settings.rules.enabled(Rule::UndefinedExport) {
if !self.path.ends_with("__init__.py") {
Expand Down Expand Up @@ -5116,8 +5113,8 @@ impl<'a> Checker<'a> {
} else {
self.ctx
.scopes
.ancestors(*scope_id)
.flat_map(|index| runtime_imports[usize::from(index)].iter())
.ancestor_ids(*scope_id)
.flat_map(|scope_id| runtime_imports[usize::from(scope_id)].iter())
.copied()
.collect()
};
Expand Down
8 changes: 3 additions & 5 deletions crates/ruff/src/rules/pyflakes/rules/undefined_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ impl Violation for UndefinedLocal {

/// F823
pub fn undefined_local(checker: &mut Checker, name: &str) {
let current = &checker.ctx.scopes[checker.ctx.scope_id];

// If the name hasn't already been defined in the current scope...
let current = checker.ctx.scope();
if !current.kind.is_function() || current.defines(name) {
return;
}
Expand All @@ -32,8 +31,7 @@ pub fn undefined_local(checker: &mut Checker, name: &str) {
};

// For every function and module scope above us...
for scope_id in checker.ctx.scopes.ancestors(parent) {
let scope = &checker.ctx.scopes[scope_id];
for scope in checker.ctx.scopes.ancestors(parent) {
if !(scope.kind.is_function() || scope.kind.is_module()) {
continue;
}
Expand All @@ -42,7 +40,7 @@ pub fn undefined_local(checker: &mut Checker, name: &str) {
if let Some(binding) = scope.get(name).map(|index| &checker.ctx.bindings[*index]) {
// And has already been accessed in the current scope...
if let Some((scope_id, location)) = binding.runtime_usage {
if scope_id == current.id {
if scope_id == checker.ctx.scope_id {
// Then it's probably an error.
checker.diagnostics.push(Diagnostic::new(
UndefinedLocal {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_semantic/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl<'a> Context<'a> {

/// Returns an iterator over all scopes, starting from the current scope.
pub fn scopes(&self) -> impl Iterator<Item = &Scope> {
self.scopes.ancestor_scopes(self.scope_id)
self.scopes.ancestors(self.scope_id)
}

/// Returns `true` if the context is in an exception handler.
Expand Down
17 changes: 7 additions & 10 deletions crates/ruff_python_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::binding::{BindingId, StarImportation};

#[derive(Debug)]
pub struct Scope<'a> {
pub id: ScopeId,
pub kind: ScopeKind<'a>,
pub parent: Option<ScopeId>,
/// Whether this scope uses the `locals()` builtin.
Expand All @@ -25,7 +24,6 @@ pub struct Scope<'a> {
impl<'a> Scope<'a> {
pub fn global() -> Self {
Scope {
id: ScopeId::global(),
kind: ScopeKind::Module,
parent: None,
uses_locals: false,
Expand All @@ -35,9 +33,8 @@ impl<'a> Scope<'a> {
}
}

pub fn local(id: ScopeId, parent: ScopeId, kind: ScopeKind<'a>) -> Self {
pub fn local(kind: ScopeKind<'a>, parent: ScopeId) -> Self {
Scope {
id,
kind,
parent: Some(parent),
uses_locals: false,
Expand Down Expand Up @@ -202,19 +199,19 @@ impl<'a> Scopes<'a> {
/// Pushes a new scope and returns its unique id
pub fn push_scope(&mut self, kind: ScopeKind<'a>, parent: ScopeId) -> ScopeId {
let next_id = ScopeId::try_from(self.0.len()).unwrap();
self.0.push(Scope::local(next_id, parent, kind));
self.0.push(Scope::local(kind, parent));
next_id
}

/// Returns an iterator over all [`ScopeId`] ancestors, starting from the given [`ScopeId`].
pub fn ancestors(&self, scope: ScopeId) -> impl Iterator<Item = ScopeId> + '_ {
std::iter::successors(Some(scope), |&scope| self[scope].parent)
pub fn ancestor_ids(&self, scope_id: ScopeId) -> impl Iterator<Item = ScopeId> + '_ {
std::iter::successors(Some(scope_id), |&scope_id| self[scope_id].parent)
}

/// Returns an iterator over all [`Scope`] ancestors, starting from the given [`ScopeId`].
pub fn ancestor_scopes(&self, scope: ScopeId) -> impl Iterator<Item = &Scope> + '_ {
std::iter::successors(Some(&self[scope]), |&scope| {
scope.parent.map(|parent| &self[parent])
pub fn ancestors(&self, scope_id: ScopeId) -> impl Iterator<Item = &Scope> + '_ {
std::iter::successors(Some(&self[scope_id]), |&scope| {
scope.parent.map(|scope_id| &self[scope_id])
})
}
}
Expand Down

0 comments on commit 30b53b7

Please sign in to comment.