Merge #3553

3553: Completions do not show for function with same name as mod r=matklad a=JoshMcguigan

fixes #3444 

I've added a test case in `crates/ra_ide/src/completion/complete_path.rs` which verifies the described behavior in #3444. Digging in, I found that [the module scope iterator](ba62d8bd1c/crates/ra_ide/src/completion/complete_path.rs (L22)) only provides the module `z`, and does not provide the function `z` (although if I name the function something else then it does show up here). 

I thought perhaps the name wasn't being properly resolved, but I added a test in `crates/ra_hir_def/src/nameres/tests.rs` which seems to suggest that it is? I've tried to figure out how to bridge the gap between these two tests (one passing, one failing) to see where the function `z` is being dropped, but to this point I haven't been able to track it down.

Any pointers on where I might look for this? 

Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>

Cargo.lock

@@ -940,6 +940,7 @@ dependencies = [
 name = "ra_hir"
 version = "0.1.0"
 dependencies = [
+ "arrayvec",
  "either",
  "itertools",
  "log",

crates/ra_hir/Cargo.toml

@@ -11,6 +11,7 @@ doctest = false
 log = "0.4.8"
 rustc-hash = "1.1.0"
 either = "1.5.3"
+arrayvec = "0.5.1"
 
 itertools = "0.8.2"
 

crates/ra_hir/src/code_model.rs

@@ -1,6 +1,7 @@
 //! FIXME: write short doc here
 use std::sync::Arc;
 
+use arrayvec::ArrayVec;
 use either::Either;
 use hir_def::{
     adt::StructKind,
@@ -226,7 +227,11 @@ impl Module {
                     Some((name, def))
                 }
             })
-            .map(|(name, def)| (name.clone(), def.into()))
+            .flat_map(|(name, def)|
+                ScopeDef::all_items(def)
+                    .into_iter()
+                    .map(move |item| (name.clone(), item))
+            )
             .collect()
     }
 
@@ -1288,15 +1293,38 @@ pub enum ScopeDef {
     Unknown,
 }
 
-impl From<PerNs> for ScopeDef {
-    fn from(def: PerNs) -> Self {
-        def.take_types()
-            .or_else(|| def.take_values())
-            .map(|module_def_id| ScopeDef::ModuleDef(module_def_id.into()))
-            .or_else(|| {
-                def.take_macros().map(|macro_def_id| ScopeDef::MacroDef(macro_def_id.into()))
-            })
-            .unwrap_or(ScopeDef::Unknown)
+impl ScopeDef {
+    pub fn all_items(def: PerNs) -> ArrayVec<[Self; 3]> {
+        let mut items = ArrayVec::new();
+
+        match (def.take_types(), def.take_values()) {
+            (Some(m1), None) => 
+                items.push(ScopeDef::ModuleDef(m1.into())),
+            (None, Some(m2)) =>
+                items.push(ScopeDef::ModuleDef(m2.into())),
+            (Some(m1), Some(m2)) => {
+                // Some items, like unit structs and enum variants, are
+                // returned as both a type and a value. Here we want
+                // to de-duplicate them.
+                if m1 != m2 {
+                    items.push(ScopeDef::ModuleDef(m1.into()));
+                    items.push(ScopeDef::ModuleDef(m2.into()));
+                } else {
+                    items.push(ScopeDef::ModuleDef(m1.into()));
+                }
+            },
+            (None, None) => {},
+        };
+
+        if let Some(macro_def_id) = def.take_macros() {
+            items.push(ScopeDef::MacroDef(macro_def_id.into()));
+        }
+
+        if items.is_empty() {
+            items.push(ScopeDef::Unknown);
+        }
+
+        items
     }
 }
 

crates/ra_hir/src/semantics.rs

@@ -344,7 +344,13 @@ impl<'a, DB: HirDatabase> SemanticsScope<'a, DB> {
 
         resolver.process_all_names(self.db, &mut |name, def| {
             let def = match def {
-                resolver::ScopeDef::PerNs(it) => it.into(),
+                resolver::ScopeDef::PerNs(it) => {
+                    let items = ScopeDef::all_items(it);
+                    for item in items {
+                        f(name.clone(), item);
+                    }
+                    return
+                },
                 resolver::ScopeDef::ImplSelfType(it) => ScopeDef::ImplSelfType(it.into()),
                 resolver::ScopeDef::AdtSelfType(it) => ScopeDef::AdtSelfType(it.into()),
                 resolver::ScopeDef::GenericParam(id) => ScopeDef::GenericParam(TypeParam { id }),

crates/ra_hir_def/src/nameres/tests.rs

@@ -101,6 +101,28 @@ fn crate_def_map_super_super() {
     "###)
 }
 
+#[test]
+fn crate_def_map_fn_mod_same_name() {
+    let map = def_map(
+        "
+        //- /lib.rs
+        mod m {
+            pub mod z {}
+            pub fn z() {}
+        }
+        ",
+    );
+    assert_snapshot!(map, @r###"
+        ⋮crate
+        ⋮m: t
+        ⋮
+        ⋮crate::m
+        ⋮z: t v
+        ⋮
+        ⋮crate::m::z
+    "###)
+}
+
 #[test]
 fn bogus_paths() {
     covers!(bogus_paths);

crates/ra_ide/src/completion/complete_path.rs

@@ -967,4 +967,43 @@ mod tests {
         ]
         "###);
     }
+
+    #[test]
+    fn function_mod_share_name() {
+        assert_debug_snapshot!(
+        do_reference_completion(
+                r"
+                fn foo() {
+                    self::m::<|>
+                }
+
+                mod m {
+                    pub mod z {}
+                    pub fn z() {}
+                }
+                ",
+        ),
+            @r###"
+        [
+            CompletionItem {
+                label: "z",
+                source_range: [57; 57),
+                delete: [57; 57),
+                insert: "z",
+                kind: Module,
+            },
+            CompletionItem {
+                label: "z()",
+                source_range: [57; 57),
+                delete: [57; 57),
+                insert: "z()$0",
+                kind: Function,
+                lookup: "z",
+                detail: "pub fn z()",
+            },
+        ]
+        "###
+        );
+    }
+
 }