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