Merge #1057

1057: simplify r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_hir/src/db.rs

@@ -34,10 +34,10 @@ pub trait DefDatabase: SourceDatabase + AsRef<HirInterner> {
     #[salsa::invoke(crate::traits::TraitData::trait_data_query)]
     fn trait_data(&self, t: Trait) -> Arc<TraitData>;
 
-    #[salsa::invoke(crate::ids::SourceFileItems::file_items_query)]
+    #[salsa::invoke(crate::source_id::SourceFileItems::file_items_query)]
     fn file_items(&self, file_id: HirFileId) -> Arc<SourceFileItems>;
 
-    #[salsa::invoke(crate::ids::SourceFileItems::file_item_query)]
+    #[salsa::invoke(crate::source_id::SourceFileItems::file_item_query)]
     fn file_item(&self, source_item_id: SourceItemId) -> TreeArc<SyntaxNode>;
 
     #[salsa::invoke(RawItems::raw_items_query)]

crates/ra_hir/src/ids.rs

@@ -5,13 +5,12 @@ use std::{
 };
 
 use ra_db::{LocationInterner, FileId};
-use ra_syntax::{TreeArc, SyntaxNode, SourceFile, AstNode, SyntaxNodePtr, ast};
-use ra_arena::{Arena, RawId, ArenaId, impl_arena_id};
+use ra_syntax::{TreeArc, SourceFile, AstNode, ast};
+use ra_arena::{RawId, ArenaId, impl_arena_id};
 use mbe::MacroRules;
 
 use crate::{
-    Module,
-    DefDatabase,
+    Module, DefDatabase, SourceItemId, SourceFileItemId,
 };
 
 #[derive(Debug, Default)]
@@ -304,110 +303,3 @@ impl AstItemDef<ast::TypeAliasDef> for TypeId {
         &interner.types
     }
 }
-
-/// Identifier of item within a specific file. This is stable over reparses, so
-/// it's OK to use it as a salsa key/value.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct SourceFileItemId(RawId);
-impl_arena_id!(SourceFileItemId);
-
-impl SourceFileItemId {
-    pub(crate) fn with_file_id(self, file_id: HirFileId) -> SourceItemId {
-        SourceItemId { file_id, item_id: self }
-    }
-}
-
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct SourceItemId {
-    pub(crate) file_id: HirFileId,
-    pub(crate) item_id: SourceFileItemId,
-}
-
-/// Maps items' `SyntaxNode`s to `SourceFileItemId`s and back.
-#[derive(Debug, PartialEq, Eq)]
-pub struct SourceFileItems {
-    file_id: HirFileId,
-    arena: Arena<SourceFileItemId, SyntaxNodePtr>,
-}
-
-impl SourceFileItems {
-    pub(crate) fn file_items_query(
-        db: &impl DefDatabase,
-        file_id: HirFileId,
-    ) -> Arc<SourceFileItems> {
-        let source_file = db.hir_parse(file_id);
-        Arc::new(SourceFileItems::from_source_file(&source_file, file_id))
-    }
-
-    pub(crate) fn file_item_query(
-        db: &impl DefDatabase,
-        source_item_id: SourceItemId,
-    ) -> TreeArc<SyntaxNode> {
-        let source_file = db.hir_parse(source_item_id.file_id);
-        db.file_items(source_item_id.file_id)[source_item_id.item_id]
-            .to_node(&source_file)
-            .to_owned()
-    }
-
-    pub(crate) fn from_source_file(
-        source_file: &SourceFile,
-        file_id: HirFileId,
-    ) -> SourceFileItems {
-        let mut res = SourceFileItems { file_id, arena: Arena::default() };
-        // By walking the tree in bread-first order we make sure that parents
-        // get lower ids then children. That is, adding a new child does not
-        // change parent's id. This means that, say, adding a new function to a
-        // trait does not change ids of top-level items, which helps caching.
-        bfs(source_file.syntax(), |it| {
-            if let Some(module_item) = ast::ModuleItem::cast(it) {
-                res.alloc(module_item.syntax());
-            } else if let Some(macro_call) = ast::MacroCall::cast(it) {
-                res.alloc(macro_call.syntax());
-            }
-        });
-        res
-    }
-
-    fn alloc(&mut self, item: &SyntaxNode) -> SourceFileItemId {
-        self.arena.alloc(SyntaxNodePtr::new(item))
-    }
-    pub(crate) fn id_of(&self, file_id: HirFileId, item: &SyntaxNode) -> SourceFileItemId {
-        assert_eq!(
-            self.file_id, file_id,
-            "SourceFileItems: wrong file, expected {:?}, got {:?}",
-            self.file_id, file_id
-        );
-        self.id_of_unchecked(item)
-    }
-    pub(crate) fn id_of_unchecked(&self, item: &SyntaxNode) -> SourceFileItemId {
-        let ptr = SyntaxNodePtr::new(item);
-        if let Some((id, _)) = self.arena.iter().find(|(_id, i)| **i == ptr) {
-            return id;
-        }
-        panic!(
-            "Can't find {:?} in SourceFileItems:\n{:?}",
-            item,
-            self.arena.iter().map(|(_id, i)| i).collect::<Vec<_>>(),
-        );
-    }
-}
-
-impl std::ops::Index<SourceFileItemId> for SourceFileItems {
-    type Output = SyntaxNodePtr;
-    fn index(&self, idx: SourceFileItemId) -> &SyntaxNodePtr {
-        &self.arena[idx]
-    }
-}
-
-/// Walks the subtree in bfs order, calling `f` for each node.
-fn bfs(node: &SyntaxNode, mut f: impl FnMut(&SyntaxNode)) {
-    let mut curr_layer = vec![node];
-    let mut next_layer = vec![];
-    while !curr_layer.is_empty() {
-        curr_layer.drain(..).for_each(|node| {
-            next_layer.extend(node.children());
-            f(node);
-        });
-        std::mem::swap(&mut curr_layer, &mut next_layer);
-    }
-}

