Merge #4472

4472: Fix path resolution for module and function with same name r=hasali19 a=hasali19

This fixes #3970 and also fixes completion for the same issue.

Co-authored-by: Hasan Ali <[email protected]>

Author: bors[bot]

crates/ra_hir/src/semantics.rs

@@ -23,7 +23,7 @@ use crate::{
     db::HirDatabase,
     diagnostics::Diagnostic,
     semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
-    source_analyzer::{resolve_hir_path, SourceAnalyzer},
+    source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer},
     AssocItem, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, Module, ModuleDef,
     Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam,
 };
@@ -451,6 +451,23 @@ impl<'a, DB: HirDatabase> SemanticsScope<'a, DB> {
     pub fn resolve_hir_path(&self, path: &Path) -> Option<PathResolution> {
         resolve_hir_path(self.db, &self.resolver, path)
     }
+
+    /// Resolves a path where we know it is a qualifier of another path.
+    ///
+    /// For example, if we have:
+    /// ```
+    /// mod my {
+    ///     pub mod foo {
+    ///         struct Bar;
+    ///     }
+    ///
+    ///     pub fn foo() {}
+    /// }
+    /// ```
+    /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
+    pub fn resolve_hir_path_qualifier(&self, path: &Path) -> Option<PathResolution> {
+        resolve_hir_path_qualifier(self.db, &self.resolver, path)
+    }
 }
 
 // FIXME: Change `HasSource` trait to work with `Semantics` and remove this?

crates/ra_hir/src/source_analyzer.rs

@@ -226,6 +226,17 @@ impl SourceAnalyzer {
         // This must be a normal source file rather than macro file.
         let hir_path =
             crate::Path::from_src(path.clone(), &Hygiene::new(db.upcast(), self.file_id))?;
+
+        // Case where path is a qualifier of another path, e.g. foo::bar::Baz where we
+        // trying to resolve foo::bar.
+        if let Some(outer_path) = path.syntax().parent().and_then(ast::Path::cast) {
+            if let Some(qualifier) = outer_path.qualifier() {
+                if path == &qualifier {
+                    return resolve_hir_path_qualifier(db, &self.resolver, &hir_path);
+                }
+            }
+        }
+
         resolve_hir_path(db, &self.resolver, &hir_path)
     }
 
@@ -404,6 +415,7 @@ pub(crate) fn resolve_hir_path(
             TypeNs::BuiltinType(it) => PathResolution::Def(it.into()),
             TypeNs::TraitId(it) => PathResolution::Def(Trait::from(it).into()),
         });
+
     let body_owner = resolver.body_owner();
     let values =
         resolver.resolve_path_in_value_ns_fully(db.upcast(), path.mod_path()).and_then(|val| {
@@ -426,9 +438,48 @@ pub(crate) fn resolve_hir_path(
         .resolve_module_path_in_items(db.upcast(), path.mod_path())
         .take_types()
         .map(|it| PathResolution::Def(it.into()));
+
     types.or(values).or(items).or_else(|| {
         resolver
             .resolve_path_as_macro(db.upcast(), path.mod_path())
             .map(|def| PathResolution::Macro(def.into()))
     })
 }
+
+/// Resolves a path where we know it is a qualifier of another path.
+///
+/// For example, if we have:
+/// ```
+/// mod my {
+///     pub mod foo {
+///         struct Bar;
+///     }
+///
+///     pub fn foo() {}
+/// }
+/// ```
+/// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
+pub(crate) fn resolve_hir_path_qualifier(
+    db: &dyn HirDatabase,
+    resolver: &Resolver,
+    path: &crate::Path,
+) -> Option<PathResolution> {
+    let items = resolver
+        .resolve_module_path_in_items(db.upcast(), path.mod_path())
+        .take_types()
+        .map(|it| PathResolution::Def(it.into()));
+
+    if items.is_some() {
+        return items;
+    }
+
+    resolver.resolve_path_in_type_ns_fully(db.upcast(), path.mod_path()).map(|ty| match ty {
+        TypeNs::SelfType(it) => PathResolution::SelfType(it.into()),
+        TypeNs::GenericParam(id) => PathResolution::TypeParam(TypeParam { id }),
+        TypeNs::AdtSelfType(it) | TypeNs::AdtId(it) => PathResolution::Def(Adt::from(it).into()),
+        TypeNs::EnumVariantId(it) => PathResolution::Def(EnumVariant::from(it).into()),
+        TypeNs::TypeAliasId(it) => PathResolution::Def(TypeAlias::from(it).into()),
+        TypeNs::BuiltinType(it) => PathResolution::Def(it.into()),
+        TypeNs::TraitId(it) => PathResolution::Def(Trait::from(it).into()),
+    })
+}

crates/ra_ide/src/completion/complete_qualified_path.rs

@@ -20,7 +20,7 @@ pub(super) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
     let scope = ctx.scope();
     let context_module = scope.module();
 
-    let res = match scope.resolve_hir_path(&path) {
+    let res = match scope.resolve_hir_path_qualifier(&path) {
         Some(res) => res,
         None => return,
     };
@@ -225,6 +225,34 @@ mod tests {
         );
     }
 
+    #[test]
+    fn completes_mod_with_same_name_as_function() {
+        assert_debug_snapshot!(
+            do_reference_completion(
+                r"
+                use self::my::<|>;
+
+                mod my {
+                    pub struct Bar;
+                }
+
+                fn my() {}
+                "
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "Bar",
+                source_range: 31..31,
+                delete: 31..31,
+                insert: "Bar",
+                kind: Struct,
+            },
+        ]
+        "###
+        );
+    }
+
     #[test]
     fn path_visibility() {
         assert_debug_snapshot!(

crates/ra_ide/src/hover.rs

@@ -921,4 +921,21 @@ fn func(foo: i32) { if true { <|>foo; }; }
             &["unsafe trait foo"],
         );
     }
+
+    #[test]
+    fn test_hover_mod_with_same_name_as_function() {
+        check_hover_result(
+            "
+            //- /lib.rs
+            use self::m<|>y::Bar;
+
+            mod my {
+                pub struct Bar;
+            }
+
+            fn my() {}
+            ",
+            &["mod my"],
+        );
+    }
 }