Merge pull request #19742 from Veykril/push-ykmuwtkzruqm

fix: Fix incorrect handling of unresolved non-module imports in name resolution

crates/hir-def/src/lib.rs

@@ -701,6 +701,16 @@ pub enum AssocItemId {
 // casting them, and somehow making the constructors private, which would be annoying.
 impl_from!(FunctionId, ConstId, TypeAliasId for AssocItemId);
 
+impl From<AssocItemId> for ModuleDefId {
+    fn from(item: AssocItemId) -> Self {
+        match item {
+            AssocItemId::FunctionId(f) => f.into(),
+            AssocItemId::ConstId(c) => c.into(),
+            AssocItemId::TypeAliasId(t) => t.into(),
+        }
+    }
+}
+
 #[derive(Debug, PartialOrd, Ord, Clone, Copy, PartialEq, Eq, Hash, salsa_macros::Supertype)]
 pub enum GenericDefId {
     AdtId(AdtId),

crates/hir-def/src/nameres/assoc.rs

@@ -66,6 +66,15 @@ impl TraitItems {
         })
     }
 
+    pub fn assoc_item_by_name(&self, name: &Name) -> Option<AssocItemId> {
+        self.items.iter().find_map(|&(ref item_name, item)| match item {
+            AssocItemId::FunctionId(_) if item_name == name => Some(item),
+            AssocItemId::TypeAliasId(_) if item_name == name => Some(item),
+            AssocItemId::ConstId(_) if item_name == name => Some(item),
+            _ => None,
+        })
+    }
+
     pub fn attribute_calls(&self) -> impl Iterator<Item = (AstId<ast::Item>, MacroCallId)> + '_ {
         self.macro_calls.iter().flat_map(|it| it.iter()).copied()
     }

crates/hir-def/src/nameres/collector.rs

@@ -26,7 +26,7 @@ use syntax::ast;
 use triomphe::Arc;
 
 use crate::{
-    AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc,
+    AdtId, AssocItemId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc,
     ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, ItemContainerId,
     LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, MacroRulesId,
     MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, ProcMacroLoc, StaticLoc,
@@ -45,7 +45,7 @@ use crate::{
         attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id},
         diagnostics::DefDiagnostic,
         mod_resolution::ModDir,
-        path_resolution::ReachedFixedPoint,
+        path_resolution::{ReachedFixedPoint, ResolvePathResult},
         proc_macro::{ProcMacroDef, ProcMacroKind, parse_macro_name_and_helper_attrs},
         sub_namespace_match,
     },
@@ -811,32 +811,35 @@ impl DefCollector<'_> {
         let _p = tracing::info_span!("resolve_import", import_path = %import.path.display(self.db, Edition::LATEST))
             .entered();
         tracing::debug!("resolving import: {:?} ({:?})", import, self.def_map.data.edition);
-        let res = self.def_map.resolve_path_fp_with_macro(
+        let ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, prefix_info } =
-            self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map),
+            self.def_map.resolve_path_fp_with_macro(
-            self.db,
+                self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map),
-            ResolveMode::Import,
+                self.db,
-            module_id,
+                ResolveMode::Import,
-            &import.path,
+                module_id,
-            BuiltinShadowMode::Module,
+                &import.path,
-            None, // An import may resolve to any kind of macro.
+                BuiltinShadowMode::Module,
-        );
+                None, // An import may resolve to any kind of macro.
+            );
 
-        let def = res.resolved_def;
+        if reached_fixedpoint == ReachedFixedPoint::No
-        if res.reached_fixedpoint == ReachedFixedPoint::No || def.is_none() {
+            || resolved_def.is_none()
+            || segment_index.is_some()
+        {
             return PartialResolvedImport::Unresolved;
         }
 
-        if res.prefix_info.differing_crate {
+        if prefix_info.differing_crate {
             return PartialResolvedImport::Resolved(
-                def.filter_visibility(|v| matches!(v, Visibility::Public)),
+                resolved_def.filter_visibility(|v| matches!(v, Visibility::Public)),
             );
         }
 
         // Check whether all namespaces are resolved.
-        if def.is_full() {
+        if resolved_def.is_full() {
-            PartialResolvedImport::Resolved(def)
+            PartialResolvedImport::Resolved(resolved_def)
         } else {
-            PartialResolvedImport::Indeterminate(def)
+            PartialResolvedImport::Indeterminate(resolved_def)
         }
     }
 
@@ -986,6 +989,43 @@ impl DefCollector<'_> {
                             Some(ImportOrExternCrate::Glob(glob)),
                         );
                     }
