Repository: rust-analyzer/rust-analyzer
Pull request #11282 (2022-01-14 10:08:13)

fix: Properly cache files in Semantics when ascending macros
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/11280
bors r+


Patch:
diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs
index a3f4a65513..8669b00ca1 100644
--- a/crates/hir/src/has_source.rs
+++ b/crates/hir/src/has_source.rs
@@ -16,6 +16,9 @@ use crate::{
 
 pub trait HasSource {
     type Ast;
+    /// Fetches the definition's source node.
+    /// Using [`crate::Semantics::source`] is preferred when working with [`crate::Semantics`],
+    /// as that caches the parsed file in the semantics' cache.
     fn source(self, db: &dyn HirDatabase) -> Option<InFile<Self::Ast>>;
 }
 
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 4b4eb38336..95696936b8 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -28,7 +28,6 @@ mod has_source;
 
 pub mod diagnostics;
 pub mod db;
-pub mod symbols;
 
 mod display;
 
@@ -41,18 +40,14 @@ use hir_def::{
     adt::{ReprKind, VariantData},
     body::{BodyDiagnostic, SyntheticSyntax},
     expr::{BindingAnnotation, LabelId, Pat, PatId},
-    item_tree::ItemTreeNode,
     lang_item::LangItemTarget,
     nameres::{self, diagnostics::DefDiagnostic},
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
-    src::HasSource as _,
-    AdtId, AssocItemId, AssocItemLoc, AttrDefId, ConstId, ConstParamId, DefWithBodyId, EnumId,
-    FunctionId, GenericDefId, HasModule, ImplId, ItemContainerId, LifetimeParamId,
-    LocalEnumVariantId, LocalFieldId, Lookup, ModuleId, StaticId, StructId, TraitId, TypeAliasId,
-    TypeParamId, UnionId,
+    AttrDefId, ConstId, ConstParamId, EnumId, FunctionId, GenericDefId, HasModule, LifetimeParamId,
+    LocalEnumVariantId, LocalFieldId, StaticId, StructId, TypeAliasId, TypeParamId, UnionId,
 };
-use hir_expand::{name::name, MacroCallKind, MacroDefId, MacroDefKind};
+use hir_expand::{name::name, MacroCallKind, MacroDefKind};
 use hir_ty::{
     autoderef,
     consteval::{eval_const, ComputedExpr, ConstEvalCtx, ConstEvalError, ConstExt},
@@ -111,14 +106,28 @@ pub use {
         builtin_attr::AttributeTemplate,
         find_path::PrefixKind,
         import_map,
-        nameres::ModuleSource,
+        item_scope::ItemScope,
+        item_tree::ItemTreeNode,
+        nameres::{DefMap, ModuleData, ModuleOrigin, ModuleSource},
         path::{ModPath, PathKind},
+        src::HasSource as DefHasSource, // xx: I don't like this shadowing of HasSource... :(
         type_ref::{Mutability, TypeRef},
         visibility::Visibility,
+        AdtId,
+        AssocItemId,
+        AssocItemLoc,
+        DefWithBodyId,
+        ImplId,
+        ItemContainerId,
+        ItemLoc,
+        Lookup,
+        ModuleDefId,
+        ModuleId,
+        TraitId,
     },
     hir_expand::{
         name::{known, Name},
-        ExpandResult, HirFileId, InFile, MacroFile, Origin,
+        ExpandResult, HirFileId, InFile, MacroDefId, MacroFile, Origin,
     },
     hir_ty::display::HirDisplay,
 };
@@ -182,11 +191,6 @@ impl Crate {
         Module { id: def_map.module_id(def_map.root()) }
     }
 
-    pub fn modules(self, db: &dyn HirDatabase) -> Vec<Module> {
-        let def_map = db.crate_def_map(self.id);
-        def_map.modules().map(|(id, _)| def_map.module_id(id).into()).collect()
-    }
-
     pub fn root_file(self, db: &dyn HirDatabase) -> FileId {
         db.crate_graph()[self.id].root_file_id
     }
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 444fb4ea54..9596e81818 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -2,7 +2,7 @@
 
 mod source_to_def;
 
-use std::{cell::RefCell, fmt};
+use std::{cell::RefCell, fmt, iter};
 
 use base_db::{FileId, FileRange};
 use either::Either;
@@ -443,8 +443,7 @@ impl<'db> SemanticsImpl<'db> {
     fn expand(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
         let sa = self.analyze_no_infer(macro_call.syntax());
         let file_id = sa.expand(self.db, InFile::new(sa.file_id, macro_call))?;
-        let node = self.db.parse_or_expand(file_id)?;
-        self.cache(node.clone(), file_id);
+        let node = self.parse_or_expand(file_id)?;
         Some(node)
     }
 
@@ -452,8 +451,7 @@ impl<'db> SemanticsImpl<'db> {
         let src = self.find_file(item.syntax()).with_value(item.clone());
         let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(src))?;
         let file_id = macro_call_id.as_file();
-        let node = self.db.parse_or_expand(file_id)?;
-        self.cache(node.clone(), file_id);
+        let node = self.parse_or_expand(file_id)?;
         Some(node)
     }
 
@@ -750,10 +748,9 @@ impl<'db> SemanticsImpl<'db> {
     }
 
     fn diagnostics_display_range(&self, src: InFile<SyntaxNodePtr>) -> FileRange {
-        let root = self.db.parse_or_expand(src.file_id).unwrap();
-        let node = src.value.to_node(&root);
-        self.cache(root, src.file_id);
-        src.with_value(&node).original_file_range(self.db.upcast())
+        let root = self.parse_or_expand(src.file_id).unwrap();
+        let node = src.map(|it| it.to_node(&root));
+        node.as_ref().original_file_range(self.db.upcast())
     }
 
     fn token_ancestors_with_macros(
@@ -768,7 +765,17 @@ impl<'db> SemanticsImpl<'db> {
         node: SyntaxNode,
     ) -> impl Iterator<Item = SyntaxNode> + Clone + '_ {
         let node = self.find_file(&node);
-        node.ancestors_with_macros(self.db.upcast()).map(|it| it.value)
+        let db = self.db.upcast();
+        iter::successors(Some(node.cloned()), move |&InFile { file_id, ref value }| {
+            match value.parent() {
+                Some(parent) => Some(InFile::new(file_id, parent)),
+                None => {
+                    self.cache(value.clone(), file_id);
+                    file_id.call_node(db)
+                }
+            }
+        })
+        .map(|it| it.value)
     }
 
     fn ancestors_at_offset_with_macros(
diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs
deleted file mode 100644
index f1669d5288..0000000000
--- a/crates/hir/src/symbols.rs
+++ /dev/null
@@ -1,359 +0,0 @@
-//! File symbol extraction.
-
-use base_db::FileRange;
-use either::Either;
-use hir_def::{
-    item_tree::ItemTreeNode, src::HasSource, AdtId, AssocItemId, AssocItemLoc, DefWithBodyId,
-    ImplId, ItemContainerId, ItemLoc, Lookup, ModuleDefId, ModuleId, TraitId,
-};
-use hir_expand::{HirFileId, InFile};
-use hir_ty::db::HirDatabase;
-use syntax::{ast::HasName, AstNode, SmolStr, SyntaxNode, SyntaxNodePtr};
-
-use crate::{HasSource as _, MacroDef, Module, Semantics};
-
-/// The actual data that is stored in the index. It should be as compact as
-/// possible.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub struct FileSymbol {
-    pub name: SmolStr,
-    pub loc: DeclarationLocation,
-    pub kind: FileSymbolKind,
-    pub container_name: Option<SmolStr>,
-}
-
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub struct DeclarationLocation {
-    /// The file id for both the `ptr` and `name_ptr`.
-    pub hir_file_id: HirFileId,
-    /// 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: SyntaxNodePtr,
-}
-
-impl DeclarationLocation {
-    pub fn syntax<DB: HirDatabase>(&self, sema: &Semantics<DB>) -> Option<SyntaxNode> {
-        let root = sema.parse_or_expand(self.hir_file_id)?;
-        Some(self.ptr.to_node(&root))
-    }
-
-    pub fn original_range(&self, db: &dyn HirDatabase) -> Option<FileRange> {
-        find_original_file_range(db, self.hir_file_id, &self.ptr)
-    }
-
-    pub fn original_name_range(&self, db: &dyn HirDatabase) -> Option<FileRange> {
-        find_original_file_range(db, self.hir_file_id, &self.name_ptr)
-    }
-}
-
-fn find_original_file_range(
-    db: &dyn HirDatabase,
-    file_id: HirFileId,
-    ptr: &SyntaxNodePtr,
-) -> Option<FileRange> {
-    let root = db.parse_or_expand(file_id)?;
-    let node = ptr.to_node(&root);
-    let node = InFile::new(file_id, &node);
-
-    Some(node.original_file_range(db.upcast()))
-}
-
-#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
-pub enum FileSymbolKind {
-    Const,
-    Enum,
-    Function,
-    Macro,
-    Module,
-    Static,
-    Struct,
-    Trait,
-    TypeAlias,
-    Union,
-}
-
-impl FileSymbolKind {
-    pub fn is_type(self: FileSymbolKind) -> bool {
-        matches!(
-            self,
-            FileSymbolKind::Struct
-                | FileSymbolKind::Enum
-                | FileSymbolKind::Trait
-                | FileSymbolKind::TypeAlias
-                | FileSymbolKind::Union
-        )
-    }
-}
-
-/// Represents an outstanding module that the symbol collector must collect symbols from.
-struct SymbolCollectorWork {
-    module_id: ModuleId,
-    parent: Option<DefWithBodyId>,
-}
-
-pub struct SymbolCollector<'a> {
-    db: &'a dyn HirDatabase,
-    symbols: Vec<FileSymbol>,
-    work: Vec<SymbolCollectorWork>,
-    current_container_name: Option<SmolStr>,
-}
-
-/// Given a [`ModuleId`] and a [`HirDatabase`], use the DefMap for the module's crate to collect
-/// all symbols that should be indexed for the given module.
-impl<'a> SymbolCollector<'a> {
-    pub fn collect(db: &dyn HirDatabase, module: Module) -> Vec<FileSymbol> {
-        let mut symbol_collector = SymbolCollector {
-            db,
-            symbols: Default::default(),
-            current_container_name: None,
-            // The initial work is the root module we're collecting, additional work will
-            // be populated as we traverse the module's definitions.
-            work: vec![SymbolCollectorWork { module_id: module.into(), parent: None }],
-        };
-
-        while let Some(work) = symbol_collector.work.pop() {
-            symbol_collector.do_work(work);
-        }
-
-        symbol_collector.symbols
-    }
-
-    fn do_work(&mut self, work: SymbolCollectorWork) {
-        self.db.unwind_if_cancelled();
-
-        let parent_name = work.parent.and_then(|id| self.def_with_body_id_name(id));
-        self.with_container_name(parent_name, |s| s.collect_from_module(work.module_id));
-    }
-
-    fn collect_from_module(&mut self, module_id: ModuleId) {
-        let def_map = module_id.def_map(self.db.upcast());
-        let scope = &def_map[module_id.local_id].scope;
-
-        for module_def_id in scope.declarations() {
-            match module_def_id {
-                ModuleDefId::ModuleId(id) => self.push_module(id),
-                ModuleDefId::FunctionId(id) => {
-                    self.push_decl_assoc(id, FileSymbolKind::Function);
-                    self.collect_from_body(id);
-                }
-                ModuleDefId::AdtId(AdtId::StructId(id)) => {
-                    self.push_decl(id, FileSymbolKind::Struct)
-                }
-                ModuleDefId::AdtId(AdtId::EnumId(id)) => self.push_decl(id, FileSymbolKind::Enum),
-                ModuleDefId::AdtId(AdtId::UnionId(id)) => self.push_decl(id, FileSymbolKind::Union),
-                ModuleDefId::ConstId(id) => {
-                    self.push_decl_assoc(id, FileSymbolKind::Const);
-                    self.collect_from_body(id);
-                }
-                ModuleDefId::StaticId(id) => {
-                    self.push_decl_assoc(id, FileSymbolKind::Static);
-                    self.collect_from_body(id);
-                }
-                ModuleDefId::TraitId(id) => {
-                    self.push_decl(id, FileSymbolKind::Trait);
-                    self.collect_from_trait(id);
-                }
-                ModuleDefId::TypeAliasId(id) => {
-                    self.push_decl_assoc(id, FileSymbolKind::TypeAlias);
-                }
-                // Don't index these.
-                ModuleDefId::BuiltinType(_) => {}
-                ModuleDefId::EnumVariantId(_) => {}
-            }
-        }
-
-        for impl_id in scope.impls() {
-            self.collect_from_impl(impl_id);
-        }
-
-        for const_id in scope.unnamed_consts() {
-            self.collect_from_body(const_id);
-        }
-
-        for macro_def_id in scope.macro_declarations() {
-            self.push_decl_macro(macro_def_id.into());
-        }
-    }
-
-    fn collect_from_body(&mut self, body_id: impl Into<DefWithBodyId>) {
-        let body_id = body_id.into();
-        let body = self.db.body(body_id);
-
-        // Descend into the blocks and enqueue collection of all modules within.
-        for (_, def_map) in body.blocks(self.db.upcast()) {
-            for (id, _) in def_map.modules() {
-                self.work.push(SymbolCollectorWork {
-                    module_id: def_map.module_id(id),
-                    parent: Some(body_id),
-                });
-            }
-        }
-    }
-
-    fn collect_from_impl(&mut self, impl_id: ImplId) {
-        let impl_data = self.db.impl_data(impl_id);
-        for &assoc_item_id in &impl_data.items {
-            self.push_assoc_item(assoc_item_id)
-        }
-    }
-
-    fn collect_from_trait(&mut self, trait_id: TraitId) {
-        let trait_data = self.db.trait_data(trait_id);
-        self.with_container_name(trait_data.name.as_text(), |s| {
-            for &(_, assoc_item_id) in &trait_data.items {
-                s.push_assoc_item(assoc_item_id);
-            }
-        });
-    }
-
-    fn with_container_name(&mut self, container_name: Option<SmolStr>, f: impl FnOnce(&mut Self)) {
-        if let Some(container_name) = container_name {
-            let prev = self.current_container_name.replace(container_name);
-            f(self);
-            self.current_container_name = prev;
-        } else {
-            f(self);
-        }
-    }
-
-    fn current_container_name(&self) -> Option<SmolStr> {
-        self.current_container_name.clone()
-    }
-
-    fn def_with_body_id_name(&self, body_id: DefWithBodyId) -> Option<SmolStr> {
-        match body_id {
-            DefWithBodyId::FunctionId(id) => Some(
-                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
-            ),
-            DefWithBodyId::StaticId(id) => Some(
-                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
-            ),
-            DefWithBodyId::ConstId(id) => Some(
-                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
-            ),
-        }
-    }
-
-    fn push_assoc_item(&mut self, assoc_item_id: AssocItemId) {
-        match assoc_item_id {
-            AssocItemId::FunctionId(id) => self.push_decl_assoc(id, FileSymbolKind::Function),
-            AssocItemId::ConstId(id) => self.push_decl_assoc(id, FileSymbolKind::Const),
-            AssocItemId::TypeAliasId(id) => self.push_decl_assoc(id, FileSymbolKind::TypeAlias),
-        }
-    }
-
-    fn push_decl_assoc<L, T>(&mut self, id: L, kind: FileSymbolKind)
-    where
-        L: Lookup<Data = AssocItemLoc<T>>,
-        T: ItemTreeNode,
-        <T as ItemTreeNode>::Source: HasName,
-    {
-        fn container_name(db: &dyn HirDatabase, container: ItemContainerId) -> Option<SmolStr> {
-            match container {
-                ItemContainerId::ModuleId(module_id) => {
-                    let module = Module::from(module_id);
-                    module.name(db).and_then(|name| name.as_text())
-                }
-                ItemContainerId::TraitId(trait_id) => {
-                    let trait_data = db.trait_data(trait_id);
-                    trait_data.name.as_text()
-                }
-                ItemContainerId::ImplId(_) | ItemContainerId::ExternBlockId(_) => None,
-            }
-        }
-
-        self.push_file_symbol(|s| {
-            let loc = id.lookup(s.db.upcast());
-            let source = loc.source(s.db.upcast());
-            let name_node = source.value.name()?;
-            let container_name =
-                container_name(s.db, loc.container).or_else(|| s.current_container_name());
-
-            Some(FileSymbol {
-                name: name_node.text().into(),
-                kind,
-                container_name,
-                loc: DeclarationLocation {
-                    hir_file_id: source.file_id,
-                    ptr: SyntaxNodePtr::new(source.value.syntax()),
-                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
-                },
-            })
-        })
-    }
-
-    fn push_decl<L, T>(&mut self, id: L, kind: FileSymbolKind)
-    where
-        L: Lookup<Data = ItemLoc<T>>,
-        T: ItemTreeNode,
-        <T as ItemTreeNode>::Source: HasName,
-    {
-        self.push_file_symbol(|s| {
-            let loc = id.lookup(s.db.upcast());
-            let source = loc.source(s.db.upcast());
-            let name_node = source.value.name()?;
-
-            Some(FileSymbol {
-                name: name_node.text().into(),
-                kind,
-                container_name: s.current_container_name(),
-                loc: DeclarationLocation {
-                    hir_file_id: source.file_id,
-                    ptr: SyntaxNodePtr::new(source.value.syntax()),
-                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
-                },
-            })
-        })
-    }
-
-    fn push_module(&mut self, module_id: ModuleId) {
-        self.push_file_symbol(|s| {
-            let def_map = module_id.def_map(s.db.upcast());
-            let module_data = &def_map[module_id.local_id];
-            let declaration = module_data.origin.declaration()?;
-            let module = declaration.to_node(s.db.upcast());
-            let name_node = module.name()?;
-
-            Some(FileSymbol {
-                name: name_node.text().into(),
-                kind: FileSymbolKind::Module,
-                container_name: s.current_container_name(),
-                loc: DeclarationLocation {
-                    hir_file_id: declaration.file_id,
-                    ptr: SyntaxNodePtr::new(module.syntax()),
-                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
-                },
-            })
-        })
-    }
-
-    fn push_decl_macro(&mut self, macro_def: MacroDef) {
-        self.push_file_symbol(|s| {
-            let name = macro_def.name(s.db)?.as_text()?;
-            let source = macro_def.source(s.db)?;
-
-            let (ptr, name_ptr) = match source.value {
-                Either::Left(m) => {
-                    (SyntaxNodePtr::new(m.syntax()), SyntaxNodePtr::new(m.name()?.syntax()))
-                }
-                Either::Right(f) => {
-                    (SyntaxNodePtr::new(f.syntax()), SyntaxNodePtr::new(f.name()?.syntax()))
-                }
-            };
-
-            Some(FileSymbol {
-                name,
-                kind: FileSymbolKind::Macro,
-                container_name: s.current_container_name(),
-                loc: DeclarationLocation { hir_file_id: source.file_id, name_ptr, ptr },
-            })
-        })
-    }
-
-    fn push_file_symbol(&mut self, f: impl FnOnce(&Self) -> Option<FileSymbol>) {
-        if let Some(file_symbol) = f(self) {
-            self.symbols.push(file_symbol);
-        }
-    }
-}
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index 5e49d3f6af..b455fd5917 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -636,10 +636,7 @@ impl<'a> InFile<&'a SyntaxNode> {
     ) -> impl Iterator<Item = InFile<SyntaxNode>> + Clone + '_ {
         iter::successors(Some(self.cloned()), move |node| match node.value.parent() {
             Some(parent) => Some(node.with_value(parent)),
-            None => {
-                let parent_node = node.file_id.call_node(db)?;
-                Some(parent_node)
-            }
+            None => node.file_id.call_node(db),
         })
     }
 
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index db7b80f71b..b8482d0222 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -66,7 +66,8 @@ use ide_db::{
         salsa::{self, ParallelDatabase},
         Env, FileLoader, FileSet, SourceDatabase, VfsPath,
     },
