Merge #1059

1059: Typed ids r=matklad a=matklad

just some type-safety and refactorings. 

closes https://github.com/rust-analyzer/rust-analyzer/issues/1054

bors r+

Co-authored-by: Aleksey Kladov <[email protected]>

1 files changed, 97 insertions, 60 deletions

diff --git a/crates/ra_hir/src/source_id.rs b/crates/ra_hir/src/source_id.rs
index 62707ba6a..0a8fb6d32 100644
--- a/crates/ra_hir/src/source_id.rs
+++ b/crates/ra_hir/src/source_id.rs
@@ -1,59 +1,122 @@
-use std::sync::Arc;
+use std::{marker::PhantomData, sync::Arc, hash::{Hash, Hasher}};
 
 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);
+/// `AstId` points to an AST node in any file.
+///
+/// It is stable across reparses, and can be used as salsa key/value.
+#[derive(Debug)]
+pub(crate) struct AstId<N: AstNode> {
+    file_id: HirFileId,
+    file_ast_id: FileAstId<N>,
+}
+
+impl<N: AstNode> Clone for AstId<N> {
+    fn clone(&self) -> AstId<N> {
+        *self
+    }
+}
+impl<N: AstNode> Copy for AstId<N> {}
+
+impl<N: AstNode> PartialEq for AstId<N> {
+    fn eq(&self, other: &Self) -> bool {
+        (self.file_id, self.file_ast_id) == (other.file_id, other.file_ast_id)
+    }
+}
+impl<N: AstNode> Eq for AstId<N> {}
+impl<N: AstNode> Hash for AstId<N> {
+    fn hash<H: Hasher>(&self, hasher: &mut H) {
+        (self.file_id, self.file_ast_id).hash(hasher);
+    }
+}
+
+impl<N: AstNode> AstId<N> {
+    pub(crate) fn file_id(&self) -> HirFileId {
+        self.file_id
+    }
+
+    pub(crate) fn to_node(&self, db: &impl DefDatabase) -> TreeArc<N> {
+        let syntax_node = db.ast_id_to_node(self.file_id, self.file_ast_id.raw);
+        N::cast(&syntax_node).unwrap().to_owned()
+    }
+}
+
+/// `AstId` points to an AST node in a specific file.
+#[derive(Debug)]
+pub(crate) struct FileAstId<N: AstNode> {
+    raw: ErasedFileAstId,
+    _ty: PhantomData<N>,
+}
+
+impl<N: AstNode> Clone for FileAstId<N> {
+    fn clone(&self) -> FileAstId<N> {
+        *self
+    }
+}
+impl<N: AstNode> Copy for FileAstId<N> {}
 
-impl SourceFileItemId {
-    pub(crate) fn with_file_id(self, file_id: HirFileId) -> SourceItemId {
-        SourceItemId { file_id, item_id: self }
+impl<N: AstNode> PartialEq for FileAstId<N> {
+    fn eq(&self, other: &Self) -> bool {
+        self.raw == other.raw
+    }
+}
+impl<N: AstNode> Eq for FileAstId<N> {}
+impl<N: AstNode> Hash for FileAstId<N> {
+    fn hash<H: Hasher>(&self, hasher: &mut H) {
+        self.raw.hash(hasher);
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct SourceItemId {
-    pub(crate) file_id: HirFileId,
-    pub(crate) item_id: SourceFileItemId,
+impl<N: AstNode> FileAstId<N> {
+    pub(crate) fn with_file_id(self, file_id: HirFileId) -> AstId<N> {
+        AstId { file_id, file_ast_id: self }
+    }
 }
 
-/// Maps items' `SyntaxNode`s to `SourceFileItemId`s and back.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct ErasedFileAstId(RawId);
+impl_arena_id!(ErasedFileAstId);
+
+/// Maps items' `SyntaxNode`s to `ErasedFileAstId`s and back.
 #[derive(Debug, PartialEq, Eq)]
-pub struct SourceFileItems {
-    file_id: HirFileId,
-    arena: Arena<SourceFileItemId, SyntaxNodePtr>,
+pub struct AstIdMap {
+    arena: Arena<ErasedFileAstId, SyntaxNodePtr>,
 }
 
-impl SourceFileItems {
-    pub(crate) fn file_items_query(
-        db: &impl DefDatabase,
-        file_id: HirFileId,
-    ) -> Arc<SourceFileItems> {
+impl AstIdMap {
+    pub(crate) fn ast_id_map_query(db: &impl DefDatabase, file_id: HirFileId) -> Arc<AstIdMap> {
         let source_file = db.hir_parse(file_id);
-        Arc::new(SourceFileItems::from_source_file(&source_file, file_id))
+        Arc::new(AstIdMap::from_source_file(&source_file))
     }
 
     pub(crate) fn file_item_query(
         db: &impl DefDatabase,
-        source_item_id: SourceItemId,
+        file_id: HirFileId,
+        ast_id: ErasedFileAstId,
     ) -> 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()
+        let source_file = db.hir_parse(file_id);
+        db.ast_id_map(file_id).arena[ast_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() };
+    pub(crate) fn ast_id<N: AstNode>(&self, item: &N) -> FileAstId<N> {
+        let ptr = SyntaxNodePtr::new(item.syntax());
+        let raw = match self.arena.iter().find(|(_id, i)| **i == ptr) {
+            Some((it, _)) => it,
+            None => panic!(
+                "Can't find {:?} in AstIdMap:\n{:?}",
+                item.syntax(),
+                self.arena.iter().map(|(_id, i)| i).collect::<Vec<_>>(),
+            ),
+        };
+
+        FileAstId { raw, _ty: PhantomData }
+    }
+
+    fn from_source_file(source_file: &SourceFile) -> AstIdMap {
+        let mut res = AstIdMap { 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
@@ -68,35 +131,9 @@ impl SourceFileItems {
         res
     }
 
-    fn alloc(&mut self, item: &SyntaxNode) -> SourceFileItemId {
+    fn alloc(&mut self, item: &SyntaxNode) -> ErasedFileAstId {
         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.