Remove name field from import binding kinds (#4817)

This commit is contained in:
Charlie Marsh 2023-06-02 23:02:47 -04:00 committed by GitHub
parent fcfd6ad129
commit 26b1dd0ca2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 88 additions and 120 deletions

View file

@ -872,7 +872,6 @@ where
name, name,
Binding { Binding {
kind: BindingKind::SubmoduleImportation(SubmoduleImportation { kind: BindingKind::SubmoduleImportation(SubmoduleImportation {
name,
full_name, full_name,
}), }),
range: alias.range(), range: alias.range(),
@ -889,7 +888,7 @@ where
self.add_binding( self.add_binding(
name, name,
Binding { Binding {
kind: BindingKind::Importation(Importation { name, full_name }), kind: BindingKind::Importation(Importation { full_name }),
range: alias.range(), range: alias.range(),
references: Vec::new(), references: Vec::new(),
source: self.semantic_model.stmt_id, source: self.semantic_model.stmt_id,
@ -1196,10 +1195,7 @@ where
self.add_binding( self.add_binding(
name, name,
Binding { Binding {
kind: BindingKind::FromImportation(FromImportation { kind: BindingKind::FromImportation(FromImportation { full_name }),
name,
full_name,
}),
range: alias.range(), range: alias.range(),
references: Vec::new(), references: Vec::new(),
source: self.semantic_model.stmt_id, source: self.semantic_model.stmt_id,

View file

@ -67,9 +67,9 @@ pub(crate) fn runtime_import_in_type_checking_block(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
) { ) {
let full_name = match &binding.kind { let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name, .. }) => full_name, BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(), BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => full_name, BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => return, _ => return,
}; };

View file

@ -183,61 +183,50 @@ fn is_implicit_import(this: &Binding, that: &Binding) -> bool {
match &this.kind { match &this.kind {
BindingKind::Importation(Importation { BindingKind::Importation(Importation {
full_name: this_name, full_name: this_name,
..
}) })
| BindingKind::SubmoduleImportation(SubmoduleImportation { | BindingKind::SubmoduleImportation(SubmoduleImportation {
name: this_name, .. full_name: this_name,
}) => match &that.kind { }) => match &that.kind {
BindingKind::FromImportation(FromImportation { BindingKind::FromImportation(FromImportation {
full_name: that_name, full_name: that_name,
..
}) => { }) => {
// Ex) `pkg.A` vs. `pkg` // Ex) `pkg.A` vs. `pkg`
this_name let this_name = this_name.split('.').next().unwrap_or(this_name);
that_name
.rfind('.') .rfind('.')
.map_or(false, |i| this_name[..i] == *that_name) .map_or(false, |i| that_name[..i] == *this_name)
} }
BindingKind::Importation(Importation { BindingKind::Importation(Importation {
full_name: that_name, full_name: that_name,
..
}) })
| BindingKind::SubmoduleImportation(SubmoduleImportation { | BindingKind::SubmoduleImportation(SubmoduleImportation {
name: that_name, .. full_name: that_name,
}) => { }) => {
// Submodule importation with an alias (`import pkg.A as B`) // Submodule importation with an alias (`import pkg.A as B`)
// are represented as `Importation`. // are represented as `Importation`.
match (this_name.find('.'), that_name.find('.')) { let this_name = this_name.split('.').next().unwrap_or(this_name);
// Ex) `pkg.A` vs. `pkg.B` let that_name = that_name.split('.').next().unwrap_or(that_name);
(Some(i), Some(j)) => this_name[..i] == that_name[..j], this_name == that_name
// Ex) `pkg.A` vs. `pkg`
(Some(i), None) => this_name[..i] == **that_name,
// Ex) `pkg` vs. `pkg.B`
(None, Some(j)) => **this_name == that_name[..j],
// Ex) `pkg` vs. `pkg`
(None, None) => this_name == that_name,
}
} }
_ => false, _ => false,
}, },
BindingKind::FromImportation(FromImportation { BindingKind::FromImportation(FromImportation {
full_name: this_name, full_name: this_name,
..
}) => match &that.kind { }) => match &that.kind {
BindingKind::Importation(Importation { BindingKind::Importation(Importation {
full_name: that_name, full_name: that_name,
..
}) })
| BindingKind::SubmoduleImportation(SubmoduleImportation { | BindingKind::SubmoduleImportation(SubmoduleImportation {
name: that_name, .. full_name: that_name,
}) => { }) => {
// Ex) `pkg.A` vs. `pkg` // Ex) `pkg.A` vs. `pkg`
let that_name = that_name.split('.').next().unwrap_or(that_name);
this_name this_name
.rfind('.') .rfind('.')
.map_or(false, |i| &this_name[..i] == *that_name) .map_or(false, |i| &this_name[..i] == that_name)
} }
BindingKind::FromImportation(FromImportation { BindingKind::FromImportation(FromImportation {
full_name: that_name, full_name: that_name,
..
}) => { }) => {
// Ex) `pkg.A` vs. `pkg.B` // Ex) `pkg.A` vs. `pkg.B`
this_name.rfind('.').map_or(false, |i| { this_name.rfind('.').map_or(false, |i| {
@ -286,9 +275,9 @@ pub(crate) fn typing_only_runtime_import(
} }
let full_name = match &binding.kind { let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name, .. }) => full_name, BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(), BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => full_name, BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => return, _ => return,
}; };

View file

@ -40,9 +40,11 @@ pub(crate) fn test_expression(expr: &Expr, model: &SemanticModel) -> Resolution
| BindingKind::LoopVar | BindingKind::LoopVar
| BindingKind::Global | BindingKind::Global
| BindingKind::Nonlocal => Resolution::RelevantLocal, | BindingKind::Nonlocal => Resolution::RelevantLocal,
BindingKind::Importation(Importation { BindingKind::Importation(Importation { full_name: module })
full_name: module, .. if module == "pandas" =>
}) if module == "pandas" => Resolution::PandasModule, {
Resolution::PandasModule
}
_ => Resolution::IrrelevantBinding, _ => Resolution::IrrelevantBinding,
} }
}) })

View file

@ -71,8 +71,7 @@ pub(crate) fn inplace_argument(
matches!( matches!(
binding.kind, binding.kind,
BindingKind::Importation(Importation { BindingKind::Importation(Importation {
full_name: "pandas", full_name: "pandas"
..
}) })
) )
}); });

View file

@ -118,9 +118,9 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
} }
let full_name = match &binding.kind { let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name, .. }) => full_name, BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(), BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => full_name, BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => continue, _ => continue,
}; };

View file

@ -48,39 +48,34 @@ impl<'a> Binding<'a> {
/// Return `true` if this binding redefines the given binding. /// Return `true` if this binding redefines the given binding.
pub fn redefines(&self, existing: &'a Binding) -> bool { pub fn redefines(&self, existing: &'a Binding) -> bool {
match &self.kind { match &self.kind {
BindingKind::Importation(Importation { full_name, .. }) => { BindingKind::Importation(Importation { full_name }) => {
if let BindingKind::SubmoduleImportation(SubmoduleImportation { if let BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing, full_name: existing,
..
}) = &existing.kind }) = &existing.kind
{ {
return full_name == existing; return full_name == existing;
} }
} }
BindingKind::FromImportation(FromImportation { full_name, .. }) => { BindingKind::FromImportation(FromImportation { full_name }) => {
if let BindingKind::SubmoduleImportation(SubmoduleImportation { if let BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing, full_name: existing,
..
}) = &existing.kind }) = &existing.kind
{ {
return full_name == existing; return full_name == existing;
} }
} }
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => { BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => {
match &existing.kind { match &existing.kind {
BindingKind::Importation(Importation { BindingKind::Importation(Importation {
full_name: existing, full_name: existing,
..
}) })
| BindingKind::SubmoduleImportation(SubmoduleImportation { | BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing, full_name: existing,
..
}) => { }) => {
return full_name == existing; return full_name == existing;
} }
BindingKind::FromImportation(FromImportation { BindingKind::FromImportation(FromImportation {
full_name: existing, full_name: existing,
..
}) => { }) => {
return full_name == existing; return full_name == existing;
} }
@ -211,37 +206,34 @@ pub struct Export<'a> {
pub names: Vec<&'a str>, pub names: Vec<&'a str>,
} }
/// A binding for an `import`, keyed on the name to which the import is bound.
/// Ex) `import foo` would be keyed on "foo".
/// Ex) `import foo as bar` would be keyed on "bar".
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct Importation<'a> { pub struct Importation<'a> {
/// The name to which the import is bound.
/// Given `import foo`, `name` would be "foo".
/// Given `import foo as bar`, `name` would be "bar".
pub name: &'a str,
/// The full name of the module being imported. /// The full name of the module being imported.
/// Given `import foo`, `full_name` would be "foo". /// Ex) Given `import foo`, `full_name` would be "foo".
/// Given `import foo as bar`, `full_name` would be "foo". /// Ex) Given `import foo as bar`, `full_name` would be "foo".
pub full_name: &'a str, pub full_name: &'a str,
} }
/// A binding for a member imported from a module, keyed on the name to which the member is bound.
/// Ex) `from foo import bar` would be keyed on "bar".
/// Ex) `from foo import bar as baz` would be keyed on "baz".
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct FromImportation<'a> { pub struct FromImportation {
/// The name to which the import is bound. /// The full name of the member being imported.
/// Given `from foo import bar`, `name` would be "bar". /// Ex) Given `from foo import bar`, `full_name` would be "foo.bar".
/// Given `from foo import bar as baz`, `name` would be "baz". /// Ex) Given `from foo import bar as baz`, `full_name` would be "foo.bar".
pub name: &'a str,
/// The full name of the module being imported.
/// Given `from foo import bar`, `full_name` would be "foo.bar".
/// Given `from foo import bar as baz`, `full_name` would be "foo.bar".
pub full_name: String, pub full_name: String,
} }
/// A binding for a submodule imported from a module, keyed on the name of the parent module.
/// Ex) `import foo.bar` would be keyed on "foo".
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct SubmoduleImportation<'a> { pub struct SubmoduleImportation<'a> {
/// The parent module imported by the submodule import.
/// Given `import foo.bar`, `module` would be "foo".
pub name: &'a str,
/// The full name of the submodule being imported. /// The full name of the submodule being imported.
/// Given `import foo.bar`, `full_name` would be "foo.bar". /// Ex) Given `import foo.bar`, `full_name` would be "foo.bar".
pub full_name: &'a str, pub full_name: &'a str,
} }
@ -261,7 +253,7 @@ pub enum BindingKind<'a> {
Export(Export<'a>), Export(Export<'a>),
FutureImportation, FutureImportation,
Importation(Importation<'a>), Importation(Importation<'a>),
FromImportation(FromImportation<'a>), FromImportation(FromImportation),
SubmoduleImportation(SubmoduleImportation<'a>), SubmoduleImportation(SubmoduleImportation<'a>),
} }

