From 27fb42ce8e4456c2618959af2d0096570ad1b888 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 6 Mar 2024 09:55:59 +0100 Subject: [PATCH] refactor: Use `QualifiedName` for `Imported::call_path` (#10214) ## Summary When you try to remove an internal representation leaking into another type and end up rewriting a simple version of `smallvec`. The goal of this PR is to replace the `Box<[&'a str]>` with `Box` to avoid that the internal `QualifiedName` representation leaks (and it gives us a nicer API too). However, doing this when `QualifiedName` uses `SmallVec` internally gives us all sort of funny lifetime errors. I was lost but @BurntSushi came to rescue me. He figured out that `smallvec` has a variance problem which is already tracked in https://github.com/servo/rust-smallvec/issues/146 To fix the variants problem, I could use the smallvec-2-alpha-4 or implement our own smallvec. I went with implementing our own small vec for this specific problem. It obviously isn't as sophisticated as smallvec (only uses safe code), e.g. it doesn't perform any size optimizations, but it does its job. Other changes: * Removed `Imported::qualified_name` (the version that returns a `String`). This can be replaced by calling `ToString` on the qualified name. * Renamed `Imported::call_path` to `qualified_name` and changed its return type to `&QualifiedName`. * Renamed `QualifiedName::imported` to `user_defined` which is the more common term when talking about builtins vs the rest/user defined functions. ## Test plan `cargo test` --- Cargo.lock | 2 - crates/ruff_linter/src/checkers/ast/mod.rs | 20 +- crates/ruff_linter/src/renamer.rs | 2 +- .../rules/flake8_debugger/rules/debugger.rs | 9 +- .../rules/unconventional_import_alias.rs | 2 +- .../unaliased_collections_abc_set_import.rs | 5 +- .../runtime_import_in_type_checking_block.rs | 6 +- .../rules/typing_only_runtime_import.rs | 16 +- .../src/rules/pandas_vet/helpers.rs | 4 +- .../src/rules/pyflakes/rules/unused_import.rs | 4 +- .../rules/pylint/rules/import_private_name.rs | 17 +- crates/ruff_linter/src/rules/ruff/typing.rs | 12 +- crates/ruff_python_ast/Cargo.toml | 1 - crates/ruff_python_ast/src/name.rs | 901 +++++++++++++----- crates/ruff_python_semantic/Cargo.toml | 1 - .../src/analyze/typing.rs | 2 +- crates/ruff_python_semantic/src/binding.rs | 47 +- crates/ruff_python_semantic/src/model.rs | 38 +- 18 files changed, 772 insertions(+), 317 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ed7b92cdd8614..5828cfc4d76fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2221,7 +2221,6 @@ dependencies = [ "ruff_text_size", "rustc-hash", "serde", - "smallvec", ] [[package]] @@ -2338,7 +2337,6 @@ dependencies = [ "ruff_source_file", "ruff_text_size", "rustc-hash", - "smallvec", ] [[package]] diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index feabe4adede31..ad320819e550b 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -43,6 +43,7 @@ use ruff_python_ast::helpers::{ collect_import_from_member, extract_handled_exceptions, is_docstring_stmt, to_module_path, }; use ruff_python_ast::identifier::Identifier; +use ruff_python_ast::name::QualifiedName; use ruff_python_ast::str::trailing_quote; use ruff_python_ast::visitor::{walk_except_handler, walk_f_string_element, walk_pattern, Visitor}; use ruff_python_ast::{helpers, str, visitor, PySourceType}; @@ -418,11 +419,13 @@ impl<'a> Visitor<'a> for Checker<'a> { self.semantic.add_module(module); if alias.asname.is_none() && alias.name.contains('.') { - let qualified_name: Box<[&str]> = alias.name.split('.').collect(); + let qualified_name = QualifiedName::user_defined(&alias.name); self.add_binding( module, alias.identifier(), - BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }), + BindingKind::SubmoduleImport(SubmoduleImport { + qualified_name: Box::new(qualified_name), + }), BindingFlags::EXTERNAL, ); } else { @@ -439,11 +442,13 @@ impl<'a> Visitor<'a> for Checker<'a> { } let name = alias.asname.as_ref().unwrap_or(&alias.name); - let qualified_name: Box<[&str]> = alias.name.split('.').collect(); + let qualified_name = QualifiedName::user_defined(&alias.name); self.add_binding( name, alias.identifier(), - BindingKind::Import(Import { qualified_name }), + BindingKind::Import(Import { + qualified_name: Box::new(qualified_name), + }), flags, ); } @@ -503,12 +508,13 @@ impl<'a> Visitor<'a> for Checker<'a> { // Attempt to resolve any relative imports; but if we don't know the current // module path, or the relative import extends beyond the package root, // fallback to a literal representation (e.g., `[".", "foo"]`). - let qualified_name = collect_import_from_member(level, module, &alias.name) - .into_boxed_slice(); + let qualified_name = collect_import_from_member(level, module, &alias.name); self.add_binding( name, alias.identifier(), - BindingKind::FromImport(FromImport { qualified_name }), + BindingKind::FromImport(FromImport { + qualified_name: Box::new(qualified_name), + }), flags, ); } diff --git a/crates/ruff_linter/src/renamer.rs b/crates/ruff_linter/src/renamer.rs index 4fa303d462ab9..8f4d560afe6ed 100644 --- a/crates/ruff_linter/src/renamer.rs +++ b/crates/ruff_linter/src/renamer.rs @@ -232,7 +232,7 @@ impl Renamer { } BindingKind::SubmoduleImport(import) => { // Ex) Rename `import pandas.core` to `import pandas as pd`. - let module_name = import.qualified_name.first().unwrap(); + let module_name = import.qualified_name.segments().first().unwrap(); Some(Edit::range_replacement( format!("{module_name} as {target}"), binding.range(), diff --git a/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs b/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs index 615e7a7916772..fa3ace456be77 100644 --- a/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs +++ b/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs @@ -2,7 +2,7 @@ use ruff_python_ast::{Expr, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder}; +use ruff_python_ast::name::QualifiedName; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -69,10 +69,7 @@ pub(crate) fn debugger_call(checker: &mut Checker, expr: &Expr, func: &Expr) { /// Checks for the presence of a debugger import. pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> Option { if let Some(module) = module { - let mut builder = - QualifiedNameBuilder::from_qualified_name(QualifiedName::imported(module)); - builder.push(name); - let qualified_name = builder.build(); + let qualified_name = QualifiedName::user_defined(module).append_member(name); if is_debugger_call(&qualified_name) { return Some(Diagnostic::new( @@ -83,7 +80,7 @@ pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> )); } } else { - let qualified_name = QualifiedName::imported(name); + let qualified_name = QualifiedName::user_defined(name); if is_debugger_import(&qualified_name) { return Some(Diagnostic::new( diff --git a/crates/ruff_linter/src/rules/flake8_import_conventions/rules/unconventional_import_alias.rs b/crates/ruff_linter/src/rules/flake8_import_conventions/rules/unconventional_import_alias.rs index 0cd207187cd62..7c1e5762a0340 100644 --- a/crates/ruff_linter/src/rules/flake8_import_conventions/rules/unconventional_import_alias.rs +++ b/crates/ruff_linter/src/rules/flake8_import_conventions/rules/unconventional_import_alias.rs @@ -65,7 +65,7 @@ pub(crate) fn unconventional_import_alias( return None; }; - let qualified_name = import.qualified_name(); + let qualified_name = import.qualified_name().to_string(); let Some(expected_alias) = conventions.get(qualified_name.as_str()) else { return None; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs index 5d3b0cd2f6844..531649c305542 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs @@ -61,7 +61,10 @@ pub(crate) fn unaliased_collections_abc_set_import( let BindingKind::FromImport(import) = &binding.kind else { return None; }; - if !matches!(import.call_path(), ["collections", "abc", "Set"]) { + if !matches!( + import.qualified_name().segments(), + ["collections", "abc", "Set"] + ) { return None; } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index b8558f89029b1..a30f8feaffe3a 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -189,7 +189,7 @@ pub(crate) fn runtime_import_in_type_checking_block( { let mut diagnostic = Diagnostic::new( RuntimeImportInTypeCheckingBlock { - qualified_name: import.qualified_name(), + qualified_name: import.qualified_name().to_string(), strategy: Strategy::MoveImport, }, range, @@ -218,7 +218,7 @@ pub(crate) fn runtime_import_in_type_checking_block( { let mut diagnostic = Diagnostic::new( RuntimeImportInTypeCheckingBlock { - qualified_name: import.qualified_name(), + qualified_name: import.qualified_name().to_string(), strategy: Strategy::QuoteUsages, }, range, @@ -245,7 +245,7 @@ pub(crate) fn runtime_import_in_type_checking_block( { let mut diagnostic = Diagnostic::new( RuntimeImportInTypeCheckingBlock { - qualified_name: import.qualified_name(), + qualified_name: import.qualified_name().to_string(), strategy: Strategy::MoveImport, }, range, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 8b12e0086a719..13c2ba673ae05 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -282,7 +282,7 @@ pub(crate) fn typing_only_runtime_import( let qualified_name = import.qualified_name(); if is_exempt( - qualified_name.as_str(), + &qualified_name.to_string(), &checker .settings .flake8_type_checking @@ -296,7 +296,7 @@ pub(crate) fn typing_only_runtime_import( // Categorize the import, using coarse-grained categorization. let import_type = match categorize( - qualified_name.as_str(), + &qualified_name.to_string(), None, &checker.settings.src, checker.package(), @@ -365,8 +365,10 @@ pub(crate) fn typing_only_runtime_import( .. } in imports { - let mut diagnostic = - Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range); + let mut diagnostic = Diagnostic::new( + diagnostic_for(import_type, import.qualified_name().to_string()), + range, + ); if let Some(range) = parent_range { diagnostic.set_parent(range.start()); } @@ -387,8 +389,10 @@ pub(crate) fn typing_only_runtime_import( .. } in imports { - let mut diagnostic = - Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range); + let mut diagnostic = Diagnostic::new( + diagnostic_for(import_type, import.qualified_name().to_string()), + range, + ); if let Some(range) = parent_range { diagnostic.set_parent(range.start()); } diff --git a/crates/ruff_linter/src/rules/pandas_vet/helpers.rs b/crates/ruff_linter/src/rules/pandas_vet/helpers.rs index 03a24ad2953c3..efcc6634a9c4a 100644 --- a/crates/ruff_linter/src/rules/pandas_vet/helpers.rs +++ b/crates/ruff_linter/src/rules/pandas_vet/helpers.rs @@ -50,7 +50,9 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti | BindingKind::ComprehensionVar | BindingKind::Global | BindingKind::Nonlocal(_) => Resolution::RelevantLocal, - BindingKind::Import(import) if matches!(import.call_path(), ["pandas"]) => { + BindingKind::Import(import) + if matches!(import.qualified_name().segments(), ["pandas"]) => + { Resolution::PandasModule } _ => Resolution::IrrelevantBinding, diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index dfbeee57786fd..1499ad779128e 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -178,7 +178,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut { let mut diagnostic = Diagnostic::new( UnusedImport { - name: import.qualified_name(), + name: import.qualified_name().to_string(), context: if in_except_handler { Some(UnusedImportContext::ExceptHandler) } else if in_init { @@ -212,7 +212,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut { let mut diagnostic = Diagnostic::new( UnusedImport { - name: import.qualified_name(), + name: import.qualified_name().to_string(), context: None, multiple: false, }, diff --git a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs index 29a94b924f0e5..1081530d0a9ff 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs @@ -4,6 +4,7 @@ use itertools::Itertools; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::name::QualifiedName; use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope}; use ruff_text_size::Ranged; @@ -113,7 +114,8 @@ pub(crate) fn import_private_name( // Ignore public imports; require at least one private name. // Ex) `from foo import bar` let Some((index, private_name)) = import_info - .call_path + .qualified_name + .segments() .iter() .find_position(|name| name.starts_with('_')) else { @@ -132,7 +134,7 @@ pub(crate) fn import_private_name( let name = (*private_name).to_string(); let module = if index > 0 { - Some(import_info.call_path[..index].join(".")) + Some(import_info.qualified_name.segments()[..index].join(".")) } else { None }; @@ -152,21 +154,22 @@ fn is_typing(reference: &ResolvedReference) -> bool { || reference.in_runtime_evaluated_annotation() } +#[allow(clippy::struct_field_names)] struct ImportInfo<'a> { module_name: &'a [&'a str], member_name: Cow<'a, str>, - call_path: &'a [&'a str], + qualified_name: &'a QualifiedName<'a>, } impl<'a> From<&'a FromImport<'_>> for ImportInfo<'a> { fn from(import: &'a FromImport) -> Self { let module_name = import.module_name(); let member_name = import.member_name(); - let call_path = import.call_path(); + let qualified_name = import.qualified_name(); Self { module_name, member_name, - call_path, + qualified_name, } } } @@ -175,11 +178,11 @@ impl<'a> From<&'a Import<'_>> for ImportInfo<'a> { fn from(import: &'a Import) -> Self { let module_name = import.module_name(); let member_name = import.member_name(); - let call_path = import.call_path(); + let qualified_name = import.qualified_name(); Self { module_name, member_name, - call_path, + qualified_name, } } } diff --git a/crates/ruff_linter/src/rules/ruff/typing.rs b/crates/ruff_linter/src/rules/ruff/typing.rs index 286f162b4c893..7668a18ebac06 100644 --- a/crates/ruff_linter/src/rules/ruff/typing.rs +++ b/crates/ruff_linter/src/rules/ruff/typing.rs @@ -298,25 +298,25 @@ mod tests { #[test] fn test_is_known_type() { - assert!(is_known_type(&QualifiedName::from_slice(&["", "int"]), 11)); + assert!(is_known_type(&QualifiedName::builtin("int"), 11)); assert!(is_known_type( - &QualifiedName::from_slice(&["builtins", "int"]), + &QualifiedName::from_iter(["builtins", "int"]), 11 )); assert!(is_known_type( - &QualifiedName::from_slice(&["typing", "Optional"]), + &QualifiedName::from_iter(["typing", "Optional"]), 11 )); assert!(is_known_type( - &QualifiedName::from_slice(&["typing_extensions", "Literal"]), + &QualifiedName::from_iter(["typing_extensions", "Literal"]), 11 )); assert!(is_known_type( - &QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]), + &QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]), 11 )); assert!(!is_known_type( - &QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]), + &QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]), 8 )); } diff --git a/crates/ruff_python_ast/Cargo.toml b/crates/ruff_python_ast/Cargo.toml index b0b9eec03847a..6918e6aed9550 100644 --- a/crates/ruff_python_ast/Cargo.toml +++ b/crates/ruff_python_ast/Cargo.toml @@ -24,7 +24,6 @@ itertools = { workspace = true } once_cell = { workspace = true } rustc-hash = { workspace = true } serde = { workspace = true, optional = true } -smallvec = { workspace = true } [dev-dependencies] insta = { workspace = true } diff --git a/crates/ruff_python_ast/src/name.rs b/crates/ruff_python_ast/src/name.rs index 0a1ff2837f0ac..225a8e749067a 100644 --- a/crates/ruff_python_ast/src/name.rs +++ b/crates/ruff_python_ast/src/name.rs @@ -1,19 +1,17 @@ -use smallvec::SmallVec; -use std::fmt::{Display, Formatter, Write}; +use std::fmt::{Debug, Display, Formatter, Write}; +use std::hash::{Hash, Hasher}; +use std::ops::Deref; use crate::{nodes, Expr}; /// A representation of a qualified name, like `typing.List`. -#[derive(Debug, Clone, Eq, Hash)] -pub struct QualifiedName<'a> { - segments: SmallVec<[&'a str; 8]>, -} +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct QualifiedName<'a>(SegmentsVec<'a>); impl<'a> QualifiedName<'a> { /// Create a [`QualifiedName`] from a dotted name. /// /// ```rust - /// # use smallvec::smallvec; /// # use ruff_python_ast::name::QualifiedName; /// /// assert_eq!(QualifiedName::from_dotted_name("typing.List").segments(), ["typing", "List"]); @@ -22,10 +20,10 @@ impl<'a> QualifiedName<'a> { #[inline] pub fn from_dotted_name(name: &'a str) -> Self { if let Some(dot) = name.find('.') { - let mut segments = SmallVec::new(); - segments.push(&name[..dot]); - segments.extend(name[dot + 1..].split('.')); - Self { segments } + let mut builder = QualifiedNameBuilder::default(); + builder.push(&name[..dot]); + builder.extend(name[dot + 1..].split('.')); + builder.build() } else { Self::builtin(name) } @@ -33,7 +31,7 @@ impl<'a> QualifiedName<'a> { /// Creates a name that's guaranteed not be a built in #[inline] - pub fn imported(name: &'a str) -> Self { + pub fn user_defined(name: &'a str) -> Self { name.split('.').collect() } @@ -41,63 +39,98 @@ impl<'a> QualifiedName<'a> { #[inline] pub fn builtin(name: &'a str) -> Self { debug_assert!(!name.contains('.')); - Self { - segments: ["", name].into_iter().collect(), - } + Self(SegmentsVec::Stack(SegmentsStack::from_slice(&["", name]))) } #[inline] - pub fn from_slice(segments: &[&'a str]) -> Self { - Self { - segments: segments.into(), - } + pub fn segments(&self) -> &[&'a str] { + self.0.as_slice() } - pub fn starts_with(&self, other: &QualifiedName) -> bool { - self.segments().starts_with(other.segments()) + /// If the first segment is empty, the `CallPath` is that of a builtin. + /// Ex) `["", "bool"]` -> `"bool"` + pub fn is_builtin(&self) -> bool { + matches!(self.segments(), ["", ..]) } - #[inline] - pub fn segments(&self) -> &[&'a str] { - &self.segments + pub fn is_user_defined(&self) -> bool { + !self.is_builtin() } - #[inline] - pub fn into_boxed_slice(self) -> Box<[&'a str]> { - self.segments.into_boxed_slice() + /// If the call path is dot-prefixed, it's an unresolved relative import. + /// Ex) `[".foo", "bar"]` -> `".foo.bar"` + pub fn is_unresolved_import(&self) -> bool { + matches!(self.segments(), [".", ..]) + } + + pub fn starts_with(&self, other: &QualifiedName<'_>) -> bool { + self.segments().starts_with(other.segments()) + } + + /// Appends a member to the qualified name. + #[must_use] + pub fn append_member(self, member: &'a str) -> Self { + let mut inner = self.0; + inner.push(member); + Self(inner) } } -impl<'a> FromIterator<&'a str> for QualifiedName<'a> { - fn from_iter>(iter: I) -> Self { - Self { - segments: iter.into_iter().collect(), +impl Display for QualifiedName<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let segments = self.segments(); + + if self.is_unresolved_import() { + let mut iter = segments.iter(); + for segment in iter.by_ref() { + if *segment == "." { + f.write_char('.')?; + } else { + f.write_str(segment)?; + break; + } + } + for segment in iter { + f.write_char('.')?; + f.write_str(segment)?; + } + } else { + let segments = if self.is_builtin() { + &segments[1..] + } else { + segments + }; + + let mut first = true; + for segment in segments { + if !first { + f.write_char('.')?; + } + + f.write_str(segment)?; + first = false; + } } + + Ok(()) } } -impl<'a, 'b> PartialEq> for QualifiedName<'a> { - #[inline] - fn eq(&self, other: &QualifiedName<'b>) -> bool { - self.segments == other.segments +impl<'a> FromIterator<&'a str> for QualifiedName<'a> { + fn from_iter>(iter: T) -> Self { + Self(SegmentsVec::from_iter(iter)) } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct QualifiedNameBuilder<'a> { - segments: SmallVec<[&'a str; 8]>, + segments: SegmentsVec<'a>, } impl<'a> QualifiedNameBuilder<'a> { pub fn with_capacity(capacity: usize) -> Self { Self { - segments: SmallVec::with_capacity(capacity), - } - } - - pub fn from_qualified_name(qualified_name: QualifiedName<'a>) -> Self { - Self { - segments: qualified_name.segments, + segments: SegmentsVec::with_capacity(capacity), } } @@ -111,6 +144,7 @@ impl<'a> QualifiedNameBuilder<'a> { self.segments.push(segment); } + #[inline] pub fn pop(&mut self) { self.segments.pop(); } @@ -120,96 +154,186 @@ impl<'a> QualifiedNameBuilder<'a> { self.segments.extend(segments); } + #[inline] pub fn extend_from_slice(&mut self, segments: &[&'a str]) { self.segments.extend_from_slice(segments); } pub fn build(self) -> QualifiedName<'a> { - QualifiedName { - segments: self.segments, - } + QualifiedName(self.segments) } } -impl Display for QualifiedName<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - format_qualified_name_segments(self.segments(), f) - } -} +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct UnqualifiedName<'a>(SegmentsVec<'a>); -pub fn format_qualified_name_segments(segments: &[&str], w: &mut dyn Write) -> std::fmt::Result { - if segments.first().is_some_and(|first| first.is_empty()) { - // If the first segment is empty, the `CallPath` is that of a builtin. - // Ex) `["", "bool"]` -> `"bool"` - let mut first = true; - - for segment in segments.iter().skip(1) { - if !first { - w.write_char('.')?; +impl<'a> UnqualifiedName<'a> { + /// Convert an `Expr` to its [`UnqualifiedName`] (like `["typing", "List"]`). + pub fn from_expr(expr: &'a Expr) -> Option { + // Unroll the loop up to eight times, to match the maximum number of expected attributes. + // In practice, unrolling appears to give about a 4x speed-up on this hot path. + let attr1 = match expr { + Expr::Attribute(attr1) => attr1, + // Ex) `foo` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[id.as_str()])) } - - w.write_str(segment)?; - first = false; - } - } else if segments.first().is_some_and(|first| matches!(*first, ".")) { - // If the call path is dot-prefixed, it's an unresolved relative import. - // Ex) `[".foo", "bar"]` -> `".foo.bar"` - - let mut iter = segments.iter(); - for segment in iter.by_ref() { - if *segment == "." { - w.write_char('.')?; - } else { - w.write_str(segment)?; - break; + _ => return None, + }; + + let attr2 = match attr1.value.as_ref() { + Expr::Attribute(attr2) => attr2, + // Ex) `foo.bar` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[id.as_str(), attr1.attr.as_str()])) } - } - for segment in iter { - w.write_char('.')?; - w.write_str(segment)?; - } - } else { - let mut first = true; - for segment in segments { - if !first { - w.write_char('.')?; + _ => return None, + }; + + let attr3 = match attr2.value.as_ref() { + Expr::Attribute(attr3) => attr3, + // Ex) `foo.bar.baz` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[ + id.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ])); + } + _ => return None, + }; + + let attr4 = match attr3.value.as_ref() { + Expr::Attribute(attr4) => attr4, + // Ex) `foo.bar.baz.bop` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[ + id.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ])); + } + _ => return None, + }; + + let attr5 = match attr4.value.as_ref() { + Expr::Attribute(attr5) => attr5, + // Ex) `foo.bar.baz.bop.bap` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[ + id.as_str(), + attr4.attr.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ])); + } + _ => return None, + }; + + let attr6 = match attr5.value.as_ref() { + Expr::Attribute(attr6) => attr6, + // Ex) `foo.bar.baz.bop.bap.bab` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[ + id.as_str(), + attr5.attr.as_str(), + attr4.attr.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ])); + } + _ => return None, + }; + + let attr7 = match attr6.value.as_ref() { + Expr::Attribute(attr7) => attr7, + // Ex) `foo.bar.baz.bop.bap.bab.bob` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self::from_slice(&[ + id.as_str(), + attr6.attr.as_str(), + attr5.attr.as_str(), + attr4.attr.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ])); + } + _ => return None, + }; + + let attr8 = match attr7.value.as_ref() { + Expr::Attribute(attr8) => attr8, + // Ex) `foo.bar.baz.bop.bap.bab.bob.bib` + Expr::Name(nodes::ExprName { id, .. }) => { + return Some(Self(SegmentsVec::from([ + id.as_str(), + attr7.attr.as_str(), + attr6.attr.as_str(), + attr5.attr.as_str(), + attr4.attr.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ]))); + } + _ => return None, + }; + + let mut segments = Vec::with_capacity(SMALL_LEN * 2); + + let mut current = &*attr8.value; + + loop { + current = match current { + Expr::Attribute(attr) => { + segments.push(attr.attr.as_str()); + &*attr.value + } + Expr::Name(nodes::ExprName { id, .. }) => { + segments.push(id.as_str()); + break; + } + _ => { + return None; + } } - - w.write_str(segment)?; - first = false; } - } - Ok(()) -} + segments.reverse(); -#[derive(Debug, Clone, Eq, Hash)] -pub struct UnqualifiedName<'a> { - segments: SmallVec<[&'a str; 8]>, -} + // Append the attributes we visited before calling into the recursion. + segments.extend_from_slice(&[ + attr8.attr.as_str(), + attr7.attr.as_str(), + attr6.attr.as_str(), + attr5.attr.as_str(), + attr4.attr.as_str(), + attr3.attr.as_str(), + attr2.attr.as_str(), + attr1.attr.as_str(), + ]); -impl<'a> UnqualifiedName<'a> { - pub fn from_expr(expr: &'a Expr) -> Option { - let segments = collect_segments(expr)?; - Some(Self { segments }) + Some(Self(SegmentsVec::from(segments))) } - pub fn segments(&self) -> &[&'a str] { - &self.segments + #[inline] + pub fn from_slice(segments: &[&'a str]) -> Self { + Self(SegmentsVec::from_slice(segments)) } -} -impl<'a, 'b> PartialEq> for UnqualifiedName<'a> { - #[inline] - fn eq(&self, other: &UnqualifiedName<'b>) -> bool { - self.segments == other.segments + pub fn segments(&self) -> &[&'a str] { + self.0.as_slice() } } impl Display for UnqualifiedName<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let mut first = true; - for segment in &self.segments { + for segment in self.segments() { if !first { f.write_char('.')?; } @@ -225,138 +349,457 @@ impl Display for UnqualifiedName<'_> { impl<'a> FromIterator<&'a str> for UnqualifiedName<'a> { #[inline] fn from_iter>(iter: T) -> Self { - Self { - segments: iter.into_iter().collect(), + Self(iter.into_iter().collect()) + } +} + +/// A smallvec like storage for qualified and unqualified name segments. +/// +/// Stores up to 8 segments inline, and falls back to a heap-allocated vector for names with more segments. +/// +/// ## Note +/// The implementation doesn't use `SmallVec` v1 because its type definition has a variance problem. +/// The incorrect variance leads the lifetime inference in the `SemanticModel` astray, causing +/// all sort of "strange" lifetime errors. We can switch back to `SmallVec` when v2 is released. +#[derive(Clone)] +enum SegmentsVec<'a> { + Stack(SegmentsStack<'a>), + Heap(Vec<&'a str>), +} + +impl<'a> SegmentsVec<'a> { + /// Creates an empty segment vec. + fn new() -> Self { + Self::Stack(SegmentsStack::default()) + } + + /// Creates a segment vec that has reserved storage for up to `capacity` items. + fn with_capacity(capacity: usize) -> Self { + if capacity <= SMALL_LEN { + Self::new() + } else { + Self::Heap(Vec::with_capacity(capacity)) } } + + #[cfg(test)] + const fn is_spilled(&self) -> bool { + matches!(self, SegmentsVec::Heap(_)) + } + + /// Initializes the segments from a slice. + #[inline] + fn from_slice(slice: &[&'a str]) -> Self { + if slice.len() <= SMALL_LEN { + SegmentsVec::Stack(SegmentsStack::from_slice(slice)) + } else { + SegmentsVec::Heap(slice.to_vec()) + } + } + + /// Returns the segments as a slice. + #[inline] + fn as_slice(&self) -> &[&'a str] { + match self { + Self::Stack(stack) => stack.as_slice(), + Self::Heap(heap) => heap.as_slice(), + } + } + + /// Pushes `name` to the end of the segments. + /// + /// Spills to the heap if the segments are stored on the stack and the 9th segment is pushed. + #[inline] + fn push(&mut self, name: &'a str) { + match self { + SegmentsVec::Stack(stack) => { + if let Err(segments) = stack.push(name) { + *self = SegmentsVec::Heap(segments); + } + } + SegmentsVec::Heap(heap) => { + heap.push(name); + } + } + } + + /// Pops the last segment from the end and returns it. + /// + /// Returns `None` if the vector is empty. + #[inline] + fn pop(&mut self) -> Option<&'a str> { + match self { + SegmentsVec::Stack(stack) => stack.pop(), + SegmentsVec::Heap(heap) => heap.pop(), + } + } + + #[inline] + fn extend_from_slice(&mut self, slice: &[&'a str]) { + match self { + SegmentsVec::Stack(stack) => { + if let Err(segments) = stack.extend_from_slice(slice) { + *self = SegmentsVec::Heap(segments); + } + } + SegmentsVec::Heap(heap) => heap.extend_from_slice(slice), + } + } +} + +impl Default for SegmentsVec<'_> { + fn default() -> Self { + Self::new() + } } -/// Convert an `Expr` to its [`QualifiedName`] segments (like `["typing", "List"]`). -fn collect_segments(expr: &Expr) -> Option> { - // Unroll the loop up to eight times, to match the maximum number of expected attributes. - // In practice, unrolling appears to give about a 4x speed-up on this hot path. - let attr1 = match expr { - Expr::Attribute(attr1) => attr1, - // Ex) `foo` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[id.as_str()])) +impl Debug for SegmentsVec<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_list().entries(self.as_slice()).finish() + } +} + +impl<'a> Deref for SegmentsVec<'a> { + type Target = [&'a str]; + fn deref(&self) -> &Self::Target { + self.as_slice() + } +} + +impl<'a, 'b> PartialEq> for SegmentsVec<'a> { + fn eq(&self, other: &SegmentsVec<'b>) -> bool { + self.as_slice() == other.as_slice() + } +} + +impl Eq for SegmentsVec<'_> {} + +impl Hash for SegmentsVec<'_> { + fn hash(&self, state: &mut H) { + self.as_slice().hash(state); + } +} + +impl<'a> FromIterator<&'a str> for SegmentsVec<'a> { + #[inline] + fn from_iter>(iter: T) -> Self { + let mut segments = SegmentsVec::default(); + segments.extend(iter); + segments + } +} + +impl<'a> From<[&'a str; 8]> for SegmentsVec<'a> { + #[inline] + fn from(segments: [&'a str; 8]) -> Self { + SegmentsVec::Stack(SegmentsStack { + segments, + len: segments.len(), + }) + } +} + +impl<'a> From> for SegmentsVec<'a> { + #[inline] + fn from(segments: Vec<&'a str>) -> Self { + SegmentsVec::Heap(segments) + } +} + +impl<'a> Extend<&'a str> for SegmentsVec<'a> { + #[inline] + fn extend>(&mut self, iter: T) { + match self { + SegmentsVec::Stack(stack) => { + if let Err(segments) = stack.extend(iter) { + *self = SegmentsVec::Heap(segments); + } + } + SegmentsVec::Heap(heap) => { + heap.extend(iter); + } } - _ => return None, - }; - - let attr2 = match attr1.value.as_ref() { - Expr::Attribute(attr2) => attr2, - // Ex) `foo.bar` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[id.as_str(), attr1.attr.as_str()])) + } +} + +const SMALL_LEN: usize = 8; + +#[derive(Debug, Clone, Default)] +struct SegmentsStack<'a> { + segments: [&'a str; SMALL_LEN], + len: usize, +} + +impl<'a> SegmentsStack<'a> { + #[inline] + fn from_slice(slice: &[&'a str]) -> Self { + assert!(slice.len() <= SMALL_LEN); + + let mut segments: [&'a str; SMALL_LEN] = Default::default(); + segments[..slice.len()].copy_from_slice(slice); + SegmentsStack { + segments, + len: slice.len(), } - _ => return None, - }; - - let attr3 = match attr2.value.as_ref() { - Expr::Attribute(attr3) => attr3, - // Ex) `foo.bar.baz` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[ - id.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + } + + const fn capacity(&self) -> usize { + SMALL_LEN - self.len + } + + #[inline] + fn as_slice(&self) -> &[&'a str] { + &self.segments[..self.len] + } + + /// Pushes `name` to the end of the segments. + /// + /// Returns `Err` with a `Vec` containing all items (including `name`) if there's not enough capacity to push the name. + #[inline] + fn push(&mut self, name: &'a str) -> Result<(), Vec<&'a str>> { + if self.len < self.segments.len() { + self.segments[self.len] = name; + self.len += 1; + Ok(()) + } else { + let mut segments = Vec::with_capacity(self.len * 2); + segments.extend_from_slice(&self.segments); + segments.push(name); + Err(segments) } - _ => return None, - }; - - let attr4 = match attr3.value.as_ref() { - Expr::Attribute(attr4) => attr4, - // Ex) `foo.bar.baz.bop` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[ - id.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + } + + /// Reserves spaces for `additional` segments. + /// + /// Returns `Err` with a `Vec` containing all segments and a capacity to store `additional` segments if + /// the stack needs to spill over to the heap to store `additional` segments. + #[inline] + fn reserve(&mut self, additional: usize) -> Result<(), Vec<&'a str>> { + if self.capacity() >= additional { + Ok(()) + } else { + let mut segments = Vec::with_capacity(self.len + additional); + segments.extend_from_slice(self.as_slice()); + Err(segments) } - _ => return None, - }; - - let attr5 = match attr4.value.as_ref() { - Expr::Attribute(attr5) => attr5, - // Ex) `foo.bar.baz.bop.bap` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[ - id.as_str(), - attr4.attr.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + } + + #[inline] + fn pop(&mut self) -> Option<&'a str> { + if self.len > 0 { + self.len -= 1; + Some(self.segments[self.len]) + } else { + None } - _ => return None, - }; - - let attr6 = match attr5.value.as_ref() { - Expr::Attribute(attr6) => attr6, - // Ex) `foo.bar.baz.bop.bap.bab` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[ - id.as_str(), - attr5.attr.as_str(), - attr4.attr.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + } + + /// Extends the segments by appending `slice` to the end. + /// + /// Returns `Err` with a `Vec` containing all segments and the segments in `slice` if there's not enough capacity to append the names. + #[inline] + fn extend_from_slice(&mut self, slice: &[&'a str]) -> Result<(), Vec<&'a str>> { + let new_len = self.len + slice.len(); + if slice.len() <= self.capacity() { + self.segments[self.len..new_len].copy_from_slice(slice); + self.len = new_len; + Ok(()) + } else { + let mut segments = Vec::with_capacity(new_len); + segments.extend_from_slice(self.as_slice()); + segments.extend_from_slice(slice); + Err(segments) } - _ => return None, - }; - - let attr7 = match attr6.value.as_ref() { - Expr::Attribute(attr7) => attr7, - // Ex) `foo.bar.baz.bop.bap.bab.bob` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from_slice(&[ - id.as_str(), - attr6.attr.as_str(), - attr5.attr.as_str(), - attr4.attr.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + } + + #[inline] + fn extend(&mut self, iter: I) -> Result<(), Vec<&'a str>> + where + I: IntoIterator, + { + let mut iter = iter.into_iter(); + let (lower, _) = iter.size_hint(); + + // There's not enough space to store the lower bound of items. Spill to the heap. + if let Err(mut segments) = self.reserve(lower) { + segments.extend(iter); + return Err(segments); } - _ => return None, - }; - - let attr8 = match attr7.value.as_ref() { - Expr::Attribute(attr8) => attr8, - // Ex) `foo.bar.baz.bop.bap.bab.bob.bib` - Expr::Name(nodes::ExprName { id, .. }) => { - return Some(SmallVec::from([ - id.as_str(), - attr7.attr.as_str(), - attr6.attr.as_str(), - attr5.attr.as_str(), - attr4.attr.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ])); + + // Copy over up to capacity items + for name in iter.by_ref().take(self.capacity()) { + self.segments[self.len] = name; + self.len += 1; } - _ => return None, - }; - collect_segments(&attr8.value).map(|mut segments| { - segments.extend([ - attr8.attr.as_str(), - attr7.attr.as_str(), - attr6.attr.as_str(), - attr5.attr.as_str(), - attr4.attr.as_str(), - attr3.attr.as_str(), - attr2.attr.as_str(), - attr1.attr.as_str(), - ]); - segments - }) + let Some(item) = iter.next() else { + // There are no more items to copy over and they all fit into capacity. + return Ok(()); + }; + + // There are more items and there's not enough capacity to store them on the stack. + // Spill over to the heap and append the remaining items. + let mut segments = Vec::with_capacity(self.len * 2); + segments.extend_from_slice(self.as_slice()); + segments.push(item); + segments.extend(iter); + Err(segments) + } +} + +#[cfg(test)] +mod tests { + use crate::name::SegmentsVec; + + #[test] + fn empty_vec() { + let empty = SegmentsVec::new(); + assert_eq!(empty.as_slice(), &[] as &[&str]); + assert!(!empty.is_spilled()); + } + + #[test] + fn from_slice_stack() { + let stack = SegmentsVec::from_slice(&["a", "b", "c"]); + + assert_eq!(stack.as_slice(), &["a", "b", "c"]); + assert!(!stack.is_spilled()); + } + + #[test] + fn from_slice_stack_capacity() { + let stack = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g", "h"]); + + assert_eq!(stack.as_slice(), &["a", "b", "c", "d", "e", "f", "g", "h"]); + assert!(!stack.is_spilled()); + } + + #[test] + fn from_slice_heap() { + let heap = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g", "h", "i"]); + + assert_eq!( + heap.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i"] + ); + assert!(heap.is_spilled()); + } + + #[test] + fn push_stack() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c"]); + stack.push("d"); + stack.push("e"); + + assert_eq!(stack.as_slice(), &["a", "b", "c", "d", "e"]); + assert!(!stack.is_spilled()); + } + + #[test] + fn push_stack_spill() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g"]); + stack.push("h"); + + assert!(!stack.is_spilled()); + + stack.push("i"); + + assert_eq!( + stack.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i"] + ); + assert!(stack.is_spilled()); + } + + #[test] + fn pop_stack() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c", "d", "e"]); + assert_eq!(stack.pop(), Some("e")); + assert_eq!(stack.pop(), Some("d")); + assert_eq!(stack.pop(), Some("c")); + assert_eq!(stack.pop(), Some("b")); + assert_eq!(stack.pop(), Some("a")); + assert_eq!(stack.pop(), None); + + assert!(!stack.is_spilled()); + } + + #[test] + fn pop_heap() { + let mut heap = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g", "h", "i"]); + + assert_eq!(heap.pop(), Some("i")); + assert_eq!(heap.pop(), Some("h")); + assert_eq!(heap.pop(), Some("g")); + + assert!(heap.is_spilled()); + } + + #[test] + fn extend_from_slice_stack() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c"]); + stack.extend_from_slice(&["d", "e", "f"]); + + assert_eq!(stack.as_slice(), &["a", "b", "c", "d", "e", "f"]); + assert!(!stack.is_spilled()); + } + + #[test] + fn extend_from_slice_stack_spill() { + let mut spilled = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f"]); + spilled.extend_from_slice(&["g", "h", "i", "j"]); + + assert_eq!( + spilled.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"] + ); + assert!(spilled.is_spilled()); + } + + #[test] + fn extend_from_slice_heap() { + let mut heap = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g", "h", "i"]); + assert!(heap.is_spilled()); + + heap.extend_from_slice(&["j", "k", "l"]); + + assert_eq!( + heap.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"] + ); + } + + #[test] + fn extend_stack() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c"]); + stack.extend(["d", "e", "f"]); + + assert_eq!(stack.as_slice(), &["a", "b", "c", "d", "e", "f"]); + assert!(!stack.is_spilled()); + } + + #[test] + fn extend_stack_spilled() { + let mut stack = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f"]); + stack.extend(["g", "h", "i", "j"]); + + assert_eq!( + stack.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"] + ); + assert!(stack.is_spilled()); + } + + #[test] + fn extend_heap() { + let mut heap = SegmentsVec::from_slice(&["a", "b", "c", "d", "e", "f", "g", "h", "i"]); + assert!(heap.is_spilled()); + + heap.extend(["j", "k", "l"]); + + assert_eq!( + heap.as_slice(), + &["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"] + ); + } } diff --git a/crates/ruff_python_semantic/Cargo.toml b/crates/ruff_python_semantic/Cargo.toml index 5e6080c5fc898..724e6933564b5 100644 --- a/crates/ruff_python_semantic/Cargo.toml +++ b/crates/ruff_python_semantic/Cargo.toml @@ -23,7 +23,6 @@ ruff_text_size = { path = "../ruff_text_size" } bitflags = { workspace = true } is-macro = { workspace = true } rustc-hash = { workspace = true } -smallvec = { workspace = true } [dev-dependencies] ruff_python_parser = { path = "../ruff_python_parser" } diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index db7c7ab146084..ea8d9b3659672 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -63,7 +63,7 @@ pub fn match_annotated_subscript<'a>( } for module in typing_modules { - let module_qualified_name = QualifiedName::imported(module); + let module_qualified_name = QualifiedName::user_defined(module); if qualified_name.starts_with(&module_qualified_name) { if let Some(member) = qualified_name.segments().last() { if is_literal_member(member) { diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 10b4d13439246..cff3c7ce75d1c 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -4,7 +4,7 @@ use std::ops::{Deref, DerefMut}; use bitflags::bitflags; use ruff_index::{newtype_index, IndexSlice, IndexVec}; -use ruff_python_ast::name::format_qualified_name_segments; +use ruff_python_ast::name::QualifiedName; use ruff_python_ast::Stmt; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; @@ -362,7 +362,7 @@ pub struct Import<'a> { /// The full name of the module being imported. /// Ex) Given `import foo`, `qualified_name` would be "foo". /// Ex) Given `import foo as bar`, `qualified_name` would be "foo". - pub qualified_name: Box<[&'a str]>, + pub qualified_name: Box>, } /// A binding for a member imported from a module, keyed on the name to which the member is bound. @@ -373,7 +373,7 @@ 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: Box<[&'a str]>, + pub qualified_name: Box>, } /// A binding for a submodule imported from a module, keyed on the name of the parent module. @@ -382,7 +382,7 @@ pub struct FromImport<'a> { 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: Box<[&'a str]>, + pub qualified_name: Box>, } #[derive(Debug, Clone, is_macro::Is)] @@ -549,7 +549,7 @@ bitflags! { /// A trait for imported symbols. pub trait Imported<'a> { /// Returns the call path to the imported symbol. - fn call_path(&self) -> &[&'a str]; + fn qualified_name(&self) -> &QualifiedName<'a>; /// Returns the module name of the imported symbol. fn module_name(&self) -> &[&'a str]; @@ -557,63 +557,56 @@ pub trait Imported<'a> { /// Returns the member name of the imported symbol. For a straight import, this is equivalent /// to the qualified name; for a `from` import, this is the name of the imported symbol. fn member_name(&self) -> Cow<'a, str>; - - /// Returns the fully-qualified name of the imported symbol. - fn qualified_name(&self) -> String { - let mut output = String::new(); - format_qualified_name_segments(self.call_path(), &mut output).unwrap(); - output - } } impl<'a> Imported<'a> for Import<'a> { /// For example, given `import foo`, returns `["foo"]`. - fn call_path(&self) -> &[&'a str] { - self.qualified_name.as_ref() + fn qualified_name(&self) -> &QualifiedName<'a> { + &self.qualified_name } /// For example, given `import foo`, returns `["foo"]`. fn module_name(&self) -> &[&'a str] { - &self.qualified_name[..1] + &self.qualified_name.segments()[..1] } /// For example, given `import foo`, returns `"foo"`. fn member_name(&self) -> Cow<'a, str> { - Cow::Owned(self.qualified_name()) + Cow::Owned(self.qualified_name().to_string()) } } impl<'a> Imported<'a> for SubmoduleImport<'a> { /// For example, given `import foo.bar`, returns `["foo", "bar"]`. - fn call_path(&self) -> &[&'a str] { - self.qualified_name.as_ref() + fn qualified_name(&self) -> &QualifiedName<'a> { + &self.qualified_name } /// For example, given `import foo.bar`, returns `["foo"]`. fn module_name(&self) -> &[&'a str] { - &self.qualified_name[..1] + &self.qualified_name.segments()[..1] } /// For example, given `import foo.bar`, returns `"foo.bar"`. fn member_name(&self) -> Cow<'a, str> { - Cow::Owned(self.qualified_name()) + Cow::Owned(self.qualified_name().to_string()) } } impl<'a> Imported<'a> for FromImport<'a> { /// For example, given `from foo import bar`, returns `["foo", "bar"]`. - fn call_path(&self) -> &[&'a str] { + fn qualified_name(&self) -> &QualifiedName<'a> { &self.qualified_name } /// For example, given `from foo import bar`, returns `["foo"]`. fn module_name(&self) -> &[&'a str] { - &self.qualified_name[..self.qualified_name.len() - 1] + &self.qualified_name.segments()[..self.qualified_name.segments().len() - 1] } /// For example, given `from foo import bar`, returns `"bar"`. fn member_name(&self) -> Cow<'a, str> { - Cow::Borrowed(self.qualified_name[self.qualified_name.len() - 1]) + Cow::Borrowed(self.qualified_name.segments()[self.qualified_name.segments().len() - 1]) } } @@ -626,11 +619,11 @@ pub enum AnyImport<'a, 'ast> { } impl<'a, 'ast> Imported<'ast> for AnyImport<'a, 'ast> { - fn call_path(&self) -> &[&'ast str] { + fn qualified_name(&self) -> &QualifiedName<'ast> { match self { - Self::Import(import) => import.call_path(), - Self::SubmoduleImport(import) => import.call_path(), - Self::FromImport(import) => import.call_path(), + Self::Import(import) => import.qualified_name(), + Self::SubmoduleImport(import) => import.qualified_name(), + Self::FromImport(import) => import.qualified_name(), } } diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 3556fb28a9b76..f6575656fdf57 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -4,7 +4,7 @@ use bitflags::bitflags; use rustc_hash::FxHashMap; use ruff_python_ast::helpers::from_relative_import; -use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder, UnqualifiedName}; +use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; use ruff_python_ast::{self as ast, Expr, Operator, Stmt}; use ruff_python_stdlib::path::is_python_stub_file; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -194,10 +194,7 @@ impl<'a> SemanticModel<'a> { if self.typing_modules.iter().any(|module| { let module = QualifiedName::from_dotted_name(module); - let mut builder = QualifiedNameBuilder::from_qualified_name(module); - builder.push(target); - let target_path = builder.build(); - qualified_name == &target_path + qualified_name == &module.append_member(target) }) { return true; } @@ -617,8 +614,8 @@ impl<'a> SemanticModel<'a> { } // Grab, e.g., `pyarrow` from `import pyarrow as pa`. - let call_path = import.call_path(); - let segment = call_path.last()?; + let call_path = import.qualified_name(); + let segment = call_path.segments().last()?; if *segment == symbol { return None; } @@ -692,8 +689,12 @@ impl<'a> SemanticModel<'a> { BindingKind::Import(Import { qualified_name }) => { let unqualified_name = UnqualifiedName::from_expr(value)?; let (_, tail) = unqualified_name.segments().split_first()?; - let resolved: QualifiedName = - qualified_name.iter().chain(tail.iter()).copied().collect(); + let resolved: QualifiedName = qualified_name + .segments() + .iter() + .chain(tail.iter()) + .copied() + .collect(); Some(resolved) } BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => { @@ -702,6 +703,7 @@ impl<'a> SemanticModel<'a> { Some( qualified_name + .segments() .iter() .take(1) .chain(tail.iter()) @@ -714,19 +716,25 @@ impl<'a> SemanticModel<'a> { let (_, tail) = value_name.segments().split_first()?; let resolved: QualifiedName = if qualified_name + .segments() .first() .map_or(false, |segment| *segment == ".") { - from_relative_import(self.module_path?, qualified_name, tail)? + from_relative_import(self.module_path?, qualified_name.segments(), tail)? } else { - qualified_name.iter().chain(tail.iter()).copied().collect() + qualified_name + .segments() + .iter() + .chain(tail.iter()) + .copied() + .collect() }; Some(resolved) } BindingKind::Builtin => { if value.is_name_expr() { // Ex) `dict` - Some(QualifiedName::from_slice(&["", head.id.as_str()])) + Some(QualifiedName::builtin(head.id.as_str())) } else { // Ex) `dict.__dict__` let value_name = UnqualifiedName::from_expr(value)?; @@ -780,7 +788,7 @@ impl<'a> SemanticModel<'a> { // `import sys` -> `sys.exit` // `import sys as sys2` -> `sys2.exit` BindingKind::Import(Import { qualified_name }) => { - if qualified_name.as_ref() == module_path.as_slice() { + if qualified_name.segments() == module_path.as_slice() { if let Some(source) = binding.source { // Verify that `sys` isn't bound in an inner scope. if self @@ -803,7 +811,7 @@ impl<'a> SemanticModel<'a> { // `from os.path import join as join2` -> `join2` BindingKind::FromImport(FromImport { qualified_name }) => { if let Some((target_member, target_module)) = - qualified_name.split_last() + qualified_name.segments().split_last() { if target_module == module_path.as_slice() && target_member == &member @@ -831,7 +839,7 @@ impl<'a> SemanticModel<'a> { // Ex) Given `module="os.path"` and `object="join"`: // `import os.path ` -> `os.path.join` BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }) => { - if qualified_name.starts_with(&module_path) { + if qualified_name.segments().starts_with(&module_path) { if let Some(source) = binding.source { // Verify that `os` isn't bound in an inner scope. if self