more type safety

crates/ra_hir/src/ids.rs

@@ -1,5 +1,4 @@
 use std::{
-    marker::PhantomData,
     hash::{Hash, Hasher},
     sync::Arc,
 };
@@ -10,7 +9,7 @@ use ra_arena::{RawId, ArenaId, impl_arena_id};
 use mbe::MacroRules;
 
 use crate::{
-    Module, DefDatabase, SourceItemId, SourceFileItemId, AstId,
+    Module, DefDatabase, AstId, FileAstId,
 };
 
 #[derive(Debug, Default)]
@@ -123,6 +122,7 @@ impl From<MacroCallId> for HirFileId {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+
 pub struct MacroDefId(pub(crate) AstId<ast::MacroCall>);
 
 pub(crate) fn macro_def_query(db: &impl DefDatabase, id: MacroDefId) -> Option<Arc<MacroRules>> {
@@ -161,26 +161,25 @@ impl MacroCallLoc {
 #[derive(Debug)]
 pub struct ItemLoc<N: AstNode> {
     pub(crate) module: Module,
-    raw: SourceItemId,
+    ast_id: AstId<N>,
-    _ty: PhantomData<N>,
 }
 
 impl<N: AstNode> PartialEq for ItemLoc<N> {
     fn eq(&self, other: &Self) -> bool {
-        self.module == other.module && self.raw == other.raw
+        self.module == other.module && self.ast_id == other.ast_id
     }
 }
 impl<N: AstNode> Eq for ItemLoc<N> {}
 impl<N: AstNode> Hash for ItemLoc<N> {
     fn hash<H: Hasher>(&self, hasher: &mut H) {
         self.module.hash(hasher);
-        self.raw.hash(hasher);
+        self.ast_id.hash(hasher);
     }
 }
 
 impl<N: AstNode> Clone for ItemLoc<N> {
     fn clone(&self) -> ItemLoc<N> {
-        ItemLoc { module: self.module, raw: self.raw, _ty: PhantomData }
+        ItemLoc { module: self.module, ast_id: self.ast_id }
     }
 }
 
@@ -208,25 +207,18 @@ pub(crate) trait AstItemDef<N: AstNode>: ArenaId + Clone {
     fn interner(interner: &HirInterner) -> &LocationInterner<ItemLoc<N>, Self>;
     fn from_ast(ctx: LocationCtx<&impl DefDatabase>, ast: &N) -> Self {
         let items = ctx.db.file_items(ctx.file_id);
-        let item_id = items.id_of(ctx.file_id, ast.syntax());
+        let item_id = items.ast_id(ast);
-        Self::from_source_item_id_unchecked(ctx, item_id)
+        Self::from_ast_id(ctx, item_id)
     }
-    fn from_source_item_id_unchecked(
+    fn from_ast_id(ctx: LocationCtx<&impl DefDatabase>, ast_id: FileAstId<N>) -> Self {
-        ctx: LocationCtx<&impl DefDatabase>,
+        let loc = ItemLoc { module: ctx.module, ast_id: ast_id.with_file_id(ctx.file_id) };
-        item_id: SourceFileItemId,
-    ) -> Self {
-        let raw = SourceItemId { file_id: ctx.file_id, item_id };
-        let loc = ItemLoc { module: ctx.module, raw, _ty: PhantomData };
-
         Self::interner(ctx.db.as_ref()).loc2id(&loc)
     }
     fn source(self, db: &impl DefDatabase) -> (HirFileId, TreeArc<N>) {
         let int = Self::interner(db.as_ref());
         let loc = int.id2loc(self);
-        let syntax = db.file_item(loc.raw);
+        let ast = loc.ast_id.to_node(db);
-        let ast =
+        (loc.ast_id.file_id(), ast)
-            N::cast(&syntax).unwrap_or_else(|| panic!("invalid ItemLoc: {:?}", loc.raw)).to_owned();
-        (loc.raw.file_id, ast)
     }
     fn module(self, db: &impl DefDatabase) -> Module {
         let int = Self::interner(db.as_ref());

crates/ra_hir/src/lib.rs

@@ -48,7 +48,7 @@ mod marks;
 use crate::{
     db::{HirDatabase, DefDatabase},
     name::{AsName, KnownName},
-    source_id::{SourceFileItemId, FileAstId, AstId},
+    source_id::{FileAstId, AstId},
 };
 
 pub use self::{

crates/ra_hir/src/nameres/collector.rs

@@ -429,23 +429,24 @@ where
     fn define_def(&mut self, def: &raw::DefData) {
         let module = Module { krate: self.def_collector.def_map.krate, module_id: self.module_id };
         let ctx = LocationCtx::new(self.def_collector.db, module, self.file_id.into());
-        macro_rules! id {
+
-            () => {
+        macro_rules! def {
-                AstItemDef::from_source_item_id_unchecked(ctx, def.source_item_id)
+            ($kind:ident, $ast_id:ident) => {
+                $kind { id: AstItemDef::from_ast_id(ctx, $ast_id) }.into()
             };
         }
         let name = def.name.clone();
         let def: PerNs<ModuleDef> = match def.kind {
-            raw::DefKind::Function => PerNs::values(Function { id: id!() }.into()),
+            raw::DefKind::Function(ast_id) => PerNs::values(def!(Function, ast_id)),
-            raw::DefKind::Struct => {
+            raw::DefKind::Struct(ast_id) => {
-                let s = Struct { id: id!() }.into();
+                let s = def!(Struct, ast_id);
                 PerNs::both(s, s)
             }
-            raw::DefKind::Enum => PerNs::types(Enum { id: id!() }.into()),
+            raw::DefKind::Enum(ast_id) => PerNs::types(def!(Enum, ast_id)),
-            raw::DefKind::Const => PerNs::values(Const { id: id!() }.into()),
+            raw::DefKind::Const(ast_id) => PerNs::values(def!(Const, ast_id)),
-            raw::DefKind::Static => PerNs::values(Static { id: id!() }.into()),
+            raw::DefKind::Static(ast_id) => PerNs::values(def!(Static, ast_id)),
-            raw::DefKind::Trait => PerNs::types(Trait { id: id!() }.into()),
+            raw::DefKind::Trait(ast_id) => PerNs::types(def!(Trait, ast_id)),
-            raw::DefKind::TypeAlias => PerNs::types(TypeAlias { id: id!() }.into()),
+            raw::DefKind::TypeAlias(ast_id) => PerNs::types(def!(TypeAlias, ast_id)),
         };
         let resolution = Resolution { def, import: None };
         self.def_collector.update(self.module_id, None, &[(name, resolution)])

crates/ra_hir/src/nameres/raw.rs

@@ -12,7 +12,7 @@ use ra_syntax::{
 
 use crate::{
     DefDatabase, Name, AsName, Path, HirFileId, ModuleSource,
-    SourceFileItemId, SourceFileItems, FileAstId,
+    SourceFileItems, FileAstId,
 };
 
 /// `RawItems` is a set of top-level items in a file (except for impls).
@@ -138,20 +138,19 @@ impl_arena_id!(Def);
 
 #[derive(Debug, PartialEq, Eq)]
 pub(super) struct DefData {
-    pub(super) source_item_id: SourceFileItemId,
     pub(super) name: Name,
     pub(super) kind: DefKind,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub(super) enum DefKind {
-    Function,
+    Function(FileAstId<ast::FnDef>),
-    Struct,
+    Struct(FileAstId<ast::StructDef>),
-    Enum,
+    Enum(FileAstId<ast::EnumDef>),
-    Const,
+    Const(FileAstId<ast::ConstDef>),
-    Static,
+    Static(FileAstId<ast::StaticDef>),
-    Trait,
+    Trait(FileAstId<ast::TraitDef>),
-    TypeAlias,
+    TypeAlias(FileAstId<ast::TypeAliasDef>),
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -200,18 +199,31 @@ impl RawItemsCollector {
                 // impls don't participate in name resolution
                 return;
             }
-            ast::ModuleItemKind::StructDef(it) => (DefKind::Struct, it.name()),
+            ast::ModuleItemKind::StructDef(it) => {
-            ast::ModuleItemKind::EnumDef(it) => (DefKind::Enum, it.name()),
+                (DefKind::Struct(self.source_file_items.ast_id(it)), it.name())
-            ast::ModuleItemKind::FnDef(it) => (DefKind::Function, it.name()),
+            }
-            ast::ModuleItemKind::TraitDef(it) => (DefKind::Trait, it.name()),
+            ast::ModuleItemKind::EnumDef(it) => {
-            ast::ModuleItemKind::TypeAliasDef(it) => (DefKind::TypeAlias, it.name()),
+                (DefKind::Enum(self.source_file_items.ast_id(it)), it.name())
-            ast::ModuleItemKind::ConstDef(it) => (DefKind::Const, it.name()),
+            }
-            ast::ModuleItemKind::StaticDef(it) => (DefKind::Static, it.name()),
+            ast::ModuleItemKind::FnDef(it) => {
+                (DefKind::Function(self.source_file_items.ast_id(it)), it.name())
+            }
+            ast::ModuleItemKind::TraitDef(it) => {
+                (DefKind::Trait(self.source_file_items.ast_id(it)), it.name())
+            }
+            ast::ModuleItemKind::TypeAliasDef(it) => {
+                (DefKind::TypeAlias(self.source_file_items.ast_id(it)), it.name())
+            }
+            ast::ModuleItemKind::ConstDef(it) => {
+                (DefKind::Const(self.source_file_items.ast_id(it)), it.name())
+            }
+            ast::ModuleItemKind::StaticDef(it) => {
+                (DefKind::Static(self.source_file_items.ast_id(it)), it.name())
+            }
         };
         if let Some(name) = name {
             let name = name.as_name();
-            let source_item_id = self.source_file_items.id_of_unchecked(item.syntax());
+            let def = self.raw_items.defs.alloc(DefData { name, kind });
-            let def = self.raw_items.defs.alloc(DefData { name, kind, source_item_id });
             self.push_item(current_module, RawItem::Def(def))
         }
     }

crates/ra_hir/src/source_id.rs

@@ -1,11 +1,11 @@
-use std::{marker::PhantomData, 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};
 
-#[derive(Debug, PartialEq, Eq, Hash)]
+#[derive(Debug)]
 pub(crate) struct AstId<N: AstNode> {
     file_id: HirFileId,
     file_ast_id: FileAstId<N>,
@@ -16,9 +16,20 @@ impl<N: AstNode> Clone for 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
@@ -30,7 +41,7 @@ impl<N: AstNode> AstId<N> {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Hash)]
+#[derive(Debug)]
 pub(crate) struct FileAstId<N: AstNode> {
     raw: SourceFileItemId,
     _ty: PhantomData<N>,
@@ -41,9 +52,20 @@ impl<N: AstNode> Clone for FileAstId<N> {
         *self
     }
 }
-
 impl<N: AstNode> Copy for FileAstId<N> {}
 
+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);
+    }
+}
+
 impl<N: AstNode> FileAstId<N> {
     pub(crate) fn with_file_id(self, file_id: HirFileId) -> AstId<N> {
         AstId { file_id, file_ast_id: self }
@@ -116,14 +138,6 @@ impl SourceFileItems {
     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) {