diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 63f4bac476..6a83f24f5c 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -4123,12 +4123,10 @@ impl<'a> Checker<'a> { // in scope. let scope = self.ctx.scope_mut(); if !(binding.kind.is_annotation() && scope.defines(name)) { - if let Some(rebound_index) = scope.add(name, binding_id) { - scope - .rebounds - .entry(name) - .or_insert_with(Vec::new) - .push(rebound_index); + if let Some(qualified_name) = binding.kind.qualified_name() { + scope.add_import(name, qualified_name, binding_id); + } else { + scope.add(name, binding_id); } } diff --git a/crates/ruff_python_ast/src/binding.rs b/crates/ruff_python_ast/src/binding.rs index 95b4c0c29e..3f9782605a 100644 --- a/crates/ruff_python_ast/src/binding.rs +++ b/crates/ruff_python_ast/src/binding.rs @@ -265,6 +265,26 @@ pub enum BindingKind<'a> { SubmoduleImportation(SubmoduleImportation<'a>), } +impl<'a> BindingKind<'a> { + pub fn qualified_name(&self) -> Option<&'a str> { + match self { + BindingKind::Importation(importation) => Some(importation.full_name), + BindingKind::FromImportation(importation) => Some(&importation.full_name), + BindingKind::SubmoduleImportation(importation) => Some(importation.name), + _ => None, + } + } + + pub fn bound_name(&self) -> Option<&'a str> { + match self { + BindingKind::Importation(importation) => Some(importation.name), + BindingKind::FromImportation(importation) => Some(importation.name), + BindingKind::SubmoduleImportation(importation) => Some(importation.name), + _ => None, + } + } +} + bitflags! { pub struct Exceptions: u32 { const NAME_ERROR = 0b0000_0001; diff --git a/crates/ruff_python_ast/src/context.rs b/crates/ruff_python_ast/src/context.rs index cba1617889..6672b2a249 100644 --- a/crates/ruff_python_ast/src/context.rs +++ b/crates/ruff_python_ast/src/context.rs @@ -227,69 +227,34 @@ impl<'a> Context<'a> { member: &str, ) -> Option<(&Stmt, String)> { self.scopes().enumerate().find_map(|(scope_index, scope)| { - scope.binding_ids().find_map(|binding_index| { - let binding = &self.bindings[*binding_index]; - match &binding.kind { - // Ex) Given `module="sys"` and `object="exit"`: - // `import sys` -> `sys.exit` - // `import sys as sys2` -> `sys2.exit` - BindingKind::Importation(Importation { name, full_name }) => { - if full_name == &module { - // Verify that `sys` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - return Some(( - binding.source.as_ref().unwrap().into(), - format!("{name}.{member}"), - )); - } - } + if let Some(id) = scope.lookup(module) { + let binding = &self.bindings[*id]; + if let Some(name) = binding.kind.bound_name() { + if self + .scopes() + .take(scope_index) + .all(|scope| !scope.defines(name)) + { + return Some(( + binding.source.as_ref().unwrap().into(), + format!("{name}.{member}"), + )); } - // Ex) Given `module="os.path"` and `object="join"`: - // `from os.path import join` -> `join` - // `from os.path import join as join2` -> `join2` - BindingKind::FromImportation(FromImportation { name, full_name }) => { - if let Some((target_module, target_member)) = full_name.split_once('.') { - if target_module == module && target_member == member { - // Verify that `join` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - return Some(( - binding.source.as_ref().unwrap().into(), - (*name).to_string(), - )); - } - } - } - } - // Ex) Given `module="os"` and `object="name"`: - // `import os.path ` -> `os.name` - BindingKind::SubmoduleImportation(SubmoduleImportation { name, .. }) => { - if name == &module { - // Verify that `os` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - return Some(( - binding.source.as_ref().unwrap().into(), - format!("{name}.{member}"), - )); - } - } - } - // Non-imports. - _ => {} } - None - }) + } + if let Some(id) = scope.lookup(&format!("{module}.{member}")) { + let binding = &self.bindings[*id]; + if let Some(name) = binding.kind.bound_name() { + if self + .scopes() + .take(scope_index) + .all(|scope| !scope.defines(name)) + { + return Some((binding.source.as_ref().unwrap().into(), name.to_string())); + } + } + } + None }) } diff --git a/crates/ruff_python_ast/src/scope.rs b/crates/ruff_python_ast/src/scope.rs index 2224221eb8..d6d7b397f3 100644 --- a/crates/ruff_python_ast/src/scope.rs +++ b/crates/ruff_python_ast/src/scope.rs @@ -16,6 +16,10 @@ pub struct Scope<'a> { star_imports: Vec>, /// A map from bound name to binding index, for live bindings. bindings: FxHashMap<&'a str, BindingId>, + /// A map from fully-qualified name to binding index, for imported symbol. + imports: FxHashMap<&'a str, BindingId>, + /// A map from the keys in `bindings` to the keys in `imports`, for imported symbols. + name_to_qualified_name: FxHashMap<&'a str, &'a str>, /// 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. @@ -34,6 +38,8 @@ impl<'a> Scope<'a> { uses_locals: false, star_imports: Vec::default(), bindings: FxHashMap::default(), + imports: FxHashMap::default(), + name_to_qualified_name: FxHashMap::default(), rebounds: FxHashMap::default(), } } @@ -43,9 +49,33 @@ impl<'a> Scope<'a> { self.bindings.get(name) } + /// Returns the [id](BindingId) of the binding with the given name. + pub fn lookup(&self, qualified_name: &str) -> Option<&BindingId> { + self.imports.get(qualified_name) + } + /// Adds a new binding with the given name to this scope. pub fn add(&mut self, name: &'a str, id: BindingId) -> Option { - self.bindings.insert(name, id) + if let Some(id) = self.bindings.insert(name, id) { + if let Some(qualified_name) = self.name_to_qualified_name.remove(name) { + self.imports.remove(qualified_name); + } + Some(id) + } else { + None + } + } + + /// Adds a new binding with the given name to this scope. + pub fn add_import( + &mut self, + name: &'a str, + qualified_name: &'a str, + id: BindingId, + ) -> Option { + self.imports.insert(qualified_name, id); + self.name_to_qualified_name.insert(name, qualified_name); + self.add(name, id) } /// Returns `true` if this scope defines a binding with the given name. @@ -55,7 +85,14 @@ impl<'a> Scope<'a> { /// Removes the binding with the given name pub fn remove(&mut self, name: &str) -> Option { - self.bindings.remove(name) + if let Some(id) = self.bindings.remove(name) { + if let Some(qualified_name) = self.name_to_qualified_name.remove(name) { + self.imports.remove(qualified_name); + } + Some(id) + } else { + None + } } /// Returns the ids of all bindings defined in this scope.