language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-27 12:29:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Invert parent-shadowed bindings map (#4847)

Browse source
This commit is contained in:
Charlie Marsh 2023-06-04 00:18:46 -04:00 • committed by GitHub
parent 3fa4440d87
commit 466719247b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files

78
crates/ruff/src/checkers/ast/mod.rs
View file

@ -4336,7 +4336,7 @@ impl<'a> Checker<'a> {
let binding = &self.semantic_model.bindings[binding_id]; let binding = &self.semantic_model.bindings[binding_id];
// Determine whether the binding shadows any existing bindings. // Determine whether the binding shadows any existing bindings.
if let Some((stack_index, existing_binding_id)) = self if let Some((stack_index, shadowed_id)) = self
.semantic_model .semantic_model
.scopes .scopes
.ancestors(self.semantic_model.scope_id) .ancestors(self.semantic_model.scope_id)
@ -4345,26 +4345,26 @@ impl<'a> Checker<'a> {
scope.get(name).map(|binding_id| (stack_index, binding_id)) scope.get(name).map(|binding_id| (stack_index, binding_id))
}) })
{ {
let existing = &self.semantic_model.bindings[existing_binding_id]; let shadowed = &self.semantic_model.bindings[shadowed_id];
let in_current_scope = stack_index == 0; let in_current_scope = stack_index == 0;
if !existing.kind.is_builtin() if !shadowed.kind.is_builtin()
&& existing.source.map_or(true, |left| { && shadowed.source.map_or(true, |left| {
binding.source.map_or(true, |right| { binding.source.map_or(true, |right| {
!branch_detection::different_forks(left, right, &self.semantic_model.stmts) !branch_detection::different_forks(left, right, &self.semantic_model.stmts)
}) })
}) })
{ {
let existing_is_import = matches!( let shadows_import = matches!(
existing.kind, shadowed.kind,
BindingKind::Importation(..) BindingKind::Importation(..)
| BindingKind::FromImportation(..) | BindingKind::FromImportation(..)
| BindingKind::SubmoduleImportation(..) | BindingKind::SubmoduleImportation(..)
| BindingKind::FutureImportation | BindingKind::FutureImportation
); );
if binding.kind.is_loop_var() && existing_is_import { if binding.kind.is_loop_var() && shadows_import {
if self.enabled(Rule::ImportShadowedByLoopVar) { if self.enabled(Rule::ImportShadowedByLoopVar) {
#[allow(deprecated)] #[allow(deprecated)]
let line = self.locator.compute_line_index(existing.range.start()); let line = self.locator.compute_line_index(shadowed.range.start());
self.diagnostics.push(Diagnostic::new( self.diagnostics.push(Diagnostic::new(
pyflakes::rules::ImportShadowedByLoopVar { pyflakes::rules::ImportShadowedByLoopVar {
@ -4375,21 +4375,21 @@ impl<'a> Checker<'a> {
)); ));
} }
} else if in_current_scope { } else if in_current_scope {
if !existing.is_used() if !shadowed.is_used()
&& binding.redefines(existing) && binding.redefines(shadowed)
&& (!self.settings.dummy_variable_rgx.is_match(name) || existing_is_import) && (!self.settings.dummy_variable_rgx.is_match(name) || shadows_import)
&& !(existing.kind.is_function_definition() && !(shadowed.kind.is_function_definition()
&& analyze::visibility::is_overload( && analyze::visibility::is_overload(
&self.semantic_model, &self.semantic_model,
cast::decorator_list( cast::decorator_list(
self.semantic_model.stmts[existing.source.unwrap()], self.semantic_model.stmts[shadowed.source.unwrap()],
), ),
)) ))
{ {
if self.enabled(Rule::RedefinedWhileUnused) { if self.enabled(Rule::RedefinedWhileUnused) {
#[allow(deprecated)] #[allow(deprecated)]
let line = self.locator.compute_line_index( let line = self.locator.compute_line_index(
existing shadowed
.trimmed_range(&self.semantic_model, self.locator) .trimmed_range(&self.semantic_model, self.locator)
.start(), .start(),
); );
@ -4412,34 +4412,32 @@ impl<'a> Checker<'a> {
self.diagnostics.push(diagnostic); self.diagnostics.push(diagnostic);
} }
} }
} else if existing_is_import && binding.redefines(existing) { } else if shadows_import && binding.redefines(shadowed) {
self.semantic_model self.semantic_model
.shadowed_bindings .shadowed_bindings
.entry(existing_binding_id) .insert(binding_id, shadowed_id);
.or_insert_with(Vec::new)
.push(binding_id);
} }
} }
} }
// If there's an existing binding in this scope, copy its references. // If there's an existing binding in this scope, copy its references.
if let Some(existing) = self.semantic_model.scopes[scope_id] if let Some(shadowed) = self.semantic_model.scopes[scope_id]
.get(name) .get(name)
.map(|binding_id| &self.semantic_model.bindings[binding_id]) .map(|binding_id| &self.semantic_model.bindings[binding_id])
{ {
match &existing.kind { match &shadowed.kind {
BindingKind::Builtin => { BindingKind::Builtin => {
// Avoid overriding builtins. // Avoid overriding builtins.
} }
kind @ (BindingKind::Global | BindingKind::Nonlocal) => { kind @ (BindingKind::Global | BindingKind::Nonlocal) => {
// If the original binding was a global or nonlocal, then the new binding is // If the original binding was a global or nonlocal, then the new binding is
// too. // too.
let references = existing.references.clone(); let references = shadowed.references.clone();
self.semantic_model.bindings[binding_id].kind = kind.clone(); self.semantic_model.bindings[binding_id].kind = kind.clone();
self.semantic_model.bindings[binding_id].references = references; self.semantic_model.bindings[binding_id].references = references;
} }
_ => { _ => {
let references = existing.references.clone(); let references = shadowed.references.clone();
self.semantic_model.bindings[binding_id].references = references; self.semantic_model.bindings[binding_id].references = references;
} }
} }
@ -5054,30 +5052,20 @@ impl<'a> Checker<'a> {
} }
// Look for any bindings that were redefined in another scope, and remain // Look for any bindings that were redefined in another scope, and remain
// unused. Note that we only store references in `redefinitions` if // unused. Note that we only store references in `shadowed_bindings` if
// the bindings are in different scopes. // the bindings are in different scopes.
if self.enabled(Rule::RedefinedWhileUnused) { if self.enabled(Rule::RedefinedWhileUnused) {
for (name, binding_id) in scope.bindings() { for (name, binding_id) in scope.bindings() {
let binding = &self.semantic_model.bindings[binding_id]; if let Some(shadowed) = self.semantic_model.shadowed_binding(binding_id) {
if shadowed.is_used() {
if matches!(
binding.kind,
BindingKind::Importation(..)
| BindingKind::FromImportation(..)
| BindingKind::SubmoduleImportation(..)
) {
if binding.is_used() {
continue; continue;
} }
if let Some(shadowed_ids) = let binding = &self.semantic_model.bindings[binding_id];
self.semantic_model.shadowed_bindings.get(&binding_id)
{
for binding_id in shadowed_ids.iter().copied() {
let rebound = &self.semantic_model.bindings[binding_id];
#[allow(deprecated)] #[allow(deprecated)]
let line = self.locator.compute_line_index( let line = self.locator.compute_line_index(
binding shadowed
.trimmed_range(&self.semantic_model, self.locator) .trimmed_range(&self.semantic_model, self.locator)
.start(), .start(),
); );
@ -5087,22 +5075,20 @@ impl<'a> Checker<'a> {
name: (*name).to_string(), name: (*name).to_string(),
line, line,
}, },
rebound.trimmed_range(&self.semantic_model, self.locator), binding.trimmed_range(&self.semantic_model, self.locator),
); );
if let Some(source) = rebound.source { if let Some(parent) = binding
let parent = &self.semantic_model.stmts[source]; .source
if matches!(parent, Stmt::ImportFrom(_)) .map(|source| &self.semantic_model.stmts[source])
&& parent.range().contains_range(rebound.range)
{ {
if parent.is_import_from_stmt() {
diagnostic.set_parent(parent.start()); diagnostic.set_parent(parent.start());
} }
}; }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
} }
} }
}
}
if enforce_typing_imports { if enforce_typing_imports {
let runtime_imports: Vec<&Binding> = if self.settings.flake8_type_checking.strict { let runtime_imports: Vec<&Binding> = if self.settings.flake8_type_checking.strict {

14
crates/ruff_python_semantic/src/model.rs
View file

@ -47,7 +47,7 @@ pub struct SemanticModel<'a> {
// Arena of global bindings. // Arena of global bindings.
globals: GlobalsArena<'a>, globals: GlobalsArena<'a>,
// Map from binding index to indexes of bindings that shadow it in other scopes. // Map from binding index to indexes of bindings that shadow it in other scopes.
pub shadowed_bindings: HashMap<BindingId, Vec<BindingId>, BuildNoHashHasher<BindingId>>, pub shadowed_bindings: HashMap<BindingId, BindingId, BuildNoHashHasher<BindingId>>,
// Body iteration; used to peek at siblings. // Body iteration; used to peek at siblings.
pub body: &'a [Stmt], pub body: &'a [Stmt],
pub body_index: usize, pub body_index: usize,
@ -145,13 +145,23 @@ impl<'a> SemanticModel<'a> {
}) })
} }
/// Return the current `Binding` for a given `name`. /// Return the current [`Binding`] for a given `name`.
pub fn find_binding(&self, member: &str) -> Option<&Binding> { pub fn find_binding(&self, member: &str) -> Option<&Binding> {
self.scopes() self.scopes()
.find_map(|scope| scope.get(member)) .find_map(|scope| scope.get(member))
.map(|binding_id| &self.bindings[binding_id]) .map(|binding_id| &self.bindings[binding_id])
} }
/// Return the [`Binding`] that the given [`BindingId`] shadows, if any.
///
/// Note that this will only return bindings that are shadowed by a binding in a parent scope.
pub fn shadowed_binding(&self, binding_id: BindingId) -> Option<&Binding> {
self.shadowed_bindings
.get(&binding_id)
.copied()
.map(|id| &self.bindings[id])
}
/// Return `true` if `member` is bound as a builtin. /// Return `true` if `member` is bound as a builtin.
pub fn is_builtin(&self, member: &str) -> bool { pub fn is_builtin(&self, member: &str) -> bool {
self.find_binding(member) self.find_binding(member)