+                    Some(ModuleDefId::TraitId(it)) => {
+                        // FIXME: Implement this correctly
+                        // We can't actually call `trait_items`, the reason being that if macro calls
+                        // occur, they will call back into the def map which we might be computing right
+                        // now resulting in a cycle.
+                        // To properly implement this, trait item collection needs to be done in def map
+                        // collection...
+                        let resolutions = if true {
+                            vec![]
+                        } else {
+                            self.db
+                                .trait_items(it)
+                                .items
+                                .iter()
+                                .map(|&(ref name, variant)| {
+                                    let res = match variant {
+                                        AssocItemId::FunctionId(it) => {
+                                            PerNs::values(it.into(), vis, None)
+                                        }
+                                        AssocItemId::ConstId(it) => {
+                                            PerNs::values(it.into(), vis, None)
+                                        }
+                                        AssocItemId::TypeAliasId(it) => {
+                                            PerNs::types(it.into(), vis, None)
+                                        }
+                                    };
+                                    (Some(name.clone()), res)
+                                })
+                                .collect::<Vec<_>>()
+                        };
+                        self.update(
+                            module_id,
+                            &resolutions,
+                            vis,
+                            Some(ImportOrExternCrate::Glob(glob)),
+                        );
+                    }
                     Some(d) => {
                         tracing::debug!("glob import {:?} from non-module/enum {:?}", import, d);
                     }

crates/hir-def/src/nameres/path_resolution.rs

@@ -17,6 +17,7 @@ use hir_expand::{
     name::Name,
 };
 use span::Edition;
+use stdx::TupleExt;
 use triomphe::Arc;
 
 use crate::{
@@ -44,6 +45,7 @@ pub(super) enum ReachedFixedPoint {
 #[derive(Debug, Clone)]
 pub(super) struct ResolvePathResult {
     pub(super) resolved_def: PerNs,
+    /// The index of the last resolved segment, or `None` if the full path has been resolved.
     pub(super) segment_index: Option<usize>,
     pub(super) reached_fixedpoint: ReachedFixedPoint,
     pub(super) prefix_info: ResolvePathResultPrefixInfo,
@@ -364,7 +366,15 @@ impl DefMap {
             },
         };
 
-        self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module)
+        self.resolve_remaining_segments(
+            db,
+            mode,
+            segments,
+            curr_per_ns,
+            path,
+            shadow,
+            original_module,
+        )
     }
 
     /// Resolves a path only in the preludes, without accounting for item scopes.
@@ -413,7 +423,15 @@ impl DefMap {
             }
         };
 
-        self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module)
+        self.resolve_remaining_segments(
+            db,
+            mode,
+            segments,
+            curr_per_ns,
+            path,
+            shadow,
+            original_module,
+        )
     }
 
     /// 2018-style absolute path -- only extern prelude