crates/ra_hir/src/lib.rs

@@ -23,6 +23,7 @@ pub mod mock;
 mod path;
 pub mod source_binder;
 
+mod source_id;
 mod ids;
 mod name;
 mod nameres;
@@ -47,12 +48,13 @@ mod marks;
 use crate::{
     db::{HirDatabase, DefDatabase},
     name::{AsName, KnownName},
-    ids::{SourceItemId, SourceFileItems},
+    source_id::SourceFileItemId,
 };
 
 pub use self::{
     path::{Path, PathKind},
     name::Name,
+    source_id::{SourceFileItems, SourceItemId},
     ids::{HirFileId, MacroDefId, MacroCallId, MacroCallLoc, HirInterner},
     nameres::{PerNs, Namespace},
     ty::{Ty, ApplicationTy, TypeCtor, Substs, display::HirDisplay},

crates/ra_hir/src/nameres.rs

@@ -61,9 +61,9 @@ use ra_db::{FileId, Edition};
 use test_utils::tested_by;
 
 use crate::{
-    ModuleDef, Name, Crate, Module,
+    ModuleDef, Name, Crate, Module, SourceItemId,
     DefDatabase, Path, PathKind, HirFileId, Trait,
-    ids::{SourceItemId, SourceFileItemId, MacroCallId, MacroDefId},
+    ids::{MacroCallId, MacroDefId},
     diagnostics::DiagnosticSink,
     nameres::diagnostics::DefDiagnostic,
 };
@@ -241,9 +241,8 @@ impl CrateDefMap {
     pub(crate) fn find_module_by_source(
         &self,
         file_id: HirFileId,
-        decl_id: Option<SourceFileItemId>,
+        decl_id: Option<SourceItemId>,
     ) -> Option<CrateModuleId> {
-        let decl_id = decl_id.map(|it| it.with_file_id(file_id));
         let (module_id, _module_data) = self.modules.iter().find(|(_module_id, module_data)| {
             if decl_id.is_some() {
                 module_data.declaration == decl_id

crates/ra_hir/src/nameres/collector.rs

@@ -6,7 +6,7 @@ use ra_db::FileId;
 
 use crate::{
     Function, Module, Struct, Enum, Const, Static, Trait, TypeAlias,
-    DefDatabase, HirFileId, Name, Path,
+    DefDatabase, HirFileId, Name, Path, SourceItemId,
     KnownName,
     nameres::{
         Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode,
@@ -14,7 +14,7 @@ use crate::{
         diagnostics::DefDiagnostic,
         raw,
     },
-    ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId, MacroDefId},
+    ids::{AstItemDef, LocationCtx, MacroCallLoc, MacroCallId, MacroDefId},
 };
 
 pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> CrateDefMap {

crates/ra_hir/src/nameres/raw.rs

@@ -12,7 +12,7 @@ use ra_syntax::{
 
 use crate::{
     DefDatabase, Name, AsName, Path, HirFileId, ModuleSource,
-    ids::{SourceFileItemId, SourceFileItems},
+    SourceFileItemId, SourceFileItems,
 };
 
 /// `RawItems` is a set of top-level items in a file (except for impls).

crates/ra_hir/src/source_binder.rs

@@ -13,9 +13,9 @@ use ra_syntax::{
 };
 
 use crate::{
-    HirDatabase, Function, Struct, Enum,
+    HirDatabase, Function, Struct, Enum, SourceItemId,
     AsName, Module, HirFileId, Crate, Trait, Resolver,
-    ids::{LocationCtx, SourceFileItemId},
+    ids::LocationCtx,
     expr
 };
 
@@ -55,7 +55,7 @@ fn module_from_inline(
     assert!(!module.has_semi());
     let file_id = file_id.into();
     let file_items = db.file_items(file_id);
-    let item_id = file_items.id_of(file_id, module.syntax());
+    let item_id = file_items.id_of(file_id, module.syntax()).with_file_id(file_id);
     module_from_source(db, file_id, Some(item_id))
 }
 
@@ -75,7 +75,7 @@ pub fn module_from_child_node(
 fn module_from_source(
     db: &impl HirDatabase,
     file_id: HirFileId,
-    decl_id: Option<SourceFileItemId>,
+    decl_id: Option<SourceItemId>,
 ) -> Option<Module> {
     let source_root_id = db.file_source_root(file_id.as_original_file());
     db.source_root_crates(source_root_id).iter().map(|&crate_id| Crate { crate_id }).find_map(

crates/ra_hir/src/source_id.rs

@@ -0,0 +1,113 @@
+use std::sync::Arc;
+
+use ra_arena::{Arena, RawId, impl_arena_id};
+use ra_syntax::{SyntaxNodePtr, TreeArc, SyntaxNode, SourceFile, AstNode, ast};
+
+use crate::{HirFileId, DefDatabase};
+
+/// Identifier of item within a specific file. This is stable over reparses, so
+/// it's OK to use it as a salsa key/value.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub(crate) struct SourceFileItemId(RawId);
+impl_arena_id!(SourceFileItemId);
+
+impl SourceFileItemId {
+    pub(crate) fn with_file_id(self, file_id: HirFileId) -> SourceItemId {
+        SourceItemId { file_id, item_id: self }
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct SourceItemId {
+    pub(crate) file_id: HirFileId,
+    pub(crate) item_id: SourceFileItemId,
+}
+
+/// Maps items' `SyntaxNode`s to `SourceFileItemId`s and back.
+#[derive(Debug, PartialEq, Eq)]
+pub struct SourceFileItems {
+    file_id: HirFileId,
+    arena: Arena<SourceFileItemId, SyntaxNodePtr>,
+}
+
+impl SourceFileItems {
+    pub(crate) fn file_items_query(
+        db: &impl DefDatabase,
+        file_id: HirFileId,
+    ) -> Arc<SourceFileItems> {
+        let source_file = db.hir_parse(file_id);
+        Arc::new(SourceFileItems::from_source_file(&source_file, file_id))
+    }
+
+    pub(crate) fn file_item_query(
+        db: &impl DefDatabase,
+        source_item_id: SourceItemId,
+    ) -> TreeArc<SyntaxNode> {
+        let source_file = db.hir_parse(source_item_id.file_id);
+        db.file_items(source_item_id.file_id)[source_item_id.item_id]
+            .to_node(&source_file)
+            .to_owned()
+    }
+
+    pub(crate) fn from_source_file(
+        source_file: &SourceFile,
+        file_id: HirFileId,
+    ) -> SourceFileItems {
+        let mut res = SourceFileItems { file_id, arena: Arena::default() };
+        // By walking the tree in bread-first order we make sure that parents
+        // get lower ids then children. That is, adding a new child does not
+        // change parent's id. This means that, say, adding a new function to a
+        // trait does not change ids of top-level items, which helps caching.
+        bfs(source_file.syntax(), |it| {
+            if let Some(module_item) = ast::ModuleItem::cast(it) {
+                res.alloc(module_item.syntax());
+            } else if let Some(macro_call) = ast::MacroCall::cast(it) {
+                res.alloc(macro_call.syntax());
+            }
+        });
+        res
+    }
+
+    fn alloc(&mut self, item: &SyntaxNode) -> SourceFileItemId {
+        self.arena.alloc(SyntaxNodePtr::new(item))
+    }
+    pub(crate) fn id_of(&self, file_id: HirFileId, item: &SyntaxNode) -> SourceFileItemId {
+        assert_eq!(
+            self.file_id, file_id,
+            "SourceFileItems: wrong file, expected {:?}, got {:?}",
+            self.file_id, file_id
+        );
+        self.id_of_unchecked(item)
+    }
+    pub(crate) fn id_of_unchecked(&self, item: &SyntaxNode) -> SourceFileItemId {
+        let ptr = SyntaxNodePtr::new(item);
+        if let Some((id, _)) = self.arena.iter().find(|(_id, i)| **i == ptr) {
+            return id;
+        }
+        panic!(
+            "Can't find {:?} in SourceFileItems:\n{:?}",
+            item,
+            self.arena.iter().map(|(_id, i)| i).collect::<Vec<_>>(),
+        );
+    }
+}
+
+impl std::ops::Index<SourceFileItemId> for SourceFileItems {
+    type Output = SyntaxNodePtr;
+    fn index(&self, idx: SourceFileItemId) -> &SyntaxNodePtr {
+        &self.arena[idx]
+    }
+}
+
+/// Walks the subtree in bfs order, calling `f` for each node.
+fn bfs(node: &SyntaxNode, mut f: impl FnMut(&SyntaxNode)) {
+    let mut curr_layer = vec![node];
+    let mut next_layer = vec![];
+    while !curr_layer.is_empty() {
+        curr_layer.drain(..).for_each(|node| {
+            next_layer.extend(node.children());
+            f(node);
+        });
+        std::mem::swap(&mut curr_layer, &mut next_layer);
+    }
+}