From 15273c6d95bf34a11122eb1fefffb84ac808488d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 25 Jul 2023 22:16:48 -0400 Subject: [PATCH] Store the call path --- crates/ruff/src/checkers/ast/mod.rs | 21 +++++- .../unaliased_collections_abc_set_import.rs | 2 +- .../runtime_import_in_type_checking_block.rs | 2 +- crates/ruff/src/rules/pandas_vet/helpers.rs | 8 ++- .../pandas_vet/rules/inplace_argument.rs | 3 +- crates/ruff_python_semantic/src/binding.rs | 36 +++++----- crates/ruff_python_semantic/src/model.rs | 65 +++++++------------ foo.py | 6 ++ 8 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 foo.py diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 442000a3f2..9946f26aa8 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -26,6 +26,7 @@ //! represents the lint-rule analysis phase. In the future, these steps may be separated into //! distinct passes over the AST. +use std::iter::once; use std::path::Path; use itertools::Itertools; @@ -320,10 +321,14 @@ where // "foo.bar". let name = alias.name.split('.').next().unwrap(); let qualified_name = &alias.name; + let call_path: Box<[&str]> = qualified_name.split('.').collect(); self.add_binding( name, alias.identifier(), - BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }), + BindingKind::SubmoduleImport(SubmoduleImport { + qualified_name, + call_path, + }), BindingFlags::EXTERNAL, ); } else { @@ -341,10 +346,14 @@ where let name = alias.asname.as_ref().unwrap_or(&alias.name); let qualified_name = &alias.name; + let call_path: Box<[&str]> = qualified_name.split('.').collect(); self.add_binding( name, alias.identifier(), - BindingKind::Import(Import { qualified_name }), + BindingKind::Import(Import { + qualified_name, + call_path, + }), flags, ); } @@ -390,10 +399,16 @@ where let name = alias.asname.as_ref().unwrap_or(&alias.name); let qualified_name = helpers::format_import_from_member(level, module, &alias.name); + let module = module.unwrap(); + let call_path: Box<[&str]> = + module.split('.').chain(once(alias.name.as_str())).collect(); self.add_binding( name, alias.identifier(), - BindingKind::FromImport(FromImport { qualified_name }), + BindingKind::FromImport(FromImport { + qualified_name, + call_path, + }), flags, ); } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs b/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs index 6afd64b25b..dc91c5330b 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs @@ -50,7 +50,7 @@ pub(crate) fn unaliased_collections_abc_set_import( checker: &Checker, binding: &Binding, ) -> Option { - let BindingKind::FromImport(FromImport { qualified_name }) = &binding.kind else { + let BindingKind::FromImport(FromImport { qualified_name, .. }) = &binding.kind else { return None; }; if qualified_name.as_str() != "collections.abc.Set" { diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index ea4899c572..08f2c91d39 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -51,7 +51,7 @@ impl Violation for RuntimeImportInTypeCheckingBlock { #[derive_message_formats] fn message(&self) -> String { - let RuntimeImportInTypeCheckingBlock { qualified_name } = self; + let RuntimeImportInTypeCheckingBlock { qualified_name, .. } = self; format!( "Move import `{qualified_name}` out of type-checking block. Import is used for more than type hinting." ) diff --git a/crates/ruff/src/rules/pandas_vet/helpers.rs b/crates/ruff/src/rules/pandas_vet/helpers.rs index 9da577d606..797dfa6901 100644 --- a/crates/ruff/src/rules/pandas_vet/helpers.rs +++ b/crates/ruff/src/rules/pandas_vet/helpers.rs @@ -39,9 +39,11 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti | BindingKind::LoopVar | BindingKind::Global | BindingKind::Nonlocal(_) => Resolution::RelevantLocal, - BindingKind::Import(Import { - qualified_name: module, - }) if module == "pandas" => Resolution::PandasModule, + BindingKind::Import(Import { qualified_name, .. }) + if qualified_name == "pandas" => + { + Resolution::PandasModule + } _ => Resolution::IrrelevantBinding, } }) diff --git a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs index 543c9cc2f2..c3a01fb65c 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs @@ -68,7 +68,8 @@ pub(crate) fn inplace_argument( matches!( binding.kind, BindingKind::Import(Import { - qualified_name: "pandas" + qualified_name: "pandas", + .. }) ) }) diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 7dd9056dd6..252219846a 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -5,6 +5,7 @@ use ruff_python_ast::Ranged; use ruff_text_size::TextRange; use ruff_index::{newtype_index, IndexSlice, IndexVec}; +use ruff_python_ast::source_code::Locator; use ruff_source_file::Locator; use crate::context::ExecutionContext; @@ -117,38 +118,38 @@ impl<'a> Binding<'a> { // import foo.baz // ``` BindingKind::Import(Import { - qualified_name: redefinition, + call_path: redefinition, }) => { if let BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: definition, + call_path: definition, }) = &existing.kind { return redefinition == definition; } } BindingKind::FromImport(FromImport { - qualified_name: redefinition, + call_path: redefinition, }) => { if let BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: definition, + call_path: definition, }) = &existing.kind { return redefinition == definition; } } BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: redefinition, + call_path: redefinition, }) => match &existing.kind { BindingKind::Import(Import { - qualified_name: definition, + call_path: definition, }) | BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: definition, + call_path: definition, }) => { return redefinition == definition; } BindingKind::FromImport(FromImport { - qualified_name: definition, + call_path: definition, }) => { return redefinition == definition; } @@ -178,9 +179,9 @@ impl<'a> Binding<'a> { /// Returns the fully-qualified symbol name, if this symbol was imported from another module. pub fn qualified_name(&self) -> Option<&str> { match &self.kind { - BindingKind::Import(Import { qualified_name }) => Some(qualified_name), - BindingKind::FromImport(FromImport { qualified_name }) => Some(qualified_name), - BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => { + BindingKind::Import(Import { qualified_name, .. }) => Some(qualified_name), + BindingKind::FromImport(FromImport { qualified_name, .. }) => Some(qualified_name), + BindingKind::SubmoduleImport(SubmoduleImport { qualified_name, .. }) => { Some(qualified_name) } _ => None, @@ -191,11 +192,11 @@ impl<'a> Binding<'a> { /// symbol was imported from another module. pub fn module_name(&self) -> Option<&str> { match &self.kind { - BindingKind::Import(Import { qualified_name }) - | BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => { + BindingKind::Import(Import { qualified_name, .. }) + | BindingKind::SubmoduleImport(SubmoduleImport { qualified_name, .. }) => { Some(qualified_name.split('.').next().unwrap_or(qualified_name)) } - BindingKind::FromImport(FromImport { qualified_name }) => Some( + BindingKind::FromImport(FromImport { qualified_name, .. }) => Some( qualified_name .rsplit_once('.') .map_or(qualified_name, |(module, _)| module), @@ -357,17 +358,19 @@ pub struct Import<'a> { /// Ex) Given `import foo`, `qualified_name` would be "foo". /// Ex) Given `import foo as bar`, `qualified_name` would be "foo". pub qualified_name: &'a str, + pub call_path: Box<[&'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(Debug, Clone)] -pub struct FromImport { +pub struct FromImport<'a> { /// The full name of the member being imported. /// Ex) Given `from foo import bar`, `qualified_name` would be "foo.bar". /// Ex) Given `from foo import bar as baz`, `qualified_name` would be "foo.bar". pub qualified_name: String, + pub call_path: Box<[&'a str]>, } /// A binding for a submodule imported from a module, keyed on the name of the parent module. @@ -377,6 +380,7 @@ pub struct SubmoduleImport<'a> { /// The full name of the submodule being imported. /// Ex) Given `import foo.bar`, `qualified_name` would be "foo.bar". pub qualified_name: &'a str, + pub call_path: Box<[&'a str]>, } #[derive(Debug, Clone, is_macro::Is)] @@ -485,7 +489,7 @@ pub enum BindingKind<'a> { /// ```python /// from foo import bar /// ``` - FromImport(FromImport), + FromImport(FromImport<'a>), /// A binding for a submodule imported from a module, like `bar` in: /// ```python diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index fc66f668de..13ac52113a 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1,15 +1,14 @@ use std::path::Path; use bitflags::bitflags; -use ruff_python_ast::{self as ast, Expr, Ranged, Stmt}; -use ruff_text_size::{TextRange, TextSize}; use rustc_hash::FxHashMap; -use smallvec::smallvec; +use smallvec::{smallvec, SmallVec}; use ruff_python_ast::call_path::{collect_call_path, from_unqualified_name, CallPath}; -use ruff_python_ast::helpers::from_relative_import; +use ruff_python_ast::{self as ast, Expr, Ranged, Stmt}; use ruff_python_stdlib::path::is_python_stub_file; use ruff_python_stdlib::typing::is_typing_extension; +use ruff_text_size::{TextRange, TextSize}; use crate::binding::{ Binding, BindingFlags, BindingId, BindingKind, Bindings, Exceptions, FromImport, Import, @@ -633,46 +632,32 @@ impl<'a> SemanticModel<'a> { .or_else(|| self.find_binding(&head.id))?; match &binding.kind { - BindingKind::Import(Import { - qualified_name: name, - }) => { - let call_path = collect_call_path(value)?; - let (_, tail) = call_path.split_first()?; + BindingKind::Import(Import { call_path, .. }) => { + let x = collect_call_path(value)?; + let (_, tail) = x.split_first()?; + let mut resolved = SmallVec::with_capacity(call_path.len() + tail.len()); + resolved.extend_from_slice(call_path); + resolved.extend_from_slice(tail); + Some(resolved) + } + BindingKind::SubmoduleImport(SubmoduleImport { qualified_name, .. }) => { + let x = collect_call_path(value)?; + let (_, tail) = x.split_first()?; + + let name = qualified_name.split('.').next().unwrap_or(qualified_name); let mut source_path: CallPath = from_unqualified_name(name); source_path.extend_from_slice(tail); Some(source_path) } - BindingKind::SubmoduleImport(SubmoduleImport { - qualified_name: name, - }) => { - let call_path = collect_call_path(value)?; - let (_, tail) = call_path.split_first()?; + BindingKind::FromImport(FromImport { call_path, .. }) => { + let x = collect_call_path(value)?; + let (_, tail) = x.split_first()?; - let name = name.split('.').next().unwrap_or(name); - let mut source_path: CallPath = from_unqualified_name(name); - source_path.extend_from_slice(tail); - Some(source_path) - } - BindingKind::FromImport(FromImport { - qualified_name: name, - }) => { - let call_path = collect_call_path(value)?; - let (_, tail) = call_path.split_first()?; - - if name.starts_with('.') { - let mut source_path = from_relative_import(self.module_path?, name); - if source_path.is_empty() { - None - } else { - source_path.extend_from_slice(tail); - Some(source_path) - } - } else { - let mut source_path: CallPath = from_unqualified_name(name); - source_path.extend_from_slice(tail); - Some(source_path) - } + let mut resolved = SmallVec::with_capacity(call_path.len() + tail.len()); + resolved.extend_from_slice(call_path); + resolved.extend_from_slice(tail); + Some(resolved) } BindingKind::Builtin => Some(smallvec!["", head.id.as_str()]), _ => None, @@ -703,7 +688,7 @@ impl<'a> SemanticModel<'a> { // Ex) Given `module="sys"` and `object="exit"`: // `import sys` -> `sys.exit` // `import sys as sys2` -> `sys2.exit` - BindingKind::Import(Import { qualified_name }) => { + BindingKind::Import(Import { qualified_name, .. }) => { if qualified_name == &module { if let Some(source) = binding.source { // Verify that `sys` isn't bound in an inner scope. @@ -724,7 +709,7 @@ impl<'a> SemanticModel<'a> { // Ex) Given `module="os.path"` and `object="join"`: // `from os.path import join` -> `join` // `from os.path import join as join2` -> `join2` - BindingKind::FromImport(FromImport { qualified_name }) => { + BindingKind::FromImport(FromImport { qualified_name, .. }) => { if let Some((target_module, target_member)) = qualified_name.split_once('.') { if target_module == module && target_member == member { diff --git a/foo.py b/foo.py new file mode 100644 index 0000000000..e441803c4d --- /dev/null +++ b/foo.py @@ -0,0 +1,6 @@ + + +print( + "Unexpected changes:\n" + "\n".join(["1", "2", "3"]) +)