Skip to content

Commit

Permalink
Make scope.bindings private (#3575)
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Mar 17, 2023
1 parent 2a088c2 commit f46f65a
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 75 deletions.
77 changes: 32 additions & 45 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ where
source: Some(RefEquality(stmt)),
context,
});
scope.bindings.insert(name, id);
scope.add(name, id);
}
}

Expand Down Expand Up @@ -238,7 +238,7 @@ where
source: Some(RefEquality(stmt)),
context,
});
scope.bindings.insert(name, id);
scope.add(name, id);
}

// Mark the binding in the defining scopes as used too. (Skip the global scope
Expand All @@ -250,9 +250,7 @@ where
scopes_iter.next_back();

for index in scopes_iter.skip(1) {
if let Some(index) =
self.ctx.scopes[*index].bindings.get(&name.as_str())
{
if let Some(index) = self.ctx.scopes[*index].get(name.as_str()) {
exists = true;
self.ctx.bindings[*index].runtime_usage = usage;
}
Expand Down Expand Up @@ -1875,7 +1873,6 @@ where
if self
.ctx
.global_scope()
.bindings
.get(name)
.map_or(true, |index| self.ctx.bindings[*index].kind.is_annotation())
{
Expand All @@ -1888,7 +1885,7 @@ where
source: Some(RefEquality(stmt)),
context: self.ctx.execution_context(),
});
self.ctx.global_scope_mut().bindings.insert(name, id);
self.ctx.global_scope_mut().add(name, id);
}
}

Expand Down Expand Up @@ -1938,7 +1935,6 @@ where
if self
.ctx
.global_scope()
.bindings
.get(name)
.map_or(true, |index| self.ctx.bindings[*index].kind.is_annotation())
{
Expand All @@ -1951,7 +1947,7 @@ where
source: Some(RefEquality(stmt)),
context: self.ctx.execution_context(),
});
self.ctx.global_scope_mut().bindings.insert(name, id);
self.ctx.global_scope_mut().add(name, id);
}
}

Expand Down Expand Up @@ -3714,7 +3710,7 @@ where
let name_range =
helpers::excepthandler_name_range(excepthandler, self.locator).unwrap();

