Skip to content

Commit 8ea5b08

Browse files
MichaReiserMichaReiser
authored
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<QualifiedName>` 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 servo/rust-smallvec#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`
1 parent 4c05c25 commit 8ea5b08

File tree

18 files changed

+772
-317
lines changed

18 files changed

+772
-317
lines changed

‎Cargo.lock

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use ruff_python_ast::helpers::{
4343
collect_import_from_member, extract_handled_exceptions, is_docstring_stmt, to_module_path,
4444
};
4545
use ruff_python_ast::identifier::Identifier;
46+
use ruff_python_ast::name::QualifiedName;
4647
use ruff_python_ast::str::trailing_quote;
4748
use ruff_python_ast::visitor::{walk_except_handler, walk_f_string_element, walk_pattern, Visitor};
4849
use ruff_python_ast::{helpers, str, visitor, PySourceType};
@@ -418,11 +419,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
418419
self.semantic.add_module(module);
419420

420421
if alias.asname.is_none() && alias.name.contains('.') {
421-
let qualified_name: Box<[&str]> = alias.name.split('.').collect();
422+
let qualified_name = QualifiedName::user_defined(&alias.name);
422423
self.add_binding(
423424
module,
424425
alias.identifier(),
425-
BindingKind::SubmoduleImport(SubmoduleImport { qualified_name }),
426+
BindingKind::SubmoduleImport(SubmoduleImport {
427+
qualified_name: Box::new(qualified_name),
428+
}),
426429
BindingFlags::EXTERNAL,
427430
);
428431
} else {
@@ -439,11 +442,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
439442
}
440443

441444
let name = alias.asname.as_ref().unwrap_or(&alias.name);
442-
let qualified_name: Box<[&str]> = alias.name.split('.').collect();
445+
let qualified_name = QualifiedName::user_defined(&alias.name);
443446
self.add_binding(
444447
name,
445448
alias.identifier(),
446-
BindingKind::Import(Import { qualified_name }),
449+
BindingKind::Import(Import {
450+
qualified_name: Box::new(qualified_name),
451+
}),
447452
flags,
448453
);
449454
}
@@ -503,12 +508,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
503508
// Attempt to resolve any relative imports; but if we don't know the current
504509
// module path, or the relative import extends beyond the package root,
505510
// fallback to a literal representation (e.g., `[".", "foo"]`).
506-
let qualified_name = collect_import_from_member(level, module, &alias.name)
507-
.into_boxed_slice();
511+
let qualified_name = collect_import_from_member(level, module, &alias.name);
508512
self.add_binding(
509513
name,
510514
alias.identifier(),
511-
BindingKind::FromImport(FromImport { qualified_name }),
515+
BindingKind::FromImport(FromImport {
516+
qualified_name: Box::new(qualified_name),
517+
}),
512518
flags,
513519
);
514520
}