View file

@ -145,7 +145,9 @@ impl<'a> SemanticModel<'a> {
self.bindings[binding_id].references.push(reference_id); self.bindings[binding_id].references.push(reference_id);
// Mark any submodule aliases as used. // Mark any submodule aliases as used.
if let Some(binding_id) = self.resolve_submodule(ScopeId::global(), binding_id) { if let Some(binding_id) =
self.resolve_submodule(symbol, ScopeId::global(), binding_id)
{
let reference_id = self.references.push(ScopeId::global(), range, context); let reference_id = self.references.push(ScopeId::global(), range, context);
self.bindings[binding_id].references.push(reference_id); self.bindings[binding_id].references.push(reference_id);
} }
@ -181,7 +183,7 @@ impl<'a> SemanticModel<'a> {
self.bindings[binding_id].references.push(reference_id); self.bindings[binding_id].references.push(reference_id);
// Mark any submodule aliases as used. // Mark any submodule aliases as used.
if let Some(binding_id) = self.resolve_submodule(scope_id, binding_id) { if let Some(binding_id) = self.resolve_submodule(symbol, scope_id, binding_id) {
let reference_id = self.references.push(self.scope_id, range, context); let reference_id = self.references.push(self.scope_id, range, context);
self.bindings[binding_id].references.push(reference_id); self.bindings[binding_id].references.push(reference_id);
} }
@ -238,7 +240,12 @@ impl<'a> SemanticModel<'a> {
} }
/// Given a `BindingId`, return the `BindingId` of the submodule import that it aliases. /// Given a `BindingId`, return the `BindingId` of the submodule import that it aliases.
fn resolve_submodule(&self, scope_id: ScopeId, binding_id: BindingId) -> Option<BindingId> { fn resolve_submodule(
&self,
symbol: &str,
scope_id: ScopeId,
binding_id: BindingId,
) -> Option<BindingId> {
// If the name of a submodule import is the same as an alias of another import, and the // If the name of a submodule import is the same as an alias of another import, and the
// alias is used, then the submodule import should be marked as used too. // alias is used, then the submodule import should be marked as used too.
// //
@ -249,21 +256,17 @@ impl<'a> SemanticModel<'a> {
// import pyarrow.csv // import pyarrow.csv
// print(pa.csv.read_csv("test.csv")) // print(pa.csv.read_csv("test.csv"))
// ``` // ```
let (name, full_name) = match &self.bindings[binding_id].kind { let full_name = match &self.bindings[binding_id].kind {
BindingKind::Importation(Importation { name, full_name }) => (*name, *full_name), BindingKind::Importation(Importation { full_name }) => *full_name,
BindingKind::SubmoduleImportation(SubmoduleImportation { name, full_name }) => { BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => *full_name,
(*name, *full_name) BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
}
BindingKind::FromImportation(FromImportation { name, full_name }) => {
(*name, full_name.as_str())
}
_ => return None, _ => return None,
}; };
let has_alias = full_name let has_alias = full_name
.split('.') .split('.')
.last() .last()
.map(|segment| segment != name) .map(|segment| segment != symbol)
.unwrap_or_default(); .unwrap_or_default();
if !has_alias { if !has_alias {
return None; return None;
@ -285,31 +288,18 @@ impl<'a> SemanticModel<'a> {
/// ///
/// ...then `resolve_call_path(${python_version})` will resolve to `sys.version_info`. /// ...then `resolve_call_path(${python_version})` will resolve to `sys.version_info`.
pub fn resolve_call_path(&'a self, value: &'a Expr) -> Option<CallPath<'a>> { pub fn resolve_call_path(&'a self, value: &'a Expr) -> Option<CallPath<'a>> {
let Some(call_path) = collect_call_path(value) else { let call_path = collect_call_path(value)?;
return None; let head = call_path.first()?;
}; let binding = self.find_binding(head)?;
let Some(head) = call_path.first() else {
return None;
};
let Some(binding) = self.find_binding(head) else {
return None;
};
match &binding.kind { match &binding.kind {
BindingKind::Importation(Importation { BindingKind::Importation(Importation { full_name: name }) => {
full_name: name, ..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation { name, .. }) => {
if name.starts_with('.') { if name.starts_with('.') {
if let Some(module) = &self.module_path { let mut source_path = from_relative_import(self.module_path?, name);
let mut source_path = from_relative_import(module, name); if source_path.is_empty() {
if source_path.is_empty() {
None
} else {
source_path.extend(call_path.into_iter().skip(1));
Some(source_path)
}
} else {
None None
} else {
source_path.extend(call_path.into_iter().skip(1));
Some(source_path)
} }
} else { } else {
let mut source_path: CallPath = from_unqualified_name(name); let mut source_path: CallPath = from_unqualified_name(name);
@ -317,20 +307,20 @@ impl<'a> SemanticModel<'a> {
Some(source_path) Some(source_path)
} }
} }
BindingKind::FromImportation(FromImportation { BindingKind::SubmoduleImportation(SubmoduleImportation { full_name: name }) => {
full_name: name, .. let name = name.split('.').next().unwrap_or(name);
}) => { let mut source_path: CallPath = from_unqualified_name(name);
source_path.extend(call_path.into_iter().skip(1));
Some(source_path)
}
BindingKind::FromImportation(FromImportation { full_name: name }) => {
if name.starts_with('.') { if name.starts_with('.') {
if let Some(module) = &self.module_path { let mut source_path = from_relative_import(self.module_path?, name);
let mut source_path = from_relative_import(module, name); if source_path.is_empty() {
if source_path.is_empty() {
None
} else {
source_path.extend(call_path.into_iter().skip(1));
Some(source_path)
}
} else {
None None
} else {
source_path.extend(call_path.into_iter().skip(1));
Some(source_path)
} }
} else { } else {
let mut source_path: CallPath = from_unqualified_name(name); let mut source_path: CallPath = from_unqualified_name(name);
@ -366,13 +356,13 @@ impl<'a> SemanticModel<'a> {
member: &str, member: &str,
) -> Option<ImportedName> { ) -> Option<ImportedName> {
self.scopes().enumerate().find_map(|(scope_index, scope)| { self.scopes().enumerate().find_map(|(scope_index, scope)| {
scope.binding_ids().find_map(|binding_id| { scope.bindings().find_map(|(name, binding_id)| {
let binding = &self.bindings[binding_id]; let binding = &self.bindings[binding_id];
match &binding.kind { match &binding.kind {
// Ex) Given `module="sys"` and `object="exit"`: // Ex) Given `module="sys"` and `object="exit"`:
// `import sys` -> `sys.exit` // `import sys` -> `sys.exit`
// `import sys as sys2` -> `sys2.exit` // `import sys as sys2` -> `sys2.exit`
BindingKind::Importation(Importation { name, full_name }) => { BindingKind::Importation(Importation { full_name }) => {
if full_name == &module { if full_name == &module {
if let Some(source) = binding.source { if let Some(source) = binding.source {
// Verify that `sys` isn't bound in an inner scope. // Verify that `sys` isn't bound in an inner scope.
@ -393,7 +383,7 @@ impl<'a> SemanticModel<'a> {
// Ex) Given `module="os.path"` and `object="join"`: // Ex) Given `module="os.path"` and `object="join"`:
// `from os.path import join` -> `join` // `from os.path import join` -> `join`
// `from os.path import join as join2` -> `join2` // `from os.path import join as join2` -> `join2`
BindingKind::FromImportation(FromImportation { name, full_name }) => { BindingKind::FromImportation(FromImportation { full_name }) => {
if let Some((target_module, target_member)) = full_name.split_once('.') { if let Some((target_module, target_member)) = full_name.split_once('.') {
if target_module == module && target_member == member { if target_module == module && target_member == member {
if let Some(source) = binding.source { if let Some(source) = binding.source {
@ -415,8 +405,8 @@ impl<'a> SemanticModel<'a> {
} }
// Ex) Given `module="os"` and `object="name"`: // Ex) Given `module="os"` and `object="name"`:
// `import os.path ` -> `os.name` // `import os.path ` -> `os.name`
BindingKind::SubmoduleImportation(SubmoduleImportation { name, .. }) => { BindingKind::SubmoduleImportation(SubmoduleImportation { .. }) => {
if name == &module { if name == module {
if let Some(source) = binding.source { if let Some(source) = binding.source {
// Verify that `os` isn't bound in an inner scope. // Verify that `os` isn't bound in an inner scope.
if self if self