-    symbol_index, LineIndexDatabase,
+    symbol_index::{self, FileSymbol},
+    LineIndexDatabase,
 };
 use syntax::SourceFile;
 
diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs
index 1c3ff06247..52e099513a 100644
--- a/crates/ide/src/navigation_target.rs
+++ b/crates/ide/src/navigation_target.rs
@@ -4,8 +4,8 @@ use std::fmt;
 
 use either::Either;
 use hir::{
-    symbols::FileSymbol, AssocItem, Documentation, FieldSource, HasAttrs, HasSource, HirDisplay,
-    InFile, ModuleSource, Semantics,
+    AssocItem, Documentation, FieldSource, HasAttrs, HasSource, HirDisplay, InFile, ModuleSource,
+    Semantics,
 };
 use ide_db::{
     base_db::{FileId, FileRange},
@@ -17,6 +17,8 @@ use syntax::{
     match_ast, AstNode, SmolStr, TextRange,
 };
 
+use crate::FileSymbol;
+
 /// `NavigationTarget` represents an element in the editor's UI which you can
 /// click on to navigate to a particular piece of code.
 ///
diff --git a/crates/ide_assists/src/handlers/expand_glob_import.rs b/crates/ide_assists/src/handlers/expand_glob_import.rs
index 4def6543ab..72c17b674c 100644
--- a/crates/ide_assists/src/handlers/expand_glob_import.rs
+++ b/crates/ide_assists/src/handlers/expand_glob_import.rs
@@ -226,7 +226,9 @@ fn find_imported_defs(ctx: &AssistContext, star: SyntaxToken) -> Option<Vec<Defi
             .flat_map(|n| n.descendants().filter_map(ast::NameRef::cast))
             .filter_map(|r| match NameRefClass::classify(&ctx.sema, &r)? {
                 NameRefClass::Definition(
-                    def @ (Definition::Macro(_)
+                    def
+                    @
+                    (Definition::Macro(_)
                     | Definition::Module(_)
                     | Definition::Function(_)
                     | Definition::Adt(_)
diff --git a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
index 3121e22981..b0b9ad440c 100644
--- a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
@@ -216,7 +216,7 @@ fs::Path
     ",
         );
     }
-
+    #[test]
     #[test]
     fn dont_import_trivial_paths() {
         cov_mark::check!(dont_import_trivial_paths);
diff --git a/crates/ide_completion/src/completions/attribute.rs b/crates/ide_completion/src/completions/attribute.rs
index 2603ad2003..f67d7d56d8 100644
--- a/crates/ide_completion/src/completions/attribute.rs
+++ b/crates/ide_completion/src/completions/attribute.rs
@@ -56,7 +56,6 @@ pub(crate) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext)
             _ => (),
         },
         (_, Some(_)) => (),
-        (_, None) if attribute.expr().is_some() => (),
         (_, None) => complete_new_attribute(acc, ctx, attribute),
     }
     Some(())
diff --git a/crates/ide_completion/src/completions/fn_param.rs b/crates/ide_completion/src/completions/fn_param.rs
index a55bab67e1..bb4ce0a24d 100644
--- a/crates/ide_completion/src/completions/fn_param.rs
+++ b/crates/ide_completion/src/completions/fn_param.rs
@@ -83,15 +83,9 @@ fn remove_duplicated(
         let whole_param = param.syntax().text().to_string();
         file_params.remove(&whole_param);
 
-        match param.pat() {
-            // remove suggestions for patterns that already exist
-            // if the type is missing we are checking the current param to be completed
-            // in which case this would find itself removing the suggestions due to itself
-            Some(pattern) if param.ty().is_some() => {
-                let binding = pattern.syntax().text().to_string();
-                file_params.retain(|_, v| v != &binding);
-            }
-            _ => (),
+        if let Some(pattern) = param.pat() {
+            let binding = pattern.syntax().text().to_string();
+            file_params.retain(|_, v| v != &binding);
         }
     })
 }
diff --git a/crates/ide_completion/src/completions/postfix.rs b/crates/ide_completion/src/completions/postfix.rs
index 0dfb8abb8b..a212a98461 100644
--- a/crates/ide_completion/src/completions/postfix.rs
+++ b/crates/ide_completion/src/completions/postfix.rs
@@ -225,10 +225,6 @@ fn build_postfix_snippet_builder<'ctx>(
 ) -> Option<impl Fn(&str, &str, &str) -> Builder + 'ctx> {
     let receiver_syntax = receiver.syntax();
     let receiver_range = ctx.sema.original_range_opt(receiver_syntax)?.range;