@@ -441,10 +459,11 @@ impl DefMap {
 
     fn resolve_remaining_segments<'a>(
         &self,
+        db: &dyn DefDatabase,
+        mode: ResolveMode,
         mut segments: impl Iterator<Item = (usize, &'a Name)>,
         mut curr_per_ns: PerNs,
         path: &ModPath,
-        db: &dyn DefDatabase,
         shadow: BuiltinShadowMode,
         original_module: LocalModuleId,
     ) -> ResolvePathResult {
@@ -478,7 +497,7 @@ impl DefMap {
                         let resolution = defp_map.resolve_path_fp_with_macro(
                             LocalDefMap::EMPTY,
                             db,
-                            ResolveMode::Other,
+                            mode,
                             module.local_id,
                             &path,
                             shadow,
@@ -553,6 +572,44 @@ impl DefMap {
                         ),
                     };
                 }
+                def @ ModuleDefId::TraitId(t) if mode == ResolveMode::Import => {
+                    // FIXME: Implement this correctly
+                    // We can't actually call `trait_items`, the reason being that if macro calls
+                    // occur, they will call back into the def map which we might be computing right
+                    // now resulting in a cycle.
+                    // To properly implement this, trait item collection needs to be done in def map
+                    // collection...
+                    let item =
+                        if true { None } else { db.trait_items(t).assoc_item_by_name(segment) };
+                    return match item {
+                        Some(item) => ResolvePathResult::new(
+                            match item {
+                                crate::AssocItemId::FunctionId(function_id) => PerNs::values(
+                                    function_id.into(),
+                                    curr.vis,
+                                    curr.import.and_then(|it| it.import_or_glob()),
+                                ),
+                                crate::AssocItemId::ConstId(const_id) => PerNs::values(
+                                    const_id.into(),
+                                    curr.vis,
+                                    curr.import.and_then(|it| it.import_or_glob()),
+                                ),
+                                crate::AssocItemId::TypeAliasId(type_alias_id) => {
+                                    PerNs::types(type_alias_id.into(), curr.vis, curr.import)
+                                }
+                            },
+                            ReachedFixedPoint::Yes,
+                            segments.next().map(TupleExt::head),
+                            ResolvePathResultPrefixInfo::default(),
+                        ),
+                        None => ResolvePathResult::new(
+                            PerNs::types(def, curr.vis, curr.import),
+                            ReachedFixedPoint::Yes,
+                            Some(i),
+                            ResolvePathResultPrefixInfo::default(),
+                        ),
+                    };
+                }
                 s => {
                     // could be an inherent method call in UFCS form
                     // (`Struct::method`), or some other kind of associated item

crates/hir-def/src/nameres/tests/mod_resolution.rs

@@ -894,3 +894,149 @@ struct AlsoShouldNotAppear;
         "#]],
     )
 }
+
+#[test]
+fn invalid_imports() {
+    check(
+        r#"
+//- /main.rs
+mod module;
+
+use self::module::S::new;
+use self::module::unresolved;
+use self::module::C::const_based;
+use self::module::Enum::Variant::NoAssoc;
+
+//- /module.rs
+pub struct S;
+impl S {
+    pub fn new() {}
+}
+pub const C: () = ();
+pub enum Enum {
+    Variant,
+}
+        "#,
+        expect![[r#"
+            crate
+            NoAssoc: _
+            const_based: _
+            module: t
+            new: _
+            unresolved: _
+
+            crate::module
+            C: v
+            Enum: t
+            S: t v
+        "#]],
+    );
+}
+
+#[test]
+fn trait_item_imports_same_crate() {
+    check(
+        r#"
+//- /main.rs
+mod module;
+
+use self::module::Trait::{AssocType, ASSOC_CONST, MACRO_CONST, method};
+
+//- /module.rs
+macro_rules! m {
+    ($name:ident) => { const $name: () = (); };
+}
+pub trait Trait {
+    type AssocType;
+    const ASSOC_CONST: ();
+    fn method(&self);
+    m!(MACRO_CONST);
+}
+        "#,
+        expect![[r#"
+            crate
+            ASSOC_CONST: _
+            AssocType: _
+            MACRO_CONST: _
+            method: _
+            module: t
+
+            crate::module
+            Trait: t
+        "#]],
+    );
+    check(
+        r#"
+//- /main.rs
+mod module;
+
+use self::module::Trait::*;
+
+//- /module.rs
+macro_rules! m {
+    ($name:ident) => { const $name: () = (); };
+}
+pub trait Trait {
+    type AssocType;
+    const ASSOC_CONST: ();
+    fn method(&self);
+    m!(MACRO_CONST);
+}
+        "#,
+        expect![[r#"
+            crate
+            module: t
+
+            crate::module
+            Trait: t
+        "#]],
+    );
+}
+
+#[test]
+fn trait_item_imports_differing_crate() {
+    check(
+        r#"
+//- /main.rs deps:lib crate:main
+use lib::Trait::{AssocType, ASSOC_CONST, MACRO_CONST, method};
+
+//- /lib.rs crate:lib
+macro_rules! m {
+    ($name:ident) => { const $name: () = (); };
+}
+pub trait Trait {
+    type AssocType;
+    const ASSOC_CONST: ();
+    fn method(&self);
+    m!(MACRO_CONST);
+}
+        "#,
+        expect![[r#"
+            crate
+            ASSOC_CONST: _
+            AssocType: _
+            MACRO_CONST: _
+            method: _
+        "#]],
+    );
+    check(
+        r#"
+//- /main.rs deps:lib crate:main
+use lib::Trait::*;
+
+//- /lib.rs crate:lib
+macro_rules! m {
+    ($name:ident) => { const $name: () = (); };
+}
+pub trait Trait {
+    type AssocType;
+    const ASSOC_CONST: ();
+    fn method(&self);
+    m!(MACRO_CONST);
+}
+        "#,
+        expect![[r#"
+            crate
+        "#]],
+    );
+}

crates/hir-ty/src/tests/traits.rs

@@ -4884,3 +4884,22 @@ async fn baz<T: AsyncFnOnce(u32) -> i32>(c: T) {
         "#]],
     );
 }
+
+#[test]
+fn import_trait_items() {
+    check_infer(
+        r#"
+//- minicore: default
+use core::default::Default::default;
+fn main() {
+    let a: i32 = default();
+}
+    "#,
+        expect![[r#"
+            47..78 '{     ...t(); }': ()
+            57..58 'a': i32
+            66..73 'default': {unknown}
+            66..75 'default()': i32
+        "#]],
+    );
+}