if self.ctx.scope().bindings.contains_key(&name.as_str()) {
if self.ctx.scope().defines(name.as_str()) {
self.handle_node_store(
name,
&Expr::new(
Expand All @@ -3728,7 +3724,7 @@ where
);
}

let definition = self.ctx.scope().bindings.get(&name.as_str()).copied();
let definition = self.ctx.scope().get(name.as_str()).copied();
self.handle_node_store(
name,
&Expr::new(
Expand All @@ -3745,7 +3741,7 @@ where

if let Some(index) = {
let scope = self.ctx.scope_mut();
&scope.bindings.remove(&name.as_str())
&scope.remove(name.as_str())
} {
if !self.ctx.bindings[*index].used() {
if self.settings.rules.enabled(Rule::UnusedVariable) {
Expand Down Expand Up @@ -3779,7 +3775,7 @@ where

if let Some(index) = definition {
let scope = self.ctx.scope_mut();
scope.bindings.insert(name, index);
scope.add(name, index);
}
}
None => walk_excepthandler(self, excepthandler),
Expand Down Expand Up @@ -3934,14 +3930,17 @@ impl<'a> Checker<'a> {
'b: 'a,
{
let binding_id = self.ctx.bindings.next_id();
if let Some((stack_index, scope_index)) = self
if let Some((stack_index, existing_binding_index)) = self
.ctx
.scope_stack
.iter()
.enumerate()
.find(|(_, scope_index)| self.ctx.scopes[**scope_index].bindings.contains_key(&name))
.find_map(|(stack_index, scope_index)| {
self.ctx.scopes[*scope_index]
.get(name)
.map(|binding_id| (stack_index, *binding_id))
})
{
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()
Expand Down Expand Up @@ -4013,7 +4012,7 @@ impl<'a> Checker<'a> {
}

let scope = self.ctx.scope();
let binding = if let Some(index) = scope.bindings.get(&name) {
let binding = if let Some(index) = scope.get(name) {
let existing = &self.ctx.bindings[*index];
match &existing.kind {
BindingKind::Builtin => {
Expand Down Expand Up @@ -4045,8 +4044,8 @@ impl<'a> Checker<'a> {
// Don't treat annotations as assignments if there is an existing value
// in scope.
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_id) {
if !(binding.kind.is_annotation() && scope.defines(name)) {
if let Some(rebound_index) = scope.add(name, binding_id) {
scope
.rebounds
.entry(name)
Expand Down Expand Up @@ -4077,7 +4076,7 @@ impl<'a> Checker<'a> {
source: None,
context: ExecutionContext::Runtime,
});
scope.bindings.insert(builtin, id);
scope.add(builtin, id);
}
}

Expand All @@ -4102,7 +4101,7 @@ impl<'a> Checker<'a> {
}
}

if let Some(index) = scope.bindings.get(&id.as_str()) {
if let Some(index) = scope.get(id.as_str()) {
// Mark the binding as used.
let context = self.ctx.execution_context();
self.ctx.bindings[*index].mark_used(scope_id, Range::from(expr), context);
Expand Down Expand Up @@ -4132,7 +4131,7 @@ impl<'a> Checker<'a> {
.unwrap_or_default();
if has_alias {
// Mark the sub-importation as used.
if let Some(index) = scope.bindings.get(full_name) {
if let Some(index) = scope.get(full_name) {
self.ctx.bindings[*index].mark_used(
scope_id,
Range::from(expr),
Expand All @@ -4149,7 +4148,7 @@ impl<'a> Checker<'a> {
.unwrap_or_default();
if has_alias {
// Mark the sub-importation as used.
if let Some(index) = scope.bindings.get(full_name.as_str()) {
if let Some(index) = scope.get(full_name.as_str()) {
self.ctx.bindings[*index].mark_used(
scope_id,
Range::from(expr),
Expand All @@ -4174,11 +4173,7 @@ impl<'a> Checker<'a> {
let mut from_list = vec![];
for scope_index in self.ctx.scope_stack.iter() {
let scope = &self.ctx.scopes[*scope_index];
for binding in scope
.bindings
.values()
.map(|index| &self.ctx.bindings[*index])
{
for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from(
level.as_ref(),
Expand Down Expand Up @@ -4261,7 +4256,6 @@ impl<'a> Checker<'a> {
if !self
.ctx
.scope()
.bindings
.get(id)
.map_or(false, |index| self.ctx.bindings[*index].kind.is_global())
{
Expand Down Expand Up @@ -4434,7 +4428,7 @@ impl<'a> Checker<'a> {
}

let scope = self.ctx.scope_mut();
if scope.bindings.remove(&id.as_str()).is_some() {
if scope.remove(id.as_str()).is_some() {
return;
}
if !self.settings.rules.enabled(Rule::UndefinedName) {
Expand Down Expand Up @@ -4664,7 +4658,6 @@ impl<'a> Checker<'a> {
let all_bindings: Option<(Vec<BindingId>, Range)> = {
let global_scope = self.ctx.global_scope();
let all_names: Option<(&Vec<String>, Range)> = global_scope
.bindings
.get("__all__")
.map(|index| &self.ctx.bindings[*index])
.and_then(|binding| match &binding.kind {
Expand All @@ -4676,7 +4669,7 @@ impl<'a> Checker<'a> {
(
names
.iter()
.filter_map(|name| global_scope.bindings.get(name.as_str()).copied())
.filter_map(|name| global_scope.get(name.as_str()).copied())
.collect(),
range,
)
Expand All @@ -4697,7 +4690,6 @@ impl<'a> Checker<'a> {
let all_names: Option<(Vec<&str>, Range)> = self
.ctx
.global_scope()
.bindings
.get("__all__")
.map(|index| &self.ctx.bindings[*index])
.and_then(|binding| match &binding.kind {
Expand All @@ -4719,8 +4711,7 @@ impl<'a> Checker<'a> {
.iter()
.map(|scope| {
scope
.bindings
.values()
.binding_ids()
.map(|index| &self.ctx.bindings[*index])
.filter(|binding| {
flake8_type_checking::helpers::is_valid_runtime_import(binding)
Expand Down Expand Up @@ -4750,7 +4741,7 @@ impl<'a> Checker<'a> {

// PLW0602
if self.settings.rules.enabled(Rule::GlobalVariableNotAssigned) {
for (name, index) in &scope.bindings {
for (name, index) in scope.bindings() {
let binding = &self.ctx.bindings[*index];
if binding.kind.is_global() {
if let Some(stmt) = &binding.source {
Expand All @@ -4776,7 +4767,7 @@ impl<'a> Checker<'a> {
// unused. Note that we only store references in `redefinitions` if
// the bindings are in different scopes.
if self.settings.rules.enabled(Rule::RedefinedWhileUnused) {
for (name, index) in &scope.bindings {
for (name, index) in scope.bindings() {
let binding = &self.ctx.bindings[*index];

if matches!(
Expand Down Expand Up @@ -4819,11 +4810,7 @@ impl<'a> Checker<'a> {
if scope.import_starred {
if let Some((names, range)) = &all_names {
let mut from_list = vec![];
for binding in scope
.bindings
.values()
.map(|index| &self.ctx.bindings[*index])
{
for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from(
level.as_ref(),
Expand All @@ -4834,7 +4821,7 @@ impl<'a> Checker<'a> {
from_list.sort();

for &name in names {
if !scope.bindings.contains_key(name) {
if !scope.defines(name) {
diagnostics.push(Diagnostic::new(
pyflakes::rules::ImportStarUsage {
name: name.to_string(),
Expand All @@ -4860,7 +4847,7 @@ impl<'a> Checker<'a> {
.copied()
.collect()
};
for (.., index) in &scope.bindings {
for index in scope.binding_ids() {
let binding = &self.ctx.bindings[*index];

if let Some(diagnostic) =
Expand Down Expand Up @@ -4896,7 +4883,7 @@ impl<'a> Checker<'a> {
let mut ignored: FxHashMap<BindingContext, Vec<UnusedImport>> =
FxHashMap::default();

for index in scope.bindings.values() {
for index in scope.binding_ids() {
let binding = &self.ctx.bindings[*index];

let full_name = match &binding.kind {
Expand Down
1 change: 0 additions & 1 deletion crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ pub fn unused_loop_control_variable(
// Find the `BindingKind::LoopVar` corresponding to the name.
let scope = checker.ctx.scope();
let binding = scope
.bindings
.get(name)
.into_iter()
.chain(scope.rebounds.get(name).into_iter().flatten())
Expand Down

0 comments on commit f46f65a

Please sign in to comment.