Merge #9716

9716: internal: Stop reexporting `hir_def`'s `ItemInNs` from HIR r=jonas-schievink a=jonas-schievink

Resolves a FIXME and simplifies downstream code

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

crates/hir/src/from_id.rs

@@ -5,14 +5,13 @@
 
 use hir_def::{
     expr::{LabelId, PatId},
-    item_scope::ItemInNs,
     AdtId, AssocItemId, DefWithBodyId, EnumVariantId, FieldId, GenericDefId, GenericParamId,
     ModuleDefId, VariantId,
 };
 
 use crate::{
-    Adt, AssocItem, BuiltinType, DefWithBody, Field, GenericDef, GenericParam, Label, Local,
+    Adt, AssocItem, BuiltinType, DefWithBody, Field, GenericDef, GenericParam, ItemInNs, Label,
-    MacroDef, ModuleDef, Variant, VariantDef,
+    Local, ModuleDef, Variant, VariantDef,
 };
 
 macro_rules! from_id {
@@ -258,19 +257,22 @@ impl From<(DefWithBodyId, LabelId)> for Label {
     }
 }
 
-impl From<MacroDef> for ItemInNs {
+impl From<hir_def::item_scope::ItemInNs> for ItemInNs {
-    fn from(macro_def: MacroDef) -> Self {
+    fn from(it: hir_def::item_scope::ItemInNs) -> Self {
-        ItemInNs::Macros(macro_def.into())
+        match it {
+            hir_def::item_scope::ItemInNs::Types(it) => ItemInNs::Types(it.into()),
+            hir_def::item_scope::ItemInNs::Values(it) => ItemInNs::Values(it.into()),
+            hir_def::item_scope::ItemInNs::Macros(it) => ItemInNs::Macros(it.into()),
+        }
     }
 }
 
-impl From<ModuleDef> for ItemInNs {
+impl From<ItemInNs> for hir_def::item_scope::ItemInNs {
-    fn from(module_def: ModuleDef) -> Self {
+    fn from(it: ItemInNs) -> Self {
-        match module_def {
+        match it {
-            ModuleDef::Static(_) | ModuleDef::Const(_) | ModuleDef::Function(_) => {
+            ItemInNs::Types(it) => Self::Types(it.into()),
-                ItemInNs::Values(module_def.into())
+            ItemInNs::Values(it) => Self::Values(it.into()),
-            }
+            ItemInNs::Macros(it) => Self::Macros(it.into()),
-            _ => ItemInNs::Types(module_def.into()),
         }
     }
 }

crates/hir/src/lib.rs

@@ -108,7 +108,6 @@ pub use {
         attr::{Attr, Attrs, AttrsWithOwner, Documentation},
         find_path::PrefixKind,
         import_map,
-        item_scope::ItemInNs, // FIXME: don't re-export ItemInNs, as it uses raw ids.
         nameres::ModuleSource,
         path::{ModPath, PathKind},
         type_ref::{Mutability, TypeRef},
@@ -194,9 +193,11 @@ impl Crate {
         query: import_map::Query,
     ) -> impl Iterator<Item = Either<ModuleDef, MacroDef>> {
         let _p = profile::span("query_external_importables");
-        import_map::search_dependencies(db, self.into(), query).into_iter().map(|item| match item {
+        import_map::search_dependencies(db, self.into(), query).into_iter().map(|item| {
-            ItemInNs::Types(mod_id) | ItemInNs::Values(mod_id) => Either::Left(mod_id.into()),
+            match ItemInNs::from(item) {
-            ItemInNs::Macros(mac_id) => Either::Right(mac_id.into()),
+                ItemInNs::Types(mod_id) | ItemInNs::Values(mod_id) => Either::Left(mod_id),
+                ItemInNs::Macros(mac_id) => Either::Right(mac_id),
+            }
         })
     }
 
@@ -656,7 +657,7 @@ impl Module {
     /// Finds a path that can be used to refer to the given item from within
     /// this module, if possible.
     pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
-        hir_def::find_path::find_path(db, item.into(), self.into())
+        hir_def::find_path::find_path(db, item.into().into(), self.into())
     }
 
     /// Finds a path that can be used to refer to the given item from within
@@ -667,7 +668,7 @@ impl Module {
         item: impl Into<ItemInNs>,
         prefix_kind: PrefixKind,
     ) -> Option<ModPath> {
-        hir_def::find_path::find_path_prefixed(db, item.into(), self.into(), prefix_kind)
+        hir_def::find_path::find_path_prefixed(db, item.into().into(), self.into(), prefix_kind)
     }
 }
 
@@ -1567,6 +1568,39 @@ impl MacroDef {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
+pub enum ItemInNs {
+    Types(ModuleDef),
+    Values(ModuleDef),
+    Macros(MacroDef),
+}
+
+impl From<MacroDef> for ItemInNs {
+    fn from(it: MacroDef) -> Self {
+        Self::Macros(it)
+    }
+}
+
+impl From<ModuleDef> for ItemInNs {
+    fn from(module_def: ModuleDef) -> Self {
+        match module_def {
+            ModuleDef::Static(_) | ModuleDef::Const(_) | ModuleDef::Function(_) => {
+                ItemInNs::Values(module_def)
+            }
+            _ => ItemInNs::Types(module_def),
+        }
+    }
+}
+
+impl ItemInNs {
+    pub fn as_module_def(self) -> Option<ModuleDef> {
+        match self {
+            ItemInNs::Types(id) | ItemInNs::Values(id) => Some(id),
+            ItemInNs::Macros(_) => None,
+        }
+    }
+}
+
 /// Invariant: `inner.as_assoc_item(db).is_some()`
 /// We do not actively enforce this invariant.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]

crates/ide_assists/src/handlers/qualify_path.rs

@@ -176,7 +176,7 @@ fn find_trait_method(
 }
 
 fn item_as_trait(db: &RootDatabase, item: hir::ItemInNs) -> Option<hir::Trait> {
-    let item_module_def = hir::ModuleDef::from(item.as_module_def_id()?);
+    let item_module_def = item.as_module_def()?;
 
     if let hir::ModuleDef::Trait(trait_) = item_module_def {
         Some(trait_)

crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs

@@ -67,7 +67,7 @@ pub(crate) fn replace_derive_with_manual_impl(
         items_locator::AssocItemSearch::Exclude,
         Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT.inner()),
     )
-    .filter_map(|item| match ModuleDef::from(item.as_module_def_id()?) {
+    .filter_map(|item| match item.as_module_def()? {
         ModuleDef::Trait(trait_) => Some(trait_),
         _ => None,
     })

crates/ide_db/src/helpers/import_assets.rs

@@ -620,6 +620,5 @@ fn path_import_candidate(
 }
 
 fn item_as_assoc(db: &RootDatabase, item: ItemInNs) -> Option<AssocItem> {
-    item.as_module_def_id()
+    item.as_module_def().and_then(|module_def| module_def.as_assoc_item(db))
-        .and_then(|module_def_id| ModuleDef::from(module_def_id).as_assoc_item(db))
 }

crates/ide_db/src/items_locator.rs

@@ -5,7 +5,7 @@
 use either::Either;
 use hir::{
     import_map::{self, ImportKind},
-    AsAssocItem, Crate, ItemInNs, ModuleDef, Semantics,
+    AsAssocItem, Crate, ItemInNs, Semantics,
 };
 use limit::Limit;
 use syntax::{ast, AstNode, SyntaxKind::NAME};
@@ -147,7 +147,5 @@ fn get_name_definition(
 }
 
 fn is_assoc_item(item: ItemInNs, db: &RootDatabase) -> bool {
-    item.as_module_def_id()
+    item.as_module_def().and_then(|module_def| module_def.as_assoc_item(db)).is_some()
-        .and_then(|module_def_id| ModuleDef::from(module_def_id).as_assoc_item(db))
-        .is_some()
 }