From 2cbd68ab7056b3b798adc5904febe63de5119485 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Mon, 12 Jan 2026 14:51:06 -0500 Subject: [PATCH] [ty] Move fully qualified name into auto-import implementation Previously, we were constructing this at a higher level layer. But this commit pushes it down a layer of abstraction. This shouldn't result in constructing the fully qualified name any more frequently than we previously did. Namely, we're still only doing it for symbols that match the caller provided search query. The motivation here is that we want to do some de-duplication based on module name, and having the fully qualified name of a symbol seems like the most straight-forward way to go about this. (We'll need one more piece of information that we add in a subsequent commit.) --- crates/ty_ide/src/all_symbols.rs | 55 ++++++++++++++++++++++++++------ crates/ty_ide/src/completion.rs | 12 +++---- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/crates/ty_ide/src/all_symbols.rs b/crates/ty_ide/src/all_symbols.rs index b8f70d4362..d3c9a0d59a 100644 --- a/crates/ty_ide/src/all_symbols.rs +++ b/crates/ty_ide/src/all_symbols.rs @@ -1,4 +1,5 @@ use ruff_db::files::File; +use ruff_python_ast::name::Name; use ty_module_resolver::{Module, ModuleName, all_modules, resolve_real_shadowable_module}; use ty_project::Db; @@ -72,21 +73,14 @@ pub fn all_symbols<'db>( let _entered = symbols_for_file_span.entered(); if query.is_match_symbol_name(module.name(&*db)) { - results.lock().unwrap().push(AllSymbolInfo { - symbol: None, - module, - file, - }); + let symbol = AllSymbolInfo::from_module(&*db, module, file); + results.lock().unwrap().push(symbol); } for (_, symbol) in symbols_for_file_global_only(&*db, file).search(query) { // It seems like we could do better here than // locking `results` for every single symbol, // but this works pretty well as it is. - results.lock().unwrap().push(AllSymbolInfo { - symbol: Some(symbol.to_owned()), - module, - file, - }); + results.lock().unwrap().push(symbol); } }); } @@ -120,6 +114,8 @@ pub struct AllSymbolInfo<'db> { /// /// When absent, this implies the symbol is the module itself. symbol: Option>, + /// The fully qualified name of this symbol. + qualified: Name, /// The module containing the symbol. module: Module<'db>, /// The file containing the symbol. @@ -130,6 +126,35 @@ pub struct AllSymbolInfo<'db> { } impl<'db> AllSymbolInfo<'db> { + fn from_non_module_symbol( + db: &'_ dyn Db, + symbol: SymbolInfo<'static>, + module: Module<'db>, + file: File, + ) -> AllSymbolInfo<'db> { + let qualified = Name::from(compact_str::format_compact!( + "{module_name}.{name}", + module_name = module.name(db), + name = symbol.name, + )); + AllSymbolInfo { + symbol: Some(symbol), + qualified, + module, + file, + } + } + + /// Creates a new symbol referencing an entire module. + fn from_module(db: &'_ dyn Db, module: Module<'db>, file: File) -> AllSymbolInfo<'db> { + AllSymbolInfo { + symbol: None, + qualified: module.name(db).as_str().into(), + module, + file, + } + } + /// Returns the name of this symbol as it exists in a file. /// /// When absent, there is no concrete symbol in a module @@ -140,6 +165,16 @@ impl<'db> AllSymbolInfo<'db> { self.symbol.as_ref().map(|symbol| &*symbol.name) } + /// Returns the fully qualified name for this symbol. + /// + /// This includes the full absolute module name for the symbol. + /// + /// When this symbol corresponds to a module, then this is + /// just the full absolute module name itself. + pub fn qualified(&self) -> &str { + &self.qualified + } + /// Returns the "kind" of this symbol. /// /// The kind of a symbol in the context of auto-import is diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 217900bda1..56df9ae666 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -1563,23 +1563,19 @@ fn add_unimported_completions<'db>( } let module_name = symbol.module().name(db); - let (name, qualified, request) = symbol + let (name, request) = symbol .name_in_file() - .map(|name| { - let qualified = format!("{module_name}.{name}"); - (name, qualified, create_import_request(module_name, name)) - }) + .map(|name| (name, create_import_request(module_name, name))) .unwrap_or_else(|| { let name = module_name.as_str(); - let qualified = name.to_string(); - (name, qualified, ImportRequest::module(name)) + (name, ImportRequest::module(name)) }); let import_action = importer.import(request, &members); // N.B. We use `add_skip_query` here because `all_symbols` // already takes our query into account. completions.add_skip_query( Completion::builder(name) - .qualified(qualified) + .qualified(symbol.qualified()) .insert(import_action.symbol_text()) .kind(symbol.kind().to_completion_kind()) .module_name(module_name)