‎crates/ruff_linter/src/renamer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ impl Renamer {
232232
}
233233
BindingKind::SubmoduleImport(import) => {
234234
// Ex) Rename `import pandas.core` to `import pandas as pd`.
235-
let module_name = import.qualified_name.first().unwrap();
235+
let module_name = import.qualified_name.segments().first().unwrap();
236236
Some(Edit::range_replacement(
237237
format!("{module_name} as {target}"),
238238
binding.range(),

‎crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use ruff_python_ast::{Expr, Stmt};
22

33
use ruff_diagnostics::{Diagnostic, Violation};
44
use ruff_macros::{derive_message_formats, violation};
5-
use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder};
5+
use ruff_python_ast::name::QualifiedName;
66
use ruff_text_size::Ranged;
77

88
use crate::checkers::ast::Checker;
@@ -69,10 +69,7 @@ pub(crate) fn debugger_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
6969
/// Checks for the presence of a debugger import.
7070
pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> Option<Diagnostic> {
7171
if let Some(module) = module {
72-
let mut builder =
73-
QualifiedNameBuilder::from_qualified_name(QualifiedName::imported(module));
74-
builder.push(name);
75-
let qualified_name = builder.build();
72+
let qualified_name = QualifiedName::user_defined(module).append_member(name);
7673

7774
if is_debugger_call(&qualified_name) {
7875
return Some(Diagnostic::new(
@@ -83,7 +80,7 @@ pub(crate) fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) ->
8380
));
8481
}
8582
} else {
86-
let qualified_name = QualifiedName::imported(name);
83+
let qualified_name = QualifiedName::user_defined(name);
8784

8885
if is_debugger_import(&qualified_name) {
8986
return Some(Diagnostic::new(

‎crates/ruff_linter/src/rules/flake8_import_conventions/rules/unconventional_import_alias.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub(crate) fn unconventional_import_alias(
6565
return None;
6666
};
6767

68-
let qualified_name = import.qualified_name();
68+
let qualified_name = import.qualified_name().to_string();
6969

7070
let Some(expected_alias) = conventions.get(qualified_name.as_str()) else {
7171
return None;

‎crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ pub(crate) fn unaliased_collections_abc_set_import(
6161
let BindingKind::FromImport(import) = &binding.kind else {
6262
return None;
6363
};
64-
if !matches!(import.call_path(), ["collections", "abc", "Set"]) {
64+
if !matches!(
65+
import.qualified_name().segments(),
66+
["collections", "abc", "Set"]
67+
) {
6568
return None;
6669
}
6770

‎crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
189189
{
190190
let mut diagnostic = Diagnostic::new(
191191
RuntimeImportInTypeCheckingBlock {
192-
qualified_name: import.qualified_name(),
192+
qualified_name: import.qualified_name().to_string(),
193193
strategy: Strategy::MoveImport,
194194
},
195195
range,
@@ -218,7 +218,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
218218
{
219219
let mut diagnostic = Diagnostic::new(
220220
RuntimeImportInTypeCheckingBlock {
221-
qualified_name: import.qualified_name(),
221+
qualified_name: import.qualified_name().to_string(),
222222
strategy: Strategy::QuoteUsages,
223223
},
224224
range,
@@ -245,7 +245,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
245245
{
246246
let mut diagnostic = Diagnostic::new(
247247
RuntimeImportInTypeCheckingBlock {
248-
qualified_name: import.qualified_name(),
248+
qualified_name: import.qualified_name().to_string(),
249249
strategy: Strategy::MoveImport,
250250
},
251251
range,

‎crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ pub(crate) fn typing_only_runtime_import(
282282
let qualified_name = import.qualified_name();
283283

284284
if is_exempt(
285-
qualified_name.as_str(),
285+
&qualified_name.to_string(),
286286
&checker
287287
.settings
288288
.flake8_type_checking
@@ -296,7 +296,7 @@ pub(crate) fn typing_only_runtime_import(
296296

297297
// Categorize the import, using coarse-grained categorization.
298298
let import_type = match categorize(
299-
qualified_name.as_str(),
299+
&qualified_name.to_string(),
300300
None,
301301
&checker.settings.src,
302302
checker.package(),
@@ -365,8 +365,10 @@ pub(crate) fn typing_only_runtime_import(
365365
..
366366
} in imports
367367
{
368-
let mut diagnostic =
369-
Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
368+
let mut diagnostic = Diagnostic::new(
369+
diagnostic_for(import_type, import.qualified_name().to_string()),
370+
range,
371+
);
370372
if let Some(range) = parent_range {
371373
diagnostic.set_parent(range.start());
372374
}
@@ -387,8 +389,10 @@ pub(crate) fn typing_only_runtime_import(
387389
..
388390
} in imports
389391
{
390-
let mut diagnostic =
391-
Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
392+
let mut diagnostic = Diagnostic::new(
393+
diagnostic_for(import_type, import.qualified_name().to_string()),
394+
range,
395+
);
392396
if let Some(range) = parent_range {
393397
diagnostic.set_parent(range.start());
394398
}

‎crates/ruff_linter/src/rules/pandas_vet/helpers.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti
5050
| BindingKind::ComprehensionVar
5151
| BindingKind::Global
5252
| BindingKind::Nonlocal(_) => Resolution::RelevantLocal,
53-
BindingKind::Import(import) if matches!(import.call_path(), ["pandas"]) => {
53+
BindingKind::Import(import)
54+
if matches!(import.qualified_name().segments(), ["pandas"]) =>
55+
{
5456
Resolution::PandasModule
5557
}
5658
_ => Resolution::IrrelevantBinding,

‎crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
178178
{
179179
let mut diagnostic = Diagnostic::new(
180180
UnusedImport {
181-
name: import.qualified_name(),
181+
name: import.qualified_name().to_string(),
182182
context: if in_except_handler {
183183
Some(UnusedImportContext::ExceptHandler)
184184
} else if in_init {
@@ -212,7 +212,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
212212
{
213213
let mut diagnostic = Diagnostic::new(
214214
UnusedImport {
215-
name: import.qualified_name(),
215+
name: import.qualified_name().to_string(),
216216
context: None,
217217
multiple: false,
218218
},

‎crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use itertools::Itertools;
44

55
use ruff_diagnostics::{Diagnostic, Violation};
66
use ruff_macros::{derive_message_formats, violation};
7+
use ruff_python_ast::name::QualifiedName;
78
use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope};
89
use ruff_text_size::Ranged;
910

@@ -113,7 +114,8 @@ pub(crate) fn import_private_name(
113114
// Ignore public imports; require at least one private name.
114115
// Ex) `from foo import bar`
115116
let Some((index, private_name)) = import_info
116-
.call_path
117+
.qualified_name
118+
.segments()
117119
.iter()
118120
.find_position(|name| name.starts_with('_'))
119121
else {
@@ -132,7 +134,7 @@ pub(crate) fn import_private_name(
132134

133135
let name = (*private_name).to_string();
134136
let module = if index > 0 {
135-
Some(import_info.call_path[..index].join("."))
137+
Some(import_info.qualified_name.segments()[..index].join("."))
136138
} else {
137139
None
138140
};
@@ -152,21 +154,22 @@ fn is_typing(reference: &ResolvedReference) -> bool {
152154
|| reference.in_runtime_evaluated_annotation()
153155
}
154156

157+
#[allow(clippy::struct_field_names)]
155158
struct ImportInfo<'a> {
156159
module_name: &'a [&'a str],
157160
member_name: Cow<'a, str>,
158-
call_path: &'a [&'a str],
161+
qualified_name: &'a QualifiedName<'a>,
159162
}
160163

161164
impl<'a> From<&'a FromImport<'_>> for ImportInfo<'a> {
162165
fn from(import: &'a FromImport) -> Self {
163166
let module_name = import.module_name();
164167
let member_name = import.member_name();
165-
let call_path = import.call_path();
168+
let qualified_name = import.qualified_name();
166169
Self {
167170
module_name,
168171
member_name,
169-
call_path,
172+
qualified_name,
170173
}
171174
}
172175
}
@@ -175,11 +178,11 @@ impl<'a> From<&'a Import<'_>> for ImportInfo<'a> {
175178
fn from(import: &'a Import) -> Self {
176179
let module_name = import.module_name();
177180
let member_name = import.member_name();
178-
let call_path = import.call_path();
181+
let qualified_name = import.qualified_name();
179182
Self {
180183
module_name,
181184
member_name,
182-
call_path,
185+
qualified_name,
183186
}
184187
}
185188
}

‎crates/ruff_linter/src/rules/ruff/typing.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,25 +298,25 @@ mod tests {
298298

299299
#[test]
300300
fn test_is_known_type() {
301-
assert!(is_known_type(&QualifiedName::from_slice(&["", "int"]), 11));
301+
assert!(is_known_type(&QualifiedName::builtin("int"), 11));
302302
assert!(is_known_type(
303-
&QualifiedName::from_slice(&["builtins", "int"]),
303+
&QualifiedName::from_iter(["builtins", "int"]),
304304
11
305305
));
306306
assert!(is_known_type(
307-
&QualifiedName::from_slice(&["typing", "Optional"]),
307+
&QualifiedName::from_iter(["typing", "Optional"]),
308308
11
309309
));
310310
assert!(is_known_type(
311-
&QualifiedName::from_slice(&["typing_extensions", "Literal"]),
311+
&QualifiedName::from_iter(["typing_extensions", "Literal"]),
312312
11
313313
));
314314
assert!(is_known_type(
315-
&QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]),
315+
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
316316
11
317317
));
318318
assert!(!is_known_type(
319-
&QualifiedName::from_slice(&["zoneinfo", "ZoneInfo"]),
319+
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
320320
8
321321
));
322322
}

‎crates/ruff_python_ast/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ itertools = { workspace = true }
2424
once_cell = { workspace = true }
2525
rustc-hash = { workspace = true }
2626
serde = { workspace = true, optional = true }
27-
smallvec = { workspace = true }
2827

2928
[dev-dependencies]
3029
insta = { workspace = true }

0 commit comments

Comments
 (0)