-    if ctx.source_range().end() < receiver_range.start() {
-        // This shouldn't happen, yet it does. I assume this might be due to an incorrect token mapping.
-        return None;
-    }
     let delete_range = TextRange::new(receiver_range.start(), ctx.source_range().end());
 
     // Wrapping impl Fn in an option ruins lifetime inference for the parameters in a way that
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index 499791abc9..2a89bd29fa 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -171,7 +171,9 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
             }
         }
         hir::PathResolution::Def(
-            def @ (hir::ModuleDef::Adt(_)
+            def
+            @
+            (hir::ModuleDef::Adt(_)
             | hir::ModuleDef::TypeAlias(_)
             | hir::ModuleDef::BuiltinType(_)),
         ) => {
diff --git a/crates/ide_completion/src/completions/trait_impl.rs b/crates/ide_completion/src/completions/trait_impl.rs
index 46da7cfa37..b214c5c154 100644
--- a/crates/ide_completion/src/completions/trait_impl.rs
+++ b/crates/ide_completion/src/completions/trait_impl.rs
@@ -31,7 +31,7 @@
 //! }
 //! ```
 
-use hir::{self, HasAttrs, HasSource};
+use hir::{self, HasAttrs};
 use ide_db::{path_transform::PathTransform, traits::get_missing_assoc_items, SymbolKind};
 use syntax::{
     ast::{self, edit_in_place::AttrsOwnerEdit},
@@ -151,7 +151,7 @@ fn add_function_impl(
 
     let range = replacement_range(ctx, fn_def_node);
 
-    if let Some(source) = func.source(ctx.db) {
+    if let Some(source) = ctx.sema.source(func) {
         let assoc_item = ast::AssocItem::Fn(source.value);
         if let Some(transformed_item) = get_transformed_assoc_item(ctx, assoc_item, impl_def) {
             let transformed_fn = match transformed_item {
@@ -189,7 +189,7 @@ fn get_transformed_assoc_item(
         target_scope,
         source_scope,
         trait_,
-        impl_def.source(ctx.db)?.value,
+        ctx.sema.source(impl_def)?.value,
     );
 
     transform.apply(assoc_item.syntax());
@@ -227,7 +227,7 @@ fn add_const_impl(
     let const_name = const_.name(ctx.db).map(|n| n.to_smol_str());
 
     if let Some(const_name) = const_name {
-        if let Some(source) = const_.source(ctx.db) {
+        if let Some(source) = ctx.sema.source(const_) {
             let assoc_item = ast::AssocItem::Const(source.value);
             if let Some(transformed_item) = get_transformed_assoc_item(ctx, assoc_item, impl_def) {
                 let transformed_const = match transformed_item {
diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs
index 5386cbcb27..8d6a634953 100644
--- a/crates/ide_completion/src/render/macro_.rs
+++ b/crates/ide_completion/src/render/macro_.rs
@@ -1,8 +1,8 @@
 //! Renderer for macro invocations.
 
 use either::Either;
-use hir::{db::HirDatabase, Documentation, HasSource};
-use ide_db::SymbolKind;
+use hir::{Documentation, HasSource, InFile, Semantics};
+use ide_db::{RootDatabase, SymbolKind};
 use syntax::{
     display::{fn_as_proc_macro_label, macro_label},
     SmolStr,
@@ -30,8 +30,6 @@ fn render(
     macro_: hir::MacroDef,
     import_to_add: Option<ImportEdit>,
 ) -> CompletionItem {
-    let db = completion.db;
-
     let source_range = if completion.is_immediately_after_macro_bang() {
         cov_mark::hit!(completes_macro_call_if_cursor_at_bang_token);
         completion.token.parent().map_or_else(|| ctx.source_range(), |it| it.text_range())
@@ -54,7 +52,7 @@ fn render(
         label(&ctx, needs_bang, bra, ket, &name),
     );
     item.set_deprecated(ctx.is_deprecated(macro_))
-        .set_detail(detail(db, macro_))
+        .set_detail(detail(&completion.sema, macro_))
         .set_documentation(docs);
 
     if let Some(import_to_add) = import_to_add {
@@ -104,9 +102,11 @@ fn banged_name(name: &str) -> SmolStr {
     SmolStr::from_iter([name, "!"])
 }
 
-fn detail(db: &dyn HirDatabase, macro_: hir::MacroDef) -> Option<String> {
+fn detail(sema: &Semantics<RootDatabase>, macro_: hir::MacroDef) -> Option<String> {
     // FIXME: This is parsing the file!
-    let detail = match macro_.source(db)?.value {
+    let InFile { file_id, value } = macro_.source(sema.db)?;
+    let _ = sema.parse_or_expand(file_id);
+    let detail = match value {
         Either::Left(node) => macro_label(&node),
         Either::Right(node) => fn_as_proc_macro_label(&node),
     };
diff --git a/crates/ide_db/src/items_locator.rs b/crates/ide_db/src/items_locator.rs
index db38a48783..ca8266b6d2 100644
--- a/crates/ide_db/src/items_locator.rs
+++ b/crates/ide_db/src/items_locator.rs
@@ -5,7 +5,6 @@
 use either::Either;
 use hir::{
     import_map::{self, ImportKind},
-    symbols::FileSymbol,
     AsAssocItem, Crate, ItemInNs, Semantics,
 };
 use limit::Limit;
@@ -14,7 +13,8 @@ use syntax::{ast, AstNode, SyntaxKind::NAME};
 use crate::{
     defs::{Definition, NameClass},
     helpers::import_assets::NameToImport,
-    symbol_index, RootDatabase,
+    symbol_index::{self, FileSymbol},
+    RootDatabase,
 };
 
 /// A value to use, when uncertain which limit to pick.
diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs
index 602eaf0de5..bcbb09e31b 100644
--- a/crates/ide_db/src/lib.rs
+++ b/crates/ide_db/src/lib.rs
@@ -26,10 +26,7 @@ use base_db::{
     salsa::{self, Durability},
     AnchoredPath, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDatabase, Upcast,
 };
-use hir::{
-    db::{AstDatabase, DefDatabase, HirDatabase},
-    symbols::FileSymbolKind,
-};
+use hir::db::{AstDatabase, DefDatabase, HirDatabase};
 use rustc_hash::FxHashSet;
 
 use crate::{line_index::LineIndex, symbol_index::SymbolsDatabase};
@@ -186,23 +183,6 @@ impl From<hir::MacroKind> for SymbolKind {
     }
 }
 
-impl From<FileSymbolKind> for SymbolKind {
-    fn from(it: FileSymbolKind) -> Self {
-        match it {
-            FileSymbolKind::Const => SymbolKind::Const,
-            FileSymbolKind::Enum => SymbolKind::Enum,
-            FileSymbolKind::Function => SymbolKind::Function,
-            FileSymbolKind::Macro => SymbolKind::Macro,
-            FileSymbolKind::Module => SymbolKind::Module,
-            FileSymbolKind::Static => SymbolKind::Static,
-            FileSymbolKind::Struct => SymbolKind::Struct,
-            FileSymbolKind::Trait => SymbolKind::Trait,
-            FileSymbolKind::TypeAlias => SymbolKind::TypeAlias,
-            FileSymbolKind::Union => SymbolKind::Union,
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     mod sourcegen_lints;
diff --git a/crates/ide_db/src/symbol_index.rs b/crates/ide_db/src/symbol_index.rs
index 9697ba24b8..62f4a81910 100644
--- a/crates/ide_db/src/symbol_index.rs
+++ b/crates/ide_db/src/symbol_index.rs
@@ -30,18 +30,21 @@ use std::{
 
 use base_db::{
     salsa::{self, ParallelDatabase},
-    SourceDatabaseExt, SourceRootId, Upcast,
+    CrateId, FileRange, SourceDatabaseExt, SourceRootId, Upcast,
 };
+use either::Either;
 use fst::{self, Streamer};
 use hir::{
-    db::HirDatabase,
-    symbols::{FileSymbol, SymbolCollector},
-    Crate, Module,
+    db::{DefDatabase, HirDatabase},
+    AdtId, AssocItemId, AssocItemLoc, DefHasSource, DefWithBodyId, HasSource, HirFileId, ImplId,
+    InFile, ItemContainerId, ItemLoc, ItemTreeNode, Lookup, MacroDef, Module, ModuleDefId,
+    ModuleId, Semantics, TraitId,
 };
 use rayon::prelude::*;
 use rustc_hash::FxHashSet;
+use syntax::{ast::HasName, AstNode, SmolStr, SyntaxNode, SyntaxNodePtr};
 
-use crate::RootDatabase;
+use crate::{RootDatabase, SymbolKind};
 
 #[derive(Debug)]
 pub struct Query {
@@ -93,7 +96,7 @@ impl Query {
 pub trait SymbolsDatabase: HirDatabase + SourceDatabaseExt + Upcast<dyn HirDatabase> {
     /// The symbol index for a given module. These modules should only be in source roots that
     /// are inside local_roots.
-    fn module_symbols(&self, module: Module) -> Arc<SymbolIndex>;
+    fn module_symbols(&self, module_id: ModuleId) -> Arc<SymbolIndex>;
 
     /// The symbol index for a given source root within library_roots.
     fn library_symbols(&self, source_root_id: SourceRootId) -> Arc<SymbolIndex>;
@@ -116,20 +119,20 @@ fn library_symbols(db: &dyn SymbolsDatabase, source_root_id: SourceRootId) -> Ar
     let symbols = db
         .source_root_crates(source_root_id)
         .iter()
-        .flat_map(|&krate| Crate::from(krate).modules(db.upcast()))
+        .flat_map(|&krate| module_ids_for_crate(db.upcast(), krate))
         // we specifically avoid calling SymbolsDatabase::module_symbols here, even they do the same thing,
         // as the index for a library is not going to really ever change, and we do not want to store each
         // module's index in salsa.
-        .map(|module| SymbolCollector::collect(db.upcast(), module))
+        .map(|module_id| SymbolCollector::collect(db, module_id))
         .flatten()
         .collect();
 
     Arc::new(SymbolIndex::new(symbols))
 }
 
-fn module_symbols(db: &dyn SymbolsDatabase, module: Module) -> Arc<SymbolIndex> {
+fn module_symbols(db: &dyn SymbolsDatabase, module_id: ModuleId) -> Arc<SymbolIndex> {
     let _p = profile::span("module_symbols");
-    let symbols = SymbolCollector::collect(db.upcast(), module);
+    let symbols = SymbolCollector::collect(db, module_id);
     Arc::new(SymbolIndex::new(symbols))
 }
 
@@ -188,36 +191,41 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec<FileSymbol> {
             .map_with(Snap::new(db), |snap, &root| snap.library_symbols(root))
             .collect()
     } else {
-        let mut modules = Vec::new();
+        let mut module_ids = Vec::new();
 
         for &root in db.local_roots().iter() {
             let crates = db.source_root_crates(root);
             for &krate in crates.iter() {
-                modules.extend(Crate::from(krate).modules(db));
+                module_ids.extend(module_ids_for_crate(db, krate));
             }
         }
 
-        modules
+        module_ids
             .par_iter()
-            .map_with(Snap::new(db), |snap, &module| snap.module_symbols(module))
+            .map_with(Snap::new(db), |snap, &module_id| snap.module_symbols(module_id))
             .collect()
     };
 
     query.search(&indices)
 }
 
-pub fn crate_symbols(db: &RootDatabase, krate: Crate, query: Query) -> Vec<FileSymbol> {
+pub fn crate_symbols(db: &RootDatabase, krate: CrateId, query: Query) -> Vec<FileSymbol> {
     let _p = profile::span("crate_symbols").detail(|| format!("{:?}", query));
 
-    let modules = krate.modules(db);
-    let indices: Vec<_> = modules
+    let module_ids = module_ids_for_crate(db, krate);
+    let indices: Vec<_> = module_ids
         .par_iter()
-        .map_with(Snap::new(db), |snap, &module| snap.module_symbols(module))
+        .map_with(Snap::new(db), |snap, &module_id| snap.module_symbols(module_id))
         .collect();
 
     query.search(&indices)
 }
 
+fn module_ids_for_crate(db: &dyn DefDatabase, krate: CrateId) -> Vec<ModuleId> {
+    let def_map = db.crate_def_map(krate);
+    def_map.modules().map(|(id, _)| def_map.module_id(id)).collect()
+}
+
 pub fn index_resolve(db: &RootDatabase, name: &str) -> Vec<FileSymbol> {
     let mut query = Query::new(name.to_string());
     query.exact();
@@ -348,12 +356,374 @@ impl Query {
     }
 }
 
+/// The actual data that is stored in the index. It should be as compact as
+/// possible.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct FileSymbol {
+    pub name: SmolStr,
+    pub loc: DeclarationLocation,
+    pub kind: FileSymbolKind,
+    pub container_name: Option<SmolStr>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct DeclarationLocation {
+    /// The file id for both the `ptr` and `name_ptr`.
+    pub hir_file_id: HirFileId,
+    /// 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: SyntaxNodePtr,
+}
+
+impl DeclarationLocation {
+    pub fn syntax(&self, semantics: &Semantics<'_, RootDatabase>) -> Option<SyntaxNode> {
+        let root = semantics.parse_or_expand(self.hir_file_id)?;
+        Some(self.ptr.to_node(&root))
+    }
+
+    pub fn original_range(&self, db: &dyn HirDatabase) -> Option<FileRange> {
+        find_original_file_range(db, self.hir_file_id, &self.ptr)
+    }
+
+    pub fn original_name_range(&self, db: &dyn HirDatabase) -> Option<FileRange> {
+        find_original_file_range(db, self.hir_file_id, &self.name_ptr)
+    }
+}
+
+fn find_original_file_range(
+    db: &dyn HirDatabase,
+    file_id: HirFileId,
+    ptr: &SyntaxNodePtr,
+) -> Option<FileRange> {
+    let root = db.parse_or_expand(file_id)?;
+    let node = ptr.to_node(&root);
+    let node = InFile::new(file_id, &node);
+
+    Some(node.original_file_range(db.upcast()))
+}
+
+#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
+pub enum FileSymbolKind {
+    Const,
+    Enum,
+    Function,
+    Macro,
+    Module,
+    Static,
+    Struct,
+    Trait,
+    TypeAlias,
+    Union,
+}
+
+impl FileSymbolKind {
+    fn is_type(self: FileSymbolKind) -> bool {
+        matches!(
+            self,
+            FileSymbolKind::Struct
+                | FileSymbolKind::Enum
+                | FileSymbolKind::Trait
+                | FileSymbolKind::TypeAlias
+                | FileSymbolKind::Union
+        )
+    }
+}
+
+impl From<FileSymbolKind> for SymbolKind {
+    fn from(it: FileSymbolKind) -> Self {
+        match it {
+            FileSymbolKind::Const => SymbolKind::Const,
+            FileSymbolKind::Enum => SymbolKind::Enum,
+            FileSymbolKind::Function => SymbolKind::Function,
+            FileSymbolKind::Macro => SymbolKind::Macro,
+            FileSymbolKind::Module => SymbolKind::Module,
+            FileSymbolKind::Static => SymbolKind::Static,
+            FileSymbolKind::Struct => SymbolKind::Struct,
+            FileSymbolKind::Trait => SymbolKind::Trait,
+            FileSymbolKind::TypeAlias => SymbolKind::TypeAlias,
+            FileSymbolKind::Union => SymbolKind::Union,
+        }
+    }
+}
+
+/// Represents an outstanding module that the symbol collector must collect symbols from.
+struct SymbolCollectorWork {
+    module_id: ModuleId,
+    parent: Option<DefWithBodyId>,
+}
+
+struct SymbolCollector<'a> {
+    db: &'a dyn SymbolsDatabase,
+    symbols: Vec<FileSymbol>,
+    work: Vec<SymbolCollectorWork>,
+    current_container_name: Option<SmolStr>,
+}
+
+/// Given a [`ModuleId`] and a [`SymbolsDatabase`], use the DefMap for the module's crate to collect all symbols that should be
+/// indexed for the given module.
+impl<'a> SymbolCollector<'a> {
+    fn collect(db: &dyn SymbolsDatabase, module_id: ModuleId) -> Vec<FileSymbol> {
+        let mut symbol_collector = SymbolCollector {
+            db,
+            symbols: Default::default(),
+            current_container_name: None,
+            // The initial work is the root module we're collecting, additional work will
+            // be populated as we traverse the module's definitions.
+            work: vec![SymbolCollectorWork { module_id, parent: None }],
+        };
+
+        while let Some(work) = symbol_collector.work.pop() {
+            symbol_collector.do_work(work);
+        }
+
+        symbol_collector.symbols
+    }
+
+    fn do_work(&mut self, work: SymbolCollectorWork) {
+        self.db.unwind_if_cancelled();
+
+        let parent_name = work.parent.and_then(|id| self.def_with_body_id_name(id));
+        self.with_container_name(parent_name, |s| s.collect_from_module(work.module_id));
+    }
+
+    fn collect_from_module(&mut self, module_id: ModuleId) {
+        let def_map = module_id.def_map(self.db.upcast());
+        let scope = &def_map[module_id.local_id].scope;
+
+        for module_def_id in scope.declarations() {
+            match module_def_id {
+                ModuleDefId::ModuleId(id) => self.push_module(id),
+                ModuleDefId::FunctionId(id) => {
+                    self.push_decl_assoc(id, FileSymbolKind::Function);
+                    self.collect_from_body(id);
+                }
+                ModuleDefId::AdtId(AdtId::StructId(id)) => {
+                    self.push_decl(id, FileSymbolKind::Struct)
+                }
+                ModuleDefId::AdtId(AdtId::EnumId(id)) => self.push_decl(id, FileSymbolKind::Enum),
+                ModuleDefId::AdtId(AdtId::UnionId(id)) => self.push_decl(id, FileSymbolKind::Union),
+                ModuleDefId::ConstId(id) => {
+                    self.push_decl_assoc(id, FileSymbolKind::Const);
+                    self.collect_from_body(id);
+                }
+                ModuleDefId::StaticId(id) => {
+                    self.push_decl_assoc(id, FileSymbolKind::Static);
+                    self.collect_from_body(id);
+                }
+                ModuleDefId::TraitId(id) => {
+                    self.push_decl(id, FileSymbolKind::Trait);
+                    self.collect_from_trait(id);
+                }
+                ModuleDefId::TypeAliasId(id) => {
+                    self.push_decl_assoc(id, FileSymbolKind::TypeAlias);
+                }
+                // Don't index these.
+                ModuleDefId::BuiltinType(_) => {}
+                ModuleDefId::EnumVariantId(_) => {}
+            }
+        }
+
+        for impl_id in scope.impls() {
+            self.collect_from_impl(impl_id);
+        }
+
+        for const_id in scope.unnamed_consts() {
+            self.collect_from_body(const_id);
+        }
+
+        for macro_def_id in scope.macro_declarations() {
+            self.push_decl_macro(macro_def_id.into());
+        }
+    }
+
+    fn collect_from_body(&mut self, body_id: impl Into<DefWithBodyId>) {
+        let body_id = body_id.into();
+        let body = self.db.body(body_id);
+
+        // Descend into the blocks and enqueue collection of all modules within.
+        for (_, def_map) in body.blocks(self.db.upcast()) {
+            for (id, _) in def_map.modules() {
+                self.work.push(SymbolCollectorWork {
+                    module_id: def_map.module_id(id),
+                    parent: Some(body_id),
+                });
+            }
+        }
+    }
+
+    fn collect_from_impl(&mut self, impl_id: ImplId) {
+        let impl_data = self.db.impl_data(impl_id);
+        for &assoc_item_id in &impl_data.items {
+            self.push_assoc_item(assoc_item_id)
+        }
+    }
+
+    fn collect_from_trait(&mut self, trait_id: TraitId) {
+        let trait_data = self.db.trait_data(trait_id);
+        self.with_container_name(trait_data.name.as_text(), |s| {
+            for &(_, assoc_item_id) in &trait_data.items {
+                s.push_assoc_item(assoc_item_id);
+            }
+        });
+    }
+
+    fn with_container_name(&mut self, container_name: Option<SmolStr>, f: impl FnOnce(&mut Self)) {
+        if let Some(container_name) = container_name {
+            let prev = self.current_container_name.replace(container_name);
+            f(self);
+            self.current_container_name = prev;
+        } else {
+            f(self);
+        }
+    }
+
+    fn current_container_name(&self) -> Option<SmolStr> {
+        self.current_container_name.clone()
+    }
+
+    fn def_with_body_id_name(&self, body_id: DefWithBodyId) -> Option<SmolStr> {
+        match body_id {
+            DefWithBodyId::FunctionId(id) => Some(
+                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
+            ),
+            DefWithBodyId::StaticId(id) => Some(
+                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
+            ),
+            DefWithBodyId::ConstId(id) => Some(
+                id.lookup(self.db.upcast()).source(self.db.upcast()).value.name()?.text().into(),
+            ),
+        }
+    }
+
+    fn push_assoc_item(&mut self, assoc_item_id: AssocItemId) {
+        match assoc_item_id {
+            AssocItemId::FunctionId(id) => self.push_decl_assoc(id, FileSymbolKind::Function),
+            AssocItemId::ConstId(id) => self.push_decl_assoc(id, FileSymbolKind::Const),
+            AssocItemId::TypeAliasId(id) => self.push_decl_assoc(id, FileSymbolKind::TypeAlias),
+        }
+    }
+
+    fn push_decl_assoc<L, T>(&mut self, id: L, kind: FileSymbolKind)
+    where
+        L: Lookup<Data = AssocItemLoc<T>>,
+        T: ItemTreeNode,
+        <T as ItemTreeNode>::Source: HasName,
+    {
+        fn container_name(db: &dyn HirDatabase, container: ItemContainerId) -> Option<SmolStr> {
+            match container {
+                ItemContainerId::ModuleId(module_id) => {
+                    let module = Module::from(module_id);
+                    module.name(db).and_then(|name| name.as_text())
+                }
+                ItemContainerId::TraitId(trait_id) => {
+                    let trait_data = db.trait_data(trait_id);
+                    trait_data.name.as_text()
+                }
+                ItemContainerId::ImplId(_) | ItemContainerId::ExternBlockId(_) => None,
+            }
+        }
+
+        self.push_file_symbol(|s| {
+            let loc = id.lookup(s.db.upcast());
+            let source = loc.source(s.db.upcast());
+            let name_node = source.value.name()?;
+            let container_name =
+                container_name(s.db.upcast(), loc.container).or_else(|| s.current_container_name());
+
+            Some(FileSymbol {
+                name: name_node.text().into(),
+                kind,
+                container_name,
+                loc: DeclarationLocation {
+                    hir_file_id: source.file_id,
+                    ptr: SyntaxNodePtr::new(source.value.syntax()),
+                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
+                },
+            })
+        })
+    }
+
+    fn push_decl<L, T>(&mut self, id: L, kind: FileSymbolKind)
+    where
+        L: Lookup<Data = ItemLoc<T>>,
+        T: ItemTreeNode,
+        <T as ItemTreeNode>::Source: HasName,
+    {
+        self.push_file_symbol(|s| {
+            let loc = id.lookup(s.db.upcast());
+            let source = loc.source(s.db.upcast());
+            let name_node = source.value.name()?;
+
+            Some(FileSymbol {
+                name: name_node.text().into(),
+                kind,
+                container_name: s.current_container_name(),
+                loc: DeclarationLocation {
+                    hir_file_id: source.file_id,
+                    ptr: SyntaxNodePtr::new(source.value.syntax()),
+                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
+                },
+            })
+        })
+    }
+
+    fn push_module(&mut self, module_id: ModuleId) {
+        self.push_file_symbol(|s| {
+            let def_map = module_id.def_map(s.db.upcast());
+            let module_data = &def_map[module_id.local_id];
+            let declaration = module_data.origin.declaration()?;
+            let module = declaration.to_node(s.db.upcast());
+            let name_node = module.name()?;
+
+            Some(FileSymbol {
+                name: name_node.text().into(),
+                kind: FileSymbolKind::Module,
+                container_name: s.current_container_name(),
+                loc: DeclarationLocation {
+                    hir_file_id: declaration.file_id,
+                    ptr: SyntaxNodePtr::new(module.syntax()),
+                    name_ptr: SyntaxNodePtr::new(name_node.syntax()),
+                },
+            })
+        })
+    }
+
+    fn push_decl_macro(&mut self, macro_def: MacroDef) {
+        self.push_file_symbol(|s| {
+            let name = macro_def.name(s.db.upcast())?.as_text()?;
+            let source = macro_def.source(s.db.upcast())?;
+
+            let (ptr, name_ptr) = match source.value {
+                Either::Left(m) => {
+                    (SyntaxNodePtr::new(m.syntax()), SyntaxNodePtr::new(m.name()?.syntax()))
+                }
+                Either::Right(f) => {
+                    (SyntaxNodePtr::new(f.syntax()), SyntaxNodePtr::new(f.name()?.syntax()))
+                }
+            };
+
+            Some(FileSymbol {
+                name,
+                kind: FileSymbolKind::Macro,
+                container_name: s.current_container_name(),
+                loc: DeclarationLocation { hir_file_id: source.file_id, name_ptr, ptr },
+            })
+        })
+    }
+
+    fn push_file_symbol(&mut self, f: impl FnOnce(&Self) -> Option<FileSymbol>) {
+        if let Some(file_symbol) = f(self) {
+            self.symbols.push(file_symbol);
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
 
     use base_db::fixture::WithFixture;
     use expect_test::expect_file;
-    use hir::symbols::SymbolCollector;
 
     use super::*;
 
@@ -422,10 +792,11 @@ struct StructInModB;
         "#,
         );
 
-        let symbols: Vec<_> = Crate::from(db.test_crate())
-            .modules(&db)
+        let symbols: Vec<_> = module_ids_for_crate(db.upcast(), db.test_crate())
             .into_iter()
-            .map(|module_id| (module_id, SymbolCollector::collect(&db, module_id)))
+            .map(|module_id| {
+                (module_id, SymbolCollector::collect(&db as &dyn SymbolsDatabase, module_id))
+            })
             .collect();
 
         expect_file!["./test_data/test_symbol_index_collection.txt"].assert_debug_eq(&symbols);


Test patch:
diff --git a/crates/ide_completion/src/tests/attribute.rs b/crates/ide_completion/src/tests/attribute.rs
index 2d2a1b867a..c2bd26e2d8 100644
--- a/crates/ide_completion/src/tests/attribute.rs
+++ b/crates/ide_completion/src/tests/attribute.rs
@@ -40,19 +40,6 @@ struct Foo;
     )
 }
 
-#[test]
-fn proc_macros_on_comment() {
-    check(
-        r#"
-//- proc_macros: identity
-/// $0
-#[proc_macros::identity]
-struct Foo;
-"#,
-        expect![[r#""#]],
-    )
-}
-
 #[test]
 fn proc_macros_qualified() {
     check(
diff --git a/crates/ide_completion/src/tests/pattern.rs b/crates/ide_completion/src/tests/pattern.rs
index 7f437e1a07..c9a31eea84 100644
--- a/crates/ide_completion/src/tests/pattern.rs
+++ b/crates/ide_completion/src/tests/pattern.rs
@@ -368,17 +368,3 @@ fn foo() {
         "#]],
     )
 }
-
-#[test]
-fn completes_fully_equal() {
-    check_empty(
-        r#"
-fn foo(bar: u32) {}
-fn bar(bar$0) {}
-"#,
-        expect![[r#"
-            bn bar: u32
-            kw mut
-        "#]],
-    )
-}
diff --git a/crates/ide_db/src/test_data/test_symbol_index_collection.txt b/crates/ide_db/src/test_data/test_symbol_index_collection.txt
index cc51d85da7..176d7e1579 100644
--- a/crates/ide_db/src/test_data/test_symbol_index_collection.txt
+++ b/crates/ide_db/src/test_data/test_symbol_index_collection.txt
@@ -1,13 +1,11 @@
 [
     (
-        Module {
-            id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
-                block: None,
-                local_id: Idx::<ModuleData>(0),
-            },
+        ModuleId {
+            krate: CrateId(
+                0,
+            ),
+            block: None,
+            local_id: Idx::<ModuleData>(0),
         },
         [
             FileSymbol {
@@ -461,14 +459,12 @@
         ],
     ),
     (
-        Module {
-            id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
-                block: None,
-                local_id: Idx::<ModuleData>(1),
-            },
+        ModuleId {
+            krate: CrateId(
+                0,
+            ),
+            block: None,
+            local_id: Idx::<ModuleData>(1),
         },
         [
             FileSymbol {
@@ -496,14 +492,12 @@
         ],
     ),
     (
-        Module {
-            id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
-                block: None,
-                local_id: Idx::<ModuleData>(2),
-            },
+        ModuleId {
+            krate: CrateId(
+                0,
+            ),
+            block: None,
+            local_id: Idx::<ModuleData>(2),
         },
         [
             FileSymbol {