Properly record meaningful imports as re-exports in symbol index

src/tools/rust-analyzer/Cargo.lock

@@ -523,6 +523,7 @@ dependencies = [
  "hir-def",
  "hir-expand",
  "hir-ty",
+ "indexmap",
  "intern",
  "itertools",
  "rustc-hash 2.0.0",

src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs

@@ -162,6 +162,20 @@ impl ItemScope {
             .map(move |name| (name, self.get(name)))
     }
 
+    pub fn values(&self) -> impl Iterator<Item = (&Name, Item<ModuleDefId, ImportId>)> + '_ {
+        self.values.iter().map(|(n, &i)| (n, i))
+    }
+
+    pub fn types(
+        &self,
+    ) -> impl Iterator<Item = (&Name, Item<ModuleDefId, ImportOrExternCrate>)> + '_ {
+        self.types.iter().map(|(n, &i)| (n, i))
+    }
+
+    pub fn macros(&self) -> impl Iterator<Item = (&Name, Item<MacroId, ImportId>)> + '_ {
+        self.macros.iter().map(|(n, &i)| (n, i))
+    }
+
     pub fn imports(&self) -> impl Iterator<Item = ImportId> + '_ {
         self.use_imports_types
             .keys()
@@ -263,11 +277,6 @@ impl ItemScope {
         self.unnamed_consts.iter().copied()
     }
 
-    /// Iterate over all module scoped macros
-    pub(crate) fn macros(&self) -> impl Iterator<Item = (&Name, MacroId)> + '_ {
-        self.entries().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_)))
-    }
-
     /// Iterate over all legacy textual scoped macros visible at the end of the module
     pub fn legacy_macros(&self) -> impl Iterator<Item = (&Name, &[MacroId])> + '_ {
         self.legacy_macros.iter().map(|(name, def)| (name, &**def))

src/tools/rust-analyzer/crates/hir-def/src/lib.rs

@@ -502,7 +502,7 @@ impl ModuleId {
     }
 
     /// Whether this module represents the crate root module
-    fn is_crate_root(&self) -> bool {
+    pub fn is_crate_root(&self) -> bool {
         self.local_id == DefMap::ROOT && self.block.is_none()
     }
 }

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

@@ -717,8 +717,8 @@ impl DefCollector<'_> {
                 }
             }
             None => {
-                for (name, def) in root_scope.macros() {
+                for (name, it) in root_scope.macros() {
-                    self.def_map.macro_use_prelude.insert(name.clone(), (def, extern_crate));
+                    self.def_map.macro_use_prelude.insert(name.clone(), (it.def, extern_crate));
                 }
             }
         }

src/tools/rust-analyzer/crates/hir-def/src/visibility.rs

