From 24a5d3b19dfa3e076df8b7413d0cc4a547aeb7d7 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 3 Mar 2021 23:55:21 +0200 Subject: Fix the completion labels and tests --- crates/ide_completion/src/completions/flyimport.rs | 34 +++++--- crates/ide_completion/src/item.rs | 41 ++++----- crates/ide_completion/src/lib.rs | 7 +- crates/ide_db/src/helpers/import_assets.rs | 99 +++++++++++++++------- 4 files changed, 116 insertions(+), 65 deletions(-) (limited to 'crates') 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, label: String, insert_text: Option, @@ -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> { 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, } 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, + ) -> Self { + Self { import_path, item_to_import, original_item, original_path } } pub fn original_item_name(&self, db: &RootDatabase) -> Option { @@ -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 { 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 { + 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::<()> -- cgit v1.2.3