Fix the completion labels and tests

4 files changed, 116 insertions, 65 deletions

diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs
index c1e3f091f..c5b3c9e27 100644
--- a/crates/ide_completion/src/completions/flyimport.rs
+++ b/crates/ide_completion/src/completions/flyimport.rs
@@ -144,7 +144,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
             .filter_map(|import| {
                 render_resolution_with_import(
                     RenderContext::new(ctx),
-                    ImportEdit { import, import_scope: import_scope.clone() },
+                    ImportEdit { import, scope: import_scope.clone() },
                 )
             }),
     );
@@ -690,8 +690,8 @@ fn main() {
 }
 "#,
             expect![[r#"
-                fn weird_function() (dep::test_mod::TestTrait) -> () DEPRECATED
                 ct SPECIAL_CONST (dep::test_mod::TestTrait) DEPRECATED
+                fn weird_function() (dep::test_mod::TestTrait) -> () DEPRECATED
             "#]],
         );
     }
@@ -807,7 +807,12 @@ fn main() {
     bar::baz::Ite$0
 }"#;
 
-        check(fixture, expect![["st Item (foo::bar::baz::Item)"]]);
+        check(
+            fixture,
+            expect![[r#"
+        st foo::bar::baz::Item
+        "#]],
+        );
 
         check_edit(
             "Item",
@@ -825,8 +830,7 @@ fn main() {
 
         fn main() {
             bar::baz::Item
-        }
-        "#,
+        }"#,
         );
     }
 
@@ -845,7 +849,12 @@ fn main() {
     Item::TEST_A$0
 }"#;
 
-        check(fixture, expect![["ct TEST_ASSOC (foo::bar::baz::Item)"]]);
+        check(
+            fixture,
+            expect![[r#"
+        ct TEST_ASSOC (foo::Item)
+        "#]],
+        );
 
         check_edit(
             "TEST_ASSOC",
@@ -863,8 +872,7 @@ mod foo {
 
 fn main() {
     Item::TEST_ASSOC
-}
-"#,
+}"#,
         );
     }
 
@@ -885,7 +893,12 @@ fn main() {
     bar::Item::TEST_A$0
 }"#;
 
-        check(fixture, expect![["ct TEST_ASSOC (foo::bar::baz::Item)"]]);
+        check(
+            fixture,
+            expect![[r#"
+        ct TEST_ASSOC (foo::bar::Item)
+    "#]],
+        );
 
         check_edit(
             "TEST_ASSOC",
@@ -905,8 +918,7 @@ mod foo {
 
 fn main() {
     bar::Item::TEST_ASSOC
-}
-"#,
+}"#,
         );
     }
 }
diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs
index d01620500..44e4a6dfd 100644
--- a/crates/ide_completion/src/item.rs
+++ b/crates/ide_completion/src/item.rs
@@ -11,7 +11,7 @@ use ide_db::{
     },
     SymbolKind,
 };
-use stdx::{impl_from, never};
+use stdx::{format_to, impl_from, never};
 use syntax::{algo, TextRange};
 use text_edit::TextEdit;
 
@@ -274,7 +274,7 @@ impl CompletionItem {
 #[derive(Debug, Clone)]
 pub struct ImportEdit {
     pub import: LocatedImport,
-    pub import_scope: ImportScope,
+    pub scope: ImportScope,
 }
 
 impl ImportEdit {
@@ -284,7 +284,7 @@ impl ImportEdit {
         let _p = profile::span("ImportEdit::to_text_edit");
 
         let rewriter = insert_use::insert_use(
-            &self.import_scope,
+            &self.scope,
             mod_path_to_ast(&self.import.import_path),
             cfg,
         );
@@ -302,7 +302,6 @@ impl ImportEdit {
 pub(crate) struct Builder {
     source_range: TextRange,
     completion_kind: CompletionKind,
-    // TODO kb also add a db here, to resolve the completion label?
     import_to_add: Option<ImportEdit>,
     label: String,
     insert_text: Option<String>,
@@ -322,22 +321,24 @@ impl Builder {
     pub(crate) fn build(self) -> CompletionItem {
         let _p = profile::span("item::Builder::build");
 
-        let label = self.label;
-        let lookup = self.lookup;
-        let insert_text = self.insert_text;
-
-        if let Some(_import_to_add) = self.import_to_add.as_ref() {
-            todo!("todo kb")
-            // let import = &import_to_add.import;
-            // let item_to_import = import.item_to_import();
-            // lookup = lookup.or_else(|| Some(label.clone()));
-            // insert_text = insert_text.or_else(|| Some(label.clone()));
-            // let display_path = import_to_add.import.display_path();
-            // if import_to_add.import {
-            //     label = format!("{} ({})", label, display_path);
-            // } else {
-            //     label = display_path.to_string();
-            // }
+        let mut label = self.label;
+        let mut lookup = self.lookup;
+        let mut insert_text = self.insert_text;
+
+        if let Some(original_path) = self
+            .import_to_add
+            .as_ref()
+            .and_then(|import_edit| import_edit.import.original_path.as_ref())
+        {
+            lookup = lookup.or_else(|| Some(label.clone()));
+            insert_text = insert_text.or_else(|| Some(label.clone()));
+
+            let original_path_label = original_path.to_string();
+            if original_path_label.ends_with(&label) {
+                label = original_path_label;
+            } else {
+                format_to!(label, " ({})", original_path)
+            }
         }
 
         let text_edit = match self.text_edit {
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs
index d19368de0..5470914fb 100644
--- a/crates/ide_completion/src/lib.rs
+++ b/crates/ide_completion/src/lib.rs
@@ -144,7 +144,7 @@ pub fn resolve_completion_edits(
 ) -> Option<Vec<TextEdit>> {
     let ctx = CompletionContext::new(db, position, config)?;
     let position_for_import = position_for_import(&ctx, None)?;
-    let import_scope = ImportScope::find_insert_use_container(position_for_import, &ctx.sema)?;
+    let scope = ImportScope::find_insert_use_container(position_for_import, &ctx.sema)?;
 
     let current_module = ctx.sema.scope(position_for_import).module()?;
     let current_crate = current_module.krate();
@@ -158,9 +158,10 @@ pub fn resolve_completion_edits(
                     .zip(Some(candidate))
             })
             .find(|(mod_path, _)| mod_path.to_string() == full_import_path)?;
-    let import = LocatedImport::new(import_path, item_to_import, item_to_import);
+    let import =
+        LocatedImport::new(import_path.clone(), item_to_import, item_to_import, Some(import_path));
 
-    ImportEdit { import, import_scope }.to_text_edit(config.insert_use).map(|edit| vec![edit])
+    ImportEdit { import, scope }.to_text_edit(config.insert_use).map(|edit| vec![edit])
 }
 
 #[cfg(test)]
diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs
index 8d16c011e..b3e90717a 100644
--- a/crates/ide_db/src/helpers/import_assets.rs
+++ b/crates/ide_db/src/helpers/import_assets.rs
@@ -132,11 +132,17 @@ pub struct LocatedImport {
     pub import_path: ModPath,
     pub item_to_import: ItemInNs,
     pub original_item: ItemInNs,
+    pub original_path: Option<ModPath>,
 }
 
 impl LocatedImport {
-    pub fn new(import_path: ModPath, item_to_import: ItemInNs, original_item: ItemInNs) -> Self {
-        Self { import_path, item_to_import, original_item }
+    pub fn new(
+        import_path: ModPath,
+        item_to_import: ItemInNs,
+        original_item: ItemInNs,
+        original_path: Option<ModPath>,
+    ) -> Self {
+        Self { import_path, item_to_import, original_item, original_path }
     }
 
     pub fn original_item_name(&self, db: &RootDatabase) -> Option<Name> {
@@ -238,7 +244,9 @@ impl<'a> ImportAssets<'a> {
         let _p = profile::span("import_assets::applicable_defs");
         let current_crate = self.module_with_candidate.krate();
 
-        let mod_path = |item| get_mod_path(db, item, &self.module_with_candidate, prefixed);
+        let mod_path = |item| {
+            get_mod_path(db, item_for_path_search(db, item)?, &self.module_with_candidate, prefixed)
+        };
 
         match &self.import_candidate {
             ImportCandidate::Path(path_candidate) => {
@@ -276,7 +284,9 @@ fn path_applicable_imports(
         Qualifier::Absent => {
             return items_with_candidate_name
                 .into_iter()
-                .filter_map(|item| Some(LocatedImport::new(mod_path(item)?, item, item)))
+                .filter_map(|item| {
+                    Some(LocatedImport::new(mod_path(item)?, item, item, mod_path(item)))
+                })
                 .collect();
         }
         Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => {
@@ -300,46 +310,69 @@ fn import_for_item(
     original_item: ItemInNs,
 ) -> Option<LocatedImport> {
     let _p = profile::span("import_assets::import_for_item");
-    let (item_candidate, trait_to_import) = match original_item.as_module_def_id() {
-        Some(module_def_id) => {
-            match ModuleDef::from(module_def_id).as_assoc_item(db).map(|assoc| assoc.container(db))
-            {
-                Some(AssocItemContainer::Trait(trait_)) => {
-                    let trait_item = ItemInNs::from(ModuleDef::from(trait_));
-                    (trait_item, Some(trait_item))
-                }
-                Some(AssocItemContainer::Impl(impl_)) => {
-                    (ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?)), None)
-                }
-                None => (original_item, None),
-            }
-        }
-        None => (original_item, None),
-    };
-    let import_path_candidate = mod_path(item_candidate)?;
 
+    let original_item_candidate = item_for_path_search(db, original_item)?;
+    let import_path_candidate = mod_path(original_item_candidate)?;
     let import_path_string = import_path_candidate.to_string();
+
     if !import_path_string.contains(unresolved_first_segment)
         || !import_path_string.contains(unresolved_qualifier)
     {
         return None;
     }
 
-    let segment_import = find_import_for_segment(db, item_candidate, &unresolved_first_segment)?;
-    Some(match (segment_import == item_candidate, trait_to_import) {
+    let segment_import =
+        find_import_for_segment(db, original_item_candidate, &unresolved_first_segment)?;
+    let trait_item_to_import = original_item
+        .as_module_def_id()
+        .and_then(|module_def_id| {
+            ModuleDef::from(module_def_id).as_assoc_item(db)?.containing_trait(db)
+        })
+        .map(|trait_| ItemInNs::from(ModuleDef::from(trait_)));
+    Some(match (segment_import == original_item_candidate, trait_item_to_import) {
         (true, Some(_)) => {
             // FIXME we should be able to import both the trait and the segment,
             // but it's unclear what to do with overlapping edits (merge imports?)
             // especially in case of lazy completion edit resolutions.
             return None;
         }
-        (false, Some(trait_to_import)) => {
-            LocatedImport::new(mod_path(trait_to_import)?, trait_to_import, original_item)
-        }
-        (true, None) => LocatedImport::new(import_path_candidate, item_candidate, original_item),
-        (false, None) => {
-            LocatedImport::new(mod_path(segment_import)?, segment_import, original_item)
+        (false, Some(trait_to_import)) => LocatedImport::new(
+            mod_path(trait_to_import)?,
+            trait_to_import,
+            original_item,
+            mod_path(original_item),
+        ),
+        (true, None) => LocatedImport::new(
+            import_path_candidate,
+            original_item_candidate,
+            original_item,
+            mod_path(original_item),
+        ),
+        (false, None) => LocatedImport::new(
+            mod_path(segment_import)?,
+            segment_import,
+            original_item,
+            mod_path(original_item),
+        ),
+    })
+}
+
+fn item_for_path_search(db: &RootDatabase, item: ItemInNs) -> Option<ItemInNs> {
+    Some(match item {
+        ItemInNs::Types(module_def_id) | ItemInNs::Values(module_def_id) => {
+            let module_def = ModuleDef::from(module_def_id);
+
+            match module_def.as_assoc_item(db) {
+                Some(assoc_item) => match assoc_item.container(db) {
+                    AssocItemContainer::Trait(trait_) => ItemInNs::from(ModuleDef::from(trait_)),
+                    AssocItemContainer::Impl(impl_) => {
+                        ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?))
+                    }
+                },
+                None => item,
+            }
         }
+        ItemInNs::Macros(_) => item,
     })
 }
 
@@ -420,10 +453,12 @@ fn trait_applicable_items(
                     }
 
                     let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?));
+                    let original_item = assoc_to_item(assoc);
                     located_imports.insert(LocatedImport::new(
                         mod_path(item)?,
                         item,
-                        assoc_to_item(assoc),
+                        original_item,
+                        mod_path(original_item),
                     ));
                 }
                 None::<()>
@@ -439,10 +474,12 @@ fn trait_applicable_items(
                 let assoc = function.as_assoc_item(db)?;
                 if required_assoc_items.contains(&assoc) {
                     let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?));
+                    let original_item = assoc_to_item(assoc);
                     located_imports.insert(LocatedImport::new(
                         mod_path(item)?,
                         item,
-                        assoc_to_item(assoc),
+                        original_item,
+                        mod_path(original_item),
                     ));
                 }
                 None::<()>