@@ -240,12 +240,12 @@ impl Visibility {
 
                 if a_ancestors.any(|m| m == mod_b.local_id) {
                     // B is above A
-                    return Some(Visibility::Module(mod_a, expl_b));
+                    return Some(Visibility::Module(mod_a, expl_a));
                 }
 
                 if b_ancestors.any(|m| m == mod_a.local_id) {
                     // A is above B
-                    return Some(Visibility::Module(mod_b, expl_a));
+                    return Some(Visibility::Module(mod_b, expl_b));
                 }
 
                 None

src/tools/rust-analyzer/crates/hir/Cargo.toml

@@ -20,6 +20,7 @@ itertools.workspace = true
 smallvec.workspace = true
 tracing.workspace = true
 triomphe.workspace = true
+indexmap.workspace = true
 
 # local deps
 base-db.workspace = true

src/tools/rust-analyzer/crates/hir/src/symbols.rs

@@ -1,22 +1,28 @@
 //! File symbol extraction.
 
+use either::Either;
 use hir_def::{
     db::DefDatabase,
-    item_scope::ItemInNs,
+    item_scope::{ImportId, ImportOrExternCrate},
+    per_ns::Item,
     src::{HasChildSource, HasSource},
-    AdtId, AssocItemId, DefWithBodyId, HasModule, ImplId, Lookup, MacroId, ModuleDefId, ModuleId,
+    visibility::{Visibility, VisibilityExplicitness},
-    TraitId,
+    AdtId, AssocItemId, DefWithBodyId, ExternCrateId, HasModule, ImplId, Lookup, MacroId,
+    ModuleDefId, ModuleId, TraitId,
 };
-use hir_expand::HirFileId;
+use hir_expand::{name::Name, HirFileId};
 use hir_ty::{
     db::HirDatabase,
     display::{hir_display_with_types_map, HirDisplay},
 };
+use rustc_hash::FxHashMap;
 use span::Edition;
 use syntax::{ast::HasName, AstNode, AstPtr, SmolStr, SyntaxNode, SyntaxNodePtr, ToSmolStr};
 
 use crate::{Module, ModuleDef, Semantics};
 
+pub type FxIndexSet<T> = indexmap::IndexSet<T, std::hash::BuildHasherDefault<rustc_hash::FxHasher>>;
+
 /// The actual data that is stored in the index. It should be as compact as
 /// possible.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -37,7 +43,7 @@ pub struct DeclarationLocation {
     /// This points to the whole syntax node of the declaration.
     pub ptr: SyntaxNodePtr,
     /// This points to the [`syntax::ast::Name`] identifier of the declaration.
-    pub name_ptr: AstPtr<syntax::ast::Name>,
+    pub name_ptr: AstPtr<Either<syntax::ast::Name, syntax::ast::NameRef>>,
 }
 
 impl DeclarationLocation {
@@ -55,7 +61,7 @@ struct SymbolCollectorWork {
 
 pub struct SymbolCollector<'a> {
     db: &'a dyn HirDatabase,
-    symbols: Vec<FileSymbol>,
+    symbols: FxIndexSet<FileSymbol>,
     work: Vec<SymbolCollectorWork>,
     current_container_name: Option<SmolStr>,
     edition: Edition,
@@ -87,7 +93,7 @@ impl<'a> SymbolCollector<'a> {
     }
 
     pub fn finish(self) -> Vec<FileSymbol> {
-        self.symbols
+        self.symbols.into_iter().collect()
     }
 
     pub fn collect_module(db: &dyn HirDatabase, module: Module) -> Vec<FileSymbol> {
@@ -104,83 +110,161 @@ impl<'a> SymbolCollector<'a> {
     }
 
     fn collect_from_module(&mut self, module_id: ModuleId) {
-        let def_map = module_id.def_map(self.db.upcast());
+        let push_decl = |this: &mut Self, def| {
-        let scope = &def_map[module_id.local_id].scope;
+            match def {
-
+                ModuleDefId::ModuleId(id) => this.push_module(id),
-        for module_def_id in scope.declarations() {
-            match module_def_id {
-                ModuleDefId::ModuleId(id) => self.push_module(id),
                 ModuleDefId::FunctionId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
-                    self.collect_from_body(id);
+                    this.collect_from_body(id);
                 }
-                ModuleDefId::AdtId(AdtId::StructId(id)) => self.push_decl(id, false),
+                ModuleDefId::AdtId(AdtId::StructId(id)) => this.push_decl(id, false),
-                ModuleDefId::AdtId(AdtId::EnumId(id)) => self.push_decl(id, false),
+                ModuleDefId::AdtId(AdtId::EnumId(id)) => this.push_decl(id, false),
-                ModuleDefId::AdtId(AdtId::UnionId(id)) => self.push_decl(id, false),
+                ModuleDefId::AdtId(AdtId::UnionId(id)) => this.push_decl(id, false),
                 ModuleDefId::ConstId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
-                    self.collect_from_body(id);
+                    this.collect_from_body(id);
                 }
                 ModuleDefId::StaticId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
-                    self.collect_from_body(id);
+                    this.collect_from_body(id);
                 }
                 ModuleDefId::TraitId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
-                    self.collect_from_trait(id);
+                    this.collect_from_trait(id);
                 }
                 ModuleDefId::TraitAliasId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
                 }
                 ModuleDefId::TypeAliasId(id) => {
-                    self.push_decl(id, false);
+                    this.push_decl(id, false);
                 }
                 ModuleDefId::MacroId(id) => match id {
-                    MacroId::Macro2Id(id) => self.push_decl(id, false),
+                    MacroId::Macro2Id(id) => this.push_decl(id, false),
-                    MacroId::MacroRulesId(id) => self.push_decl(id, false),
+                    MacroId::MacroRulesId(id) => this.push_decl(id, false),
-                    MacroId::ProcMacroId(id) => self.push_decl(id, false),
+                    MacroId::ProcMacroId(id) => this.push_decl(id, false),
                 },
                 // Don't index these.
                 ModuleDefId::BuiltinType(_) => {}
                 ModuleDefId::EnumVariantId(_) => {}
             }
-        }
+        };
+
+        // Nested trees are very common, so a cache here will hit a lot.
+        let import_child_source_cache = &mut FxHashMap::default();
+
+        let mut push_import = |this: &mut Self, i: ImportId, name: &Name, def: ModuleDefId| {
+            let source = import_child_source_cache
+                .entry(i.import)
+                .or_insert_with(|| i.import.child_source(this.db.upcast()));
+            let Some(use_tree_src) = source.value.get(i.idx) else { return };
+            let Some(name_ptr) = use_tree_src
+                .rename()
+                .and_then(|rename| rename.name())
+                .map(Either::Left)
+                .or_else(|| use_tree_src.path()?.segment()?.name_ref().map(Either::Right))
+                .map(|it| AstPtr::new(&it))
+            else {
+                return;
+            };
+            let dec_loc = DeclarationLocation {
+                hir_file_id: source.file_id,
+                ptr: SyntaxNodePtr::new(use_tree_src.syntax()),
+                name_ptr,
+            };
+            this.symbols.insert(FileSymbol {
+                name: name.as_str().into(),
+                def: def.into(),
+                container_name: this.current_container_name.clone(),
+                loc: dec_loc,
+                is_alias: false,
+                is_assoc: false,
+            });
+        };
+
+        let push_extern_crate =
+            |this: &mut Self, i: ExternCrateId, name: &Name, def: ModuleDefId| {
+                let loc = i.lookup(this.db.upcast());
+                let source = loc.source(this.db.upcast());
+                let Some(name_ptr) = source
+                    .value
+                    .rename()
+                    .and_then(|rename| rename.name())
+                    .map(Either::Left)
+                    .or_else(|| source.value.name_ref().map(Either::Right))
+                    .map(|it| AstPtr::new(&it))
+                else {
+                    return;
+                };
+                let dec_loc = DeclarationLocation {
+                    hir_file_id: source.file_id,
+                    ptr: SyntaxNodePtr::new(source.value.syntax()),
+                    name_ptr,
+                };
+                this.symbols.insert(FileSymbol {
+                    name: name.as_str().into(),
+                    def: def.into(),
+                    container_name: this.current_container_name.clone(),
+                    loc: dec_loc,
+                    is_alias: false,
+                    is_assoc: false,
+                });
+            };
+
+        let is_explicit_import = |vis| {
+            match vis {
+                Visibility::Module(_, VisibilityExplicitness::Explicit) => true,
+                Visibility::Module(_, VisibilityExplicitness::Implicit) => {
+                    // consider imports in the crate root explicit, as these are visibly
+                    // crate-wide anyways
+                    module_id.is_crate_root()
+                }
+                Visibility::Public => true,
+            }
+        };
+
+        let def_map = module_id.def_map(self.db.upcast());
+        let scope = &def_map[module_id.local_id].scope;
 
         for impl_id in scope.impls() {
             self.collect_from_impl(impl_id);
         }
 
-        // Record renamed imports.
+        for (name, Item { def, vis, import }) in scope.types() {
-        // FIXME: In case it imports multiple items under different namespaces we just pick one arbitrarily
+            if let Some(i) = import {
-        // for now.
+                if is_explicit_import(vis) {
-        for id in scope.imports() {
+                    match i {
-            let source = id.import.child_source(self.db.upcast());
+                        ImportOrExternCrate::Import(i) => push_import(self, i, name, def),
-            let Some(use_tree_src) = source.value.get(id.idx) else { continue };
+                        ImportOrExternCrate::ExternCrate(i) => {
-            let Some(rename) = use_tree_src.rename() else { continue };
+                            push_extern_crate(self, i, name, def)
-            let Some(name) = rename.name() else { continue };
+                        }
-
+                    }
-            let res = scope.fully_resolve_import(self.db.upcast(), id);
-            res.iter_items().for_each(|(item, _)| {
-                let def = match item {
-                    ItemInNs::Types(def) | ItemInNs::Values(def) => def,
-                    ItemInNs::Macros(def) => ModuleDefId::from(def),
                 }
-                .into();
+                continue;
-                let dec_loc = DeclarationLocation {
+            }
-                    hir_file_id: source.file_id,
+            // self is a declaration
-                    ptr: SyntaxNodePtr::new(use_tree_src.syntax()),
+            push_decl(self, def)
-                    name_ptr: AstPtr::new(&name),
+        }
-                };
 
-                self.symbols.push(FileSymbol {
+        for (name, Item { def, vis, import }) in scope.macros() {
-                    name: name.text().into(),
+            if let Some(i) = import {
-                    def,
+                if is_explicit_import(vis) {
-                    container_name: self.current_container_name.clone(),
+                    push_import(self, i, name, def.into());
-                    loc: dec_loc,
+                }
-                    is_alias: false,
+                continue;
-                    is_assoc: false,
+            }
-                });
+            // self is a declaration
-            });
+            push_decl(self, def.into())
+        }
+
+        for (name, Item { def, vis, import }) in scope.values() {
+            if let Some(i) = import {
+                if is_explicit_import(vis) {
+                    push_import(self, i, name, def);
+                }
+                continue;
+            }
+            // self is a declaration
+            push_decl(self, def)
         }
 
         for const_id in scope.unnamed_consts() {
@@ -287,12 +371,12 @@ impl<'a> SymbolCollector<'a> {
         let dec_loc = DeclarationLocation {
             hir_file_id: source.file_id,
             ptr: SyntaxNodePtr::new(source.value.syntax()),
-            name_ptr: AstPtr::new(&name_node),
+            name_ptr: AstPtr::new(&name_node).wrap_left(),
         };
 
         if let Some(attrs) = def.attrs(self.db) {
             for alias in attrs.doc_aliases() {
-                self.symbols.push(FileSymbol {
+                self.symbols.insert(FileSymbol {
                     name: alias.as_str().into(),
                     def,
                     loc: dec_loc.clone(),
@@ -303,7 +387,7 @@ impl<'a> SymbolCollector<'a> {
             }
         }
 
-        self.symbols.push(FileSymbol {
+        self.symbols.insert(FileSymbol {
             name: name_node.text().into(),
             def,
             container_name: self.current_container_name.clone(),
@@ -322,14 +406,14 @@ impl<'a> SymbolCollector<'a> {
         let dec_loc = DeclarationLocation {
             hir_file_id: declaration.file_id,
             ptr: SyntaxNodePtr::new(module.syntax()),
-            name_ptr: AstPtr::new(&name_node),
+            name_ptr: AstPtr::new(&name_node).wrap_left(),
         };
 
         let def = ModuleDef::Module(module_id.into());
 
         if let Some(attrs) = def.attrs(self.db) {
             for alias in attrs.doc_aliases() {
-                self.symbols.push(FileSymbol {
+                self.symbols.insert(FileSymbol {
                     name: alias.as_str().into(),
                     def,
                     loc: dec_loc.clone(),
@@ -340,7 +424,7 @@ impl<'a> SymbolCollector<'a> {
             }
         }
 
-        self.symbols.push(FileSymbol {
+        self.symbols.insert(FileSymbol {
             name: name_node.text().into(),
             def: ModuleDef::Module(module_id.into()),
             container_name: self.current_container_name.clone(),

src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs

@@ -1746,7 +1746,7 @@ fn intrinsics() {
     fn function() {
             transmute$0
     }
-    "#,
+"#,
         expect![[r#"
             fn transmute(…) (use core::mem::transmute) unsafe fn(Src) -> Dst
         "#]],
@@ -1767,7 +1767,9 @@ fn function() {
         mem::transmute$0
 }
 "#,
-        expect![""],
+        expect![[r#"
+            fn transmute(…) (use core::mem) unsafe fn(Src) -> Dst
+        "#]],
     );
 }
 

src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs

@@ -350,6 +350,9 @@ fn path_applicable_imports(
             .take(DEFAULT_QUERY_SEARCH_LIMIT.inner())
             .collect()
         }
+        // we have some unresolved qualifier that we search an import for
+        // The key here is that whatever we import must form a resolved path for the remainder of
+        // what follows
         [first_qsegment, qualifier_rest @ ..] => items_locator::items_with_name(
             sema,
             current_crate,
@@ -357,14 +360,16 @@ fn path_applicable_imports(
             AssocSearchMode::Exclude,
         )
         .filter_map(|item| {
-            import_for_item(
+            // we found imports for `first_qsegment`, now we need to filter these imports by whether
+            // they result in resolving the rest of the path successfully
+            validate_resolvable(
                 sema,
                 scope,
                 mod_path,
+                scope_filter,
                 &path_candidate.name,
                 item,
                 qualifier_rest,
-                scope_filter,
             )
         })
         .take(DEFAULT_QUERY_SEARCH_LIMIT.inner())
@@ -372,14 +377,16 @@ fn path_applicable_imports(
     }
 }
 
-fn import_for_item(
+/// Validates and builds an import for `resolved_qualifier` if the `unresolved_qualifier` appended
+/// to it resolves and there is a validate `candidate` after that.
+fn validate_resolvable(
     sema: &Semantics<'_, RootDatabase>,
     scope: &SemanticsScope<'_>,
     mod_path: impl Fn(ItemInNs) -> Option<ModPath>,
+    scope_filter: impl Fn(ItemInNs) -> bool,
     candidate: &NameToImport,
     resolved_qualifier: ItemInNs,
     unresolved_qualifier: &[SmolStr],
-    scope_filter: impl Fn(ItemInNs) -> bool,
 ) -> Option<LocatedImport> {
     let _p = tracing::info_span!("ImportAssets::import_for_item").entered();
 

src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs

@@ -108,6 +108,7 @@ pub fn items_with_name_in_module<'a>(
         }
     };
     let mut local_results = Vec::new();
+    // FIXME: This using module_symbols is likely wrong?
     local_query.search(&[sema.db.module_symbols(module)], |local_candidate| {
         local_results.push(match local_candidate.def {
             hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def),

src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs

@@ -476,9 +476,9 @@ use Macro as ItemLikeMacro;
 use Macro as Trait; // overlay namespaces
 //- /b_mod.rs
 struct StructInModB;
-use super::Macro as SuperItemLikeMacro;
+pub(self) use super::Macro as SuperItemLikeMacro;
-use crate::b_mod::StructInModB as ThisStruct;
+pub(self) use crate::b_mod::StructInModB as ThisStruct;
-use crate::Trait as IsThisJustATrait;
+pub(self) use crate::Trait as IsThisJustATrait;
 "#,
         );
 

src/tools/rust-analyzer/crates/ide-db/src/test_data/test_symbol_index_collection.txt

@@ -631,7 +631,7 @@
                 def: Function(
                     Function {
                         id: FunctionId(
-                            3,
+                            2,
                         ),
                     },
                 ),
@@ -664,7 +664,7 @@
                 def: Function(
                     Function {
                         id: FunctionId(
-                            2,
+                            1,
                         ),
                     },
                 ),
@@ -794,7 +794,7 @@
                 def: Function(
                     Function {
                         id: FunctionId(
-                            1,
+                            3,
                         ),
                     },
                 ),
@@ -877,6 +877,37 @@
             },
         },
         [
+            FileSymbol {
+                name: "IsThisJustATrait",
+                def: Trait(
+                    Trait {
+                        id: TraitId(
+                            0,
+                        ),
+                    },
+                ),
+                loc: DeclarationLocation {
+                    hir_file_id: EditionedFileId(
+                        FileId(
+                            1,
+                        ),
+                        Edition2021,
+                    ),
+                    ptr: SyntaxNodePtr {
+                        kind: USE_TREE,
+                        range: 141..173,
+                    },
+                    name_ptr: AstPtr(
+                        SyntaxNodePtr {
+                            kind: NAME,
+                            range: 157..173,
+                        },
+                    ),
+                },
+                container_name: None,
+                is_alias: false,
+                is_assoc: false,
+            },
             FileSymbol {
                 name: "IsThisJustATrait",
                 def: Macro(
@@ -897,12 +928,12 @@
                     ),
                     ptr: SyntaxNodePtr {
                         kind: USE_TREE,
-                        range: 111..143,
+                        range: 141..173,
                     },
                     name_ptr: AstPtr(
                         SyntaxNodePtr {
                             kind: NAME,
-                            range: 127..143,
+                            range: 157..173,
                         },
                     ),
                 },
@@ -963,78 +994,12 @@
                     ),
                     ptr: SyntaxNodePtr {
                         kind: USE_TREE,
-                        range: 25..59,
+                        range: 35..69,
                     },
                     name_ptr: AstPtr(
                         SyntaxNodePtr {
                             kind: NAME,
-                            range: 41..59,
+                            range: 51..69,
-                        },
-                    ),
-                },
-                container_name: None,
-                is_alias: false,
-                is_assoc: false,
-            },
-            FileSymbol {
-                name: "ThisStruct",
-                def: Adt(
-                    Struct(
-                        Struct {
-                            id: StructId(
-                                4,
-                            ),
-                        },
-                    ),
-                ),
-                loc: DeclarationLocation {
-                    hir_file_id: EditionedFileId(
-                        FileId(
-                            1,
-                        ),
-                        Edition2021,
-                    ),
-                    ptr: SyntaxNodePtr {
-                        kind: USE_TREE,
-                        range: 65..105,
-                    },
-                    name_ptr: AstPtr(
-                        SyntaxNodePtr {
-                            kind: NAME,
-                            range: 95..105,
-                        },
-                    ),
-                },
-                container_name: None,
-                is_alias: false,
-                is_assoc: false,
-            },
-            FileSymbol {
-                name: "ThisStruct",
-                def: Adt(
-                    Struct(
-                        Struct {
-                            id: StructId(
-                                4,
-                            ),
-                        },
-                    ),
-                ),
-                loc: DeclarationLocation {
-                    hir_file_id: EditionedFileId(
-                        FileId(
-                            1,
-                        ),
-                        Edition2021,
-                    ),
-                    ptr: SyntaxNodePtr {
-                        kind: USE_TREE,
-                        range: 65..105,
-                    },
-                    name_ptr: AstPtr(
-                        SyntaxNodePtr {
-                            kind: NAME,
-                            range: 95..105,
                         },
                     ),
                 },