From 005bc49d744f6a7c2ef38a4abd3327b0804709d1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 20 Feb 2021 23:32:21 +0200 Subject: [PATCH 01/24] Test and initial refactoring --- .../ide_assists/src/handlers/auto_import.rs | 35 ---- .../ide_assists/src/handlers/qualify_path.rs | 4 +- .../src/completions/flyimport.rs | 37 ++++- crates/ide_db/src/helpers/import_assets.rs | 152 ++++++++---------- 4 files changed, 102 insertions(+), 126 deletions(-) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 1422224ac0..5fe3f47fd9 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -220,41 +220,6 @@ mod tests { ); } - #[test] - fn auto_imports_are_merged() { - check_assist( - auto_import, - r" - use PubMod::PubStruct1; - - struct Test { - test: Pub$0Struct2, - } - - pub mod PubMod { - pub struct PubStruct1; - pub struct PubStruct2 { - _t: T, - } - } - ", - r" - use PubMod::{PubStruct1, PubStruct2}; - - struct Test { - test: PubStruct2, - } - - pub mod PubMod { - pub struct PubStruct1; - pub struct PubStruct2 { - _t: T, - } - } - ", - ); - } - #[test] fn applicable_when_found_multiple_imports() { check_assist( diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index d3e34e5403..1fb4931cd0 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -45,7 +45,7 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let qualify_candidate = match candidate { ImportCandidate::Path(candidate) => { - if candidate.qualifier.is_some() { + if candidate.unresolved_qualifier.is_some() { cov_mark::hit!(qualify_path_qualifier_start); let path = ast::Path::cast(syntax_under_caret)?; let (prev_segment, segment) = (path.qualifier()?.segment()?, path.segment()?); @@ -192,7 +192,7 @@ fn group_label(candidate: &ImportCandidate) -> GroupLabel { fn label(candidate: &ImportCandidate, import: &hir::ModPath) -> String { match candidate { ImportCandidate::Path(candidate) => { - if candidate.qualifier.is_some() { + if candidate.unresolved_qualifier.is_some() { format!("Qualify with `{}`", &import) } else { format!("Qualify as `{}`", &import) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index f34764b610..16384551cf 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -168,15 +168,15 @@ fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option, + pub unresolved_qualifier: Option, pub name: NameToImport, } @@ -82,38 +82,14 @@ impl ImportAssets { } pub fn for_fuzzy_path( - module_with_path: Module, + module_with_candidate: Module, qualifier: Option, fuzzy_name: String, sema: &Semantics, ) -> Option { - Some(match qualifier { - Some(qualifier) => { - let qualifier_resolution = sema.resolve_path(&qualifier)?; - match qualifier_resolution { - hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path)) => Self { - import_candidate: ImportCandidate::TraitAssocItem(TraitImportCandidate { - receiver_ty: assoc_item_path.ty(sema.db), - name: NameToImport::Fuzzy(fuzzy_name), - }), - module_with_candidate: module_with_path, - }, - _ => Self { - import_candidate: ImportCandidate::Path(PathImportCandidate { - qualifier: Some(qualifier), - name: NameToImport::Fuzzy(fuzzy_name), - }), - module_with_candidate: module_with_path, - }, - } - } - None => Self { - import_candidate: ImportCandidate::Path(PathImportCandidate { - qualifier: None, - name: NameToImport::Fuzzy(fuzzy_name), - }), - module_with_candidate: module_with_path, - }, + Some(Self { + import_candidate: ImportCandidate::for_fuzzy_path(qualifier, fuzzy_name, sema)?, + module_with_candidate, }) } @@ -169,8 +145,9 @@ impl ImportAssets { prefixed: Option, ) -> Vec<(hir::ModPath, hir::ItemInNs)> { let current_crate = self.module_with_candidate.krate(); + let import_candidate = &self.import_candidate; - let unfiltered_imports = match self.name_to_import() { + let imports_for_candidate_name = match self.name_to_import() { NameToImport::Exact(exact_name) => { imports_locator::find_exact_imports(sema, current_crate, exact_name.clone()) } @@ -180,11 +157,10 @@ impl ImportAssets { // and https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Blanket.20trait.20impls.20lookup // for the details NameToImport::Fuzzy(fuzzy_name) => { - let (assoc_item_search, limit) = match self.import_candidate { - ImportCandidate::TraitAssocItem(_) | ImportCandidate::TraitMethod(_) => { - (AssocItemSearch::AssocItemsOnly, None) - } - _ => (AssocItemSearch::Exclude, Some(DEFAULT_QUERY_SEARCH_LIMIT)), + let (assoc_item_search, limit) = if import_candidate.is_trait_candidate() { + (AssocItemSearch::AssocItemsOnly, None) + } else { + (AssocItemSearch::Exclude, Some(DEFAULT_QUERY_SEARCH_LIMIT)) }; imports_locator::find_similar_imports( sema, @@ -198,24 +174,22 @@ impl ImportAssets { let db = sema.db; let mut res = - applicable_defs(self.import_candidate(), current_crate, db, unfiltered_imports) + applicable_defs(import_candidate, current_crate, db, imports_for_candidate_name) .filter_map(|candidate| { let item: hir::ItemInNs = candidate.clone().either(Into::into, Into::into); - let item_to_search = match self.import_candidate { - ImportCandidate::TraitAssocItem(_) | ImportCandidate::TraitMethod(_) => { - let canidate_trait = match candidate { - Either::Left(module_def) => { - module_def.as_assoc_item(db)?.containing_trait(db) - } - _ => None, - }?; - ModuleDef::from(canidate_trait).into() - } - _ => item, + let item_to_search = if import_candidate.is_trait_candidate() { + let canidate_trait = match candidate { + Either::Left(module_def) => { + module_def.as_assoc_item(db)?.containing_trait(db) + } + _ => None, + }?; + ModuleDef::from(canidate_trait).into() + } else { + item }; - - if let Some(prefix_kind) = prefixed { + let mod_path = if let Some(prefix_kind) = prefixed { self.module_with_candidate.find_use_path_prefixed( db, item_to_search, @@ -223,8 +197,9 @@ impl ImportAssets { ) } else { self.module_with_candidate.find_use_path(db, item_to_search) - } - .map(|path| (path, item)) + }; + + mod_path.zip(Some(item)) }) .filter(|(use_path, _)| use_path.len() > 1) .collect::>(); @@ -239,6 +214,7 @@ fn applicable_defs<'a>( db: &RootDatabase, unfiltered_imports: Box> + 'a>, ) -> Box> + 'a> { + // TODO kb this needs to consider various path prefixes, etc. let receiver_ty = match import_candidate { ImportCandidate::Path(_) => return unfiltered_imports, ImportCandidate::TraitAssocItem(candidate) | ImportCandidate::TraitMethod(candidate) => { @@ -325,41 +301,45 @@ impl ImportCandidate { if sema.resolve_path(path).is_some() { return None; } + path_import_candidate( + sema, + path.qualifier(), + NameToImport::Exact(path.segment()?.name_ref()?.to_string()), + ) + } - let segment = path.segment()?; - let candidate = if let Some(qualifier) = path.qualifier() { - let qualifier_start = qualifier.syntax().descendants().find_map(ast::NameRef::cast)?; - let qualifier_start_path = - qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; - if let Some(qualifier_start_resolution) = sema.resolve_path(&qualifier_start_path) { - let qualifier_resolution = if qualifier_start_path == qualifier { - qualifier_start_resolution - } else { - sema.resolve_path(&qualifier)? - }; - match qualifier_resolution { - hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path)) => { - ImportCandidate::TraitAssocItem(TraitImportCandidate { - receiver_ty: assoc_item_path.ty(sema.db), - name: NameToImport::Exact(segment.name_ref()?.to_string()), - }) - } - _ => return None, - } - } else { - ImportCandidate::Path(PathImportCandidate { - qualifier: Some(qualifier), - name: NameToImport::Exact(qualifier_start.to_string()), - }) - } - } else { - ImportCandidate::Path(PathImportCandidate { - qualifier: None, - name: NameToImport::Exact( - segment.syntax().descendants().find_map(ast::NameRef::cast)?.to_string(), - ), - }) - }; - Some(candidate) + fn for_fuzzy_path( + qualifier: Option, + fuzzy_name: String, + sema: &Semantics, + ) -> Option { + path_import_candidate(sema, qualifier, NameToImport::Fuzzy(fuzzy_name)) + } + + fn is_trait_candidate(&self) -> bool { + matches!(self, ImportCandidate::TraitAssocItem(_) | ImportCandidate::TraitMethod(_)) } } + +fn path_import_candidate( + sema: &Semantics, + qualifier: Option, + name: NameToImport, +) -> Option { + Some(match qualifier { + Some(qualifier) => match sema.resolve_path(&qualifier) { + None => ImportCandidate::Path(PathImportCandidate { + unresolved_qualifier: Some(qualifier), + name, + }), + Some(hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path))) => { + ImportCandidate::TraitAssocItem(TraitImportCandidate { + receiver_ty: assoc_item_path.ty(sema.db), + name, + }) + } + Some(_) => return None, + }, + None => ImportCandidate::Path(PathImportCandidate { unresolved_qualifier: None, name }), + }) +} From 7584260b9a222658e3e38f1d2506da95e6106283 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 21 Feb 2021 00:14:27 +0200 Subject: [PATCH 02/24] Find the code to change --- crates/ide_db/src/helpers/import_assets.rs | 185 +++++++++++++++------ 1 file changed, 130 insertions(+), 55 deletions(-) diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 513128eae2..0da4bdd0ed 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,6 +1,9 @@ //! Look up accessible paths for items. use either::Either; -use hir::{AsAssocItem, AssocItem, Crate, MacroDef, Module, ModuleDef, PrefixKind, Semantics}; +use hir::{ + AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, PathResolution, + PrefixKind, Semantics, +}; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -145,7 +148,6 @@ impl ImportAssets { prefixed: Option, ) -> Vec<(hir::ModPath, hir::ItemInNs)> { let current_crate = self.module_with_candidate.krate(); - let import_candidate = &self.import_candidate; let imports_for_candidate_name = match self.name_to_import() { NameToImport::Exact(exact_name) => { @@ -157,7 +159,7 @@ impl ImportAssets { // and https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Blanket.20trait.20impls.20lookup // for the details NameToImport::Fuzzy(fuzzy_name) => { - let (assoc_item_search, limit) = if import_candidate.is_trait_candidate() { + let (assoc_item_search, limit) = if self.import_candidate.is_trait_candidate() { (AssocItemSearch::AssocItemsOnly, None) } else { (AssocItemSearch::Exclude, Some(DEFAULT_QUERY_SEARCH_LIMIT)) @@ -172,59 +174,108 @@ impl ImportAssets { } }; - let db = sema.db; - let mut res = - applicable_defs(import_candidate, current_crate, db, imports_for_candidate_name) - .filter_map(|candidate| { - let item: hir::ItemInNs = candidate.clone().either(Into::into, Into::into); - - let item_to_search = if import_candidate.is_trait_candidate() { - let canidate_trait = match candidate { - Either::Left(module_def) => { - module_def.as_assoc_item(db)?.containing_trait(db) - } - _ => None, - }?; - ModuleDef::from(canidate_trait).into() - } else { - item - }; - let mod_path = if let Some(prefix_kind) = prefixed { - self.module_with_candidate.find_use_path_prefixed( - db, - item_to_search, - prefix_kind, - ) - } else { - self.module_with_candidate.find_use_path(db, item_to_search) - }; - - mod_path.zip(Some(item)) - }) - .filter(|(use_path, _)| use_path.len() > 1) - .collect::>(); + let mut res = self + .applicable_defs(sema, prefixed, imports_for_candidate_name) + .filter(|(use_path, _)| use_path.len() > 1) + .collect::>(); res.sort_by_cached_key(|(path, _)| path.clone()); res } + + fn applicable_defs<'a>( + &self, + sema: &'a Semantics, + prefixed: Option, + unfiltered_imports: Box> + 'a>, + ) -> Box + 'a> { + let current_crate = self.module_with_candidate.krate(); + let db = sema.db; + + match &self.import_candidate { + ImportCandidate::Path(path_candidate) => path_applicable_defs( + sema, + path_candidate, + unfiltered_imports, + self.module_with_candidate, + prefixed, + ), + ImportCandidate::TraitAssocItem(trait_candidate) => trait_applicable_defs( + db, + current_crate, + trait_candidate, + true, + unfiltered_imports, + self.module_with_candidate, + prefixed, + ), + ImportCandidate::TraitMethod(trait_candidate) => trait_applicable_defs( + db, + current_crate, + trait_candidate, + false, + unfiltered_imports, + self.module_with_candidate, + prefixed, + ), + } + } } -fn applicable_defs<'a>( - import_candidate: &ImportCandidate, - current_crate: Crate, - db: &RootDatabase, - unfiltered_imports: Box> + 'a>, -) -> Box> + 'a> { - // TODO kb this needs to consider various path prefixes, etc. - let receiver_ty = match import_candidate { - ImportCandidate::Path(_) => return unfiltered_imports, - ImportCandidate::TraitAssocItem(candidate) | ImportCandidate::TraitMethod(candidate) => { - &candidate.receiver_ty +fn path_applicable_defs<'a>( + sema: &'a Semantics, + path_candidate: &PathImportCandidate, + unfiltered_defs: Box> + 'a>, + module_with_candidate: Module, + prefixed: Option, +) -> Box + 'a> { + let applicable_defs = unfiltered_defs + .map(|candidate| candidate.either(ItemInNs::from, ItemInNs::from)) + .filter_map(move |item_to_search| { + get_mod_path(sema.db, item_to_search, &module_with_candidate, prefixed) + .zip(Some(item_to_search)) + }); + + let unresolved_qualifier = match &path_candidate.unresolved_qualifier { + Some(qualifier) => qualifier, + None => { + // TODO kb too many boxes tossed around + return Box::new(applicable_defs); } }; + // TODO kb filter out items: found path should end with `qualifier::Name` or `qualifier::Something` for fuzzy search case. + + // TODO kb find a way to turn a qualifier into the corresponding ModuleDef. Maybe through the unfiltered data? + if let Some(qualifier_start_resolution) = resolve_qualifier_start(sema, unresolved_qualifier) { + // TODO kb ascend until an unresolved segment part appears + } else { + // first segment is already unresolved, need to turn it into ModuleDef somehow + } + + return Box::new(applicable_defs); +} + +fn resolve_qualifier_start( + sema: &Semantics, + qualifier: &ast::Path, +) -> Option { + let qualifier_start = qualifier.syntax().descendants().find_map(ast::NameRef::cast)?; + let qualifier_start_path = qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; + sema.resolve_path(&qualifier_start_path) +} + +fn trait_applicable_defs<'a>( + db: &'a RootDatabase, + current_crate: Crate, + trait_candidate: &TraitImportCandidate, + trait_assoc_item: bool, + unfiltered_defs: Box> + 'a>, + module_with_candidate: Module, + prefixed: Option, +) -> Box + 'a> { let mut required_assoc_items = FxHashSet::default(); - let trait_candidates = unfiltered_imports + let trait_candidates = unfiltered_defs .filter_map(|input| match input { Either::Left(module_def) => module_def.as_assoc_item(db), _ => None, @@ -238,9 +289,8 @@ fn applicable_defs<'a>( let mut applicable_defs = FxHashSet::default(); - match import_candidate { - ImportCandidate::Path(_) => unreachable!(), - ImportCandidate::TraitAssocItem(_) => receiver_ty.iterate_path_candidates( + if trait_assoc_item { + trait_candidate.receiver_ty.iterate_path_candidates( db, current_crate, &trait_candidates, @@ -252,12 +302,13 @@ fn applicable_defs<'a>( return None; } } - applicable_defs.insert(Either::Left(assoc_to_module_def(assoc))); + applicable_defs.insert(assoc_to_module_def(assoc)); } None::<()> }, - ), - ImportCandidate::TraitMethod(_) => receiver_ty.iterate_method_candidates( + ) + } else { + trait_candidate.receiver_ty.iterate_method_candidates( db, current_crate, &trait_candidates, @@ -265,14 +316,38 @@ fn applicable_defs<'a>( |_, function| { let assoc = function.as_assoc_item(db)?; if required_assoc_items.contains(&assoc) { - applicable_defs.insert(Either::Left(assoc_to_module_def(assoc))); + applicable_defs.insert(assoc_to_module_def(assoc)); } None::<()> }, - ), + ) }; - Box::new(applicable_defs.into_iter()) + Box::new( + applicable_defs + .into_iter() + .filter_map(move |candidate| { + let canidate_trait = candidate.as_assoc_item(db)?.containing_trait(db)?; + Some(ItemInNs::from(ModuleDef::from(canidate_trait))) + }) + .filter_map(move |item_to_search| { + get_mod_path(db, item_to_search, &module_with_candidate, prefixed) + .zip(Some(item_to_search)) + }), + ) +} + +fn get_mod_path( + db: &RootDatabase, + item_to_search: ItemInNs, + module_with_candidate: &Module, + prefixed: Option, +) -> Option { + if let Some(prefix_kind) = prefixed { + module_with_candidate.find_use_path_prefixed(db, item_to_search, prefix_kind) + } else { + module_with_candidate.find_use_path(db, item_to_search) + } } fn assoc_to_module_def(assoc: AssocItem) -> ModuleDef { From f08c0cdd2a99d2851d472d2c6b3f54d37aa13e8a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 21 Feb 2021 23:52:29 +0200 Subject: [PATCH 03/24] Simplify --- crates/ide_db/src/helpers/import_assets.rs | 101 ++++++++------------- 1 file changed, 37 insertions(+), 64 deletions(-) diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 0da4bdd0ed..66e60b698a 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -183,39 +183,41 @@ impl ImportAssets { } fn applicable_defs<'a>( - &self, + &'a self, sema: &'a Semantics, prefixed: Option, - unfiltered_imports: Box> + 'a>, + unfiltered_defs: impl Iterator> + 'a, ) -> Box + 'a> { let current_crate = self.module_with_candidate.krate(); let db = sema.db; match &self.import_candidate { - ImportCandidate::Path(path_candidate) => path_applicable_defs( + ImportCandidate::Path(path_candidate) => Box::new(path_applicable_defs( sema, path_candidate, - unfiltered_imports, - self.module_with_candidate, - prefixed, + unfiltered_defs + .into_iter() + .map(|def| def.either(ItemInNs::from, ItemInNs::from)) + .filter_map(move |item_to_search| { + get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) + .zip(Some(item_to_search)) + }), + )), + ImportCandidate::TraitAssocItem(trait_candidate) => Box::new( + trait_applicable_defs(db, current_crate, trait_candidate, true, unfiltered_defs) + .into_iter() + .filter_map(move |item_to_search| { + get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) + .zip(Some(item_to_search)) + }), ), - ImportCandidate::TraitAssocItem(trait_candidate) => trait_applicable_defs( - db, - current_crate, - trait_candidate, - true, - unfiltered_imports, - self.module_with_candidate, - prefixed, - ), - ImportCandidate::TraitMethod(trait_candidate) => trait_applicable_defs( - db, - current_crate, - trait_candidate, - false, - unfiltered_imports, - self.module_with_candidate, - prefixed, + ImportCandidate::TraitMethod(trait_candidate) => Box::new( + trait_applicable_defs(db, current_crate, trait_candidate, false, unfiltered_defs) + .into_iter() + .filter_map(move |item_to_search| { + get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) + .zip(Some(item_to_search)) + }), ), } } @@ -224,22 +226,12 @@ impl ImportAssets { fn path_applicable_defs<'a>( sema: &'a Semantics, path_candidate: &PathImportCandidate, - unfiltered_defs: Box> + 'a>, - module_with_candidate: Module, - prefixed: Option, -) -> Box + 'a> { - let applicable_defs = unfiltered_defs - .map(|candidate| candidate.either(ItemInNs::from, ItemInNs::from)) - .filter_map(move |item_to_search| { - get_mod_path(sema.db, item_to_search, &module_with_candidate, prefixed) - .zip(Some(item_to_search)) - }); - + unfiltered_defs: impl Iterator + 'a, +) -> impl Iterator + 'a { let unresolved_qualifier = match &path_candidate.unresolved_qualifier { Some(qualifier) => qualifier, None => { - // TODO kb too many boxes tossed around - return Box::new(applicable_defs); + return unfiltered_defs; } }; @@ -252,7 +244,7 @@ fn path_applicable_defs<'a>( // first segment is already unresolved, need to turn it into ModuleDef somehow } - return Box::new(applicable_defs); + return unfiltered_defs; } fn resolve_qualifier_start( @@ -269,10 +261,8 @@ fn trait_applicable_defs<'a>( current_crate: Crate, trait_candidate: &TraitImportCandidate, trait_assoc_item: bool, - unfiltered_defs: Box> + 'a>, - module_with_candidate: Module, - prefixed: Option, -) -> Box + 'a> { + unfiltered_defs: impl Iterator> + 'a, +) -> FxHashSet { let mut required_assoc_items = FxHashSet::default(); let trait_candidates = unfiltered_defs @@ -287,7 +277,7 @@ fn trait_applicable_defs<'a>( }) .collect(); - let mut applicable_defs = FxHashSet::default(); + let mut applicable_traits = FxHashSet::default(); if trait_assoc_item { trait_candidate.receiver_ty.iterate_path_candidates( @@ -302,7 +292,8 @@ fn trait_applicable_defs<'a>( return None; } } - applicable_defs.insert(assoc_to_module_def(assoc)); + applicable_traits + .insert(ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?))); } None::<()> }, @@ -316,25 +307,15 @@ fn trait_applicable_defs<'a>( |_, function| { let assoc = function.as_assoc_item(db)?; if required_assoc_items.contains(&assoc) { - applicable_defs.insert(assoc_to_module_def(assoc)); + applicable_traits + .insert(ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?))); } None::<()> }, ) }; - Box::new( - applicable_defs - .into_iter() - .filter_map(move |candidate| { - let canidate_trait = candidate.as_assoc_item(db)?.containing_trait(db)?; - Some(ItemInNs::from(ModuleDef::from(canidate_trait))) - }) - .filter_map(move |item_to_search| { - get_mod_path(db, item_to_search, &module_with_candidate, prefixed) - .zip(Some(item_to_search)) - }), - ) + applicable_traits } fn get_mod_path( @@ -350,14 +331,6 @@ fn get_mod_path( } } -fn assoc_to_module_def(assoc: AssocItem) -> ModuleDef { - match assoc { - AssocItem::Function(f) => f.into(), - AssocItem::Const(c) => c.into(), - AssocItem::TypeAlias(t) => t.into(), - } -} - impl ImportCandidate { fn for_method_call( sema: &Semantics, From c395c3311dc2ac59251e86eaa6b86b597358d31f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 22 Feb 2021 00:51:13 +0200 Subject: [PATCH 04/24] Filter out path items by the qualifier --- crates/ide_db/src/helpers/import_assets.rs | 39 +++++++++------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 66e60b698a..976dc92fbc 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,8 +1,8 @@ //! Look up accessible paths for items. use either::Either; use hir::{ - AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, PathResolution, - PrefixKind, Semantics, + AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, PrefixKind, + Semantics, }; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -192,7 +192,7 @@ impl ImportAssets { let db = sema.db; match &self.import_candidate { - ImportCandidate::Path(path_candidate) => Box::new(path_applicable_defs( + ImportCandidate::Path(path_candidate) => Box::new(path_applicable_items( sema, path_candidate, unfiltered_defs @@ -223,37 +223,28 @@ impl ImportAssets { } } -fn path_applicable_defs<'a>( +fn path_applicable_items<'a>( sema: &'a Semantics, path_candidate: &PathImportCandidate, unfiltered_defs: impl Iterator + 'a, -) -> impl Iterator + 'a { +) -> Box + 'a> { let unresolved_qualifier = match &path_candidate.unresolved_qualifier { Some(qualifier) => qualifier, None => { - return unfiltered_defs; + return Box::new(unfiltered_defs); } }; - // TODO kb filter out items: found path should end with `qualifier::Name` or `qualifier::Something` for fuzzy search case. + let qualifier_string = unresolved_qualifier.to_string(); + Box::new(unfiltered_defs.filter(move |(candidate_path, _)| { + let mut candidate_qualifier = candidate_path.clone(); + candidate_qualifier.pop_segment(); - // TODO kb find a way to turn a qualifier into the corresponding ModuleDef. Maybe through the unfiltered data? - if let Some(qualifier_start_resolution) = resolve_qualifier_start(sema, unresolved_qualifier) { - // TODO kb ascend until an unresolved segment part appears - } else { - // first segment is already unresolved, need to turn it into ModuleDef somehow - } - - return unfiltered_defs; -} - -fn resolve_qualifier_start( - sema: &Semantics, - qualifier: &ast::Path, -) -> Option { - let qualifier_start = qualifier.syntax().descendants().find_map(ast::NameRef::cast)?; - let qualifier_start_path = qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; - sema.resolve_path(&qualifier_start_path) + // TODO kb + // * take 1st segment of `unresolved_qualifier` and return it instead of the original `ItemInNs` + // * Update `ModPath`: pop until 1st segment of `unresolved_qualifier` reached (do not rely on name comparison, nested mod names can repeat) + candidate_qualifier.to_string().ends_with(&qualifier_string) + })) } fn trait_applicable_defs<'a>( From 309421c117fc20e58b9f30fb28a01a89f50b0086 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 24 Feb 2021 01:20:00 +0200 Subject: [PATCH 05/24] Draft the qualifier import resolution --- .../ide_assists/src/handlers/qualify_path.rs | 9 +- .../src/completions/flyimport.rs | 84 ++++++++- crates/ide_db/src/helpers/import_assets.rs | 173 ++++++++++++++---- crates/ide_db/src/imports_locator.rs | 1 + 4 files changed, 224 insertions(+), 43 deletions(-) diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index 1fb4931cd0..c0463311ef 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -1,7 +1,10 @@ use std::iter; use hir::AsAssocItem; -use ide_db::helpers::{import_assets::ImportCandidate, mod_path_to_ast}; +use ide_db::helpers::{ + import_assets::{ImportCandidate, Qualifier}, + mod_path_to_ast, +}; use ide_db::RootDatabase; use syntax::{ ast, @@ -45,7 +48,7 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let qualify_candidate = match candidate { ImportCandidate::Path(candidate) => { - if candidate.unresolved_qualifier.is_some() { + if !matches!(candidate.qualifier, Qualifier::Absent) { cov_mark::hit!(qualify_path_qualifier_start); let path = ast::Path::cast(syntax_under_caret)?; let (prev_segment, segment) = (path.qualifier()?.segment()?, path.segment()?); @@ -192,7 +195,7 @@ fn group_label(candidate: &ImportCandidate) -> GroupLabel { fn label(candidate: &ImportCandidate, import: &hir::ModPath) -> String { match candidate { ImportCandidate::Path(candidate) => { - if candidate.unresolved_qualifier.is_some() { + if !matches!(candidate.qualifier, Qualifier::Absent) { format!("Qualify with `{}`", &import) } else { format!("Qualify as `{}`", &import) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 16384551cf..64b60bbdd4 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -775,18 +775,92 @@ fn main() { } #[test] - fn unresolved_qualifiers() { + fn unresolved_qualifier() { + check_edit( + "Item", + r#" +mod foo { + pub mod bar { + pub mod baz { + pub struct Item; + } + } +} + +fn main() { + bar::baz::Ite$0 +} +"#, + r#" +use foo::bar; + +mod foo { + pub mod bar { + pub mod baz { + pub struct Item; + } + } +} + +fn main() { + bar::baz::Item +} +"#, + ); + } + + #[test] + fn unresolved_assoc_item_container() { + check_edit( + "Item", + r#" +mod foo { + pub struct Item; + + impl Item { + pub const TEST_ASSOC: usize = 3; + } +} + +fn main() { + Item::TEST_A$0; +} +"#, + r#" +use foo::Item; + +mod foo { + pub struct Item; + + impl Item { + pub const TEST_ASSOC: usize = 3; + } +} + +fn main() { + Item::TEST_ASSOC +} +"#, + ); + } + + #[test] + fn unresolved_assoc_item_container_with_path() { check_edit( "Item", r#" mod foo { pub mod bar { pub struct Item; + + impl Item { + pub const TEST_ASSOC: usize = 3; + } } } fn main() { - bar::Ite$0 + bar::Item::TEST_A$0; } "#, r#" @@ -795,11 +869,15 @@ use foo::bar; mod foo { pub mod bar { pub struct Item; + + impl Item { + pub const TEST_ASSOC: usize = 3; + } } } fn main() { - bar::Item + bar::Item::TEST_ASSOC } "#, ); diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 976dc92fbc..dc3b92a643 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,8 +1,8 @@ //! Look up accessible paths for items. use either::Either; use hir::{ - AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, PrefixKind, - Semantics, + AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, Name, + PrefixKind, Semantics, }; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -34,10 +34,16 @@ pub struct TraitImportCandidate { #[derive(Debug)] pub struct PathImportCandidate { - pub unresolved_qualifier: Option, + pub qualifier: Qualifier, pub name: NameToImport, } +#[derive(Debug)] +pub enum Qualifier { + Absent, + FirstSegmentUnresolved(ast::PathSegment, ast::Path), +} + #[derive(Debug)] pub enum NameToImport { Exact(String), @@ -162,8 +168,9 @@ impl ImportAssets { let (assoc_item_search, limit) = if self.import_candidate.is_trait_candidate() { (AssocItemSearch::AssocItemsOnly, None) } else { - (AssocItemSearch::Exclude, Some(DEFAULT_QUERY_SEARCH_LIMIT)) + (AssocItemSearch::Include, Some(DEFAULT_QUERY_SEARCH_LIMIT)) }; + imports_locator::find_similar_imports( sema, current_crate, @@ -192,17 +199,16 @@ impl ImportAssets { let db = sema.db; match &self.import_candidate { - ImportCandidate::Path(path_candidate) => Box::new(path_applicable_items( - sema, - path_candidate, - unfiltered_defs - .into_iter() - .map(|def| def.either(ItemInNs::from, ItemInNs::from)) - .filter_map(move |item_to_search| { - get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) - .zip(Some(item_to_search)) - }), - )), + ImportCandidate::Path(path_candidate) => Box::new( + path_applicable_items( + db, + path_candidate, + &self.module_with_candidate, + prefixed, + unfiltered_defs, + ) + .into_iter(), + ), ImportCandidate::TraitAssocItem(trait_candidate) => Box::new( trait_applicable_defs(db, current_crate, trait_candidate, true, unfiltered_defs) .into_iter() @@ -224,27 +230,110 @@ impl ImportAssets { } fn path_applicable_items<'a>( - sema: &'a Semantics, - path_candidate: &PathImportCandidate, - unfiltered_defs: impl Iterator + 'a, -) -> Box + 'a> { - let unresolved_qualifier = match &path_candidate.unresolved_qualifier { - Some(qualifier) => qualifier, - None => { - return Box::new(unfiltered_defs); + db: &'a RootDatabase, + path_candidate: &'a PathImportCandidate, + module_with_candidate: &hir::Module, + prefixed: Option, + unfiltered_defs: impl Iterator> + 'a, +) -> FxHashSet<(ModPath, ItemInNs)> { + let applicable_items = unfiltered_defs + .filter_map(|def| { + let (assoc_original, candidate) = match def { + Either::Left(module_def) => match module_def.as_assoc_item(db) { + Some(assoc_item) => match assoc_item.container(db) { + hir::AssocItemContainer::Trait(trait_) => { + (Some(module_def), ItemInNs::from(ModuleDef::from(trait_))) + } + hir::AssocItemContainer::Impl(impl_) => ( + Some(module_def), + ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?)), + ), + }, + None => (None, ItemInNs::from(module_def)), + }, + Either::Right(macro_def) => (None, ItemInNs::from(macro_def)), + }; + Some((assoc_original, candidate)) + }) + .filter_map(|(assoc_original, candidate)| { + get_mod_path(db, candidate, module_with_candidate, prefixed) + .zip(Some((assoc_original, candidate))) + }); + + let (unresolved_first_segment, unresolved_qualifier) = match &path_candidate.qualifier { + Qualifier::Absent => { + return applicable_items + .map(|(candidate_path, (_, candidate))| (candidate_path, candidate)) + .collect(); } + Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier), }; - let qualifier_string = unresolved_qualifier.to_string(); - Box::new(unfiltered_defs.filter(move |(candidate_path, _)| { - let mut candidate_qualifier = candidate_path.clone(); - candidate_qualifier.pop_segment(); + // TODO kb need to remove turbofish from the qualifier, maybe use the segments instead? + let unresolved_qualifier_string = unresolved_qualifier.to_string(); + let unresolved_first_segment_string = unresolved_first_segment.to_string(); - // TODO kb - // * take 1st segment of `unresolved_qualifier` and return it instead of the original `ItemInNs` - // * Update `ModPath`: pop until 1st segment of `unresolved_qualifier` reached (do not rely on name comparison, nested mod names can repeat) - candidate_qualifier.to_string().ends_with(&qualifier_string) - })) + applicable_items + .filter(|(candidate_path, _)| { + let candidate_path_string = candidate_path.to_string(); + candidate_path_string.contains(&unresolved_qualifier_string) + && candidate_path_string.contains(&unresolved_first_segment_string) + }) + // TODO kb need to adjust the return type: I get the results rendered rather badly + .filter_map(|(candidate_path, (assoc_original, candidate))| { + if let Some(assoc_original) = assoc_original { + if item_name(db, candidate)?.to_string() == unresolved_first_segment_string { + return Some((candidate_path, ItemInNs::from(assoc_original))); + } + } + + let matching_module = + module_with_matching_name(db, &unresolved_first_segment_string, candidate)?; + let path = get_mod_path( + db, + ItemInNs::from(ModuleDef::from(matching_module)), + module_with_candidate, + prefixed, + )?; + Some((path, candidate)) + }) + .collect() +} + +fn item_name(db: &RootDatabase, item: ItemInNs) -> Option { + match item { + ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).name(db), + ItemInNs::Values(module_def_id) => ModuleDef::from(module_def_id).name(db), + ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).name(db), + } +} + +fn item_module(db: &RootDatabase, item: ItemInNs) -> Option { + match item { + ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).module(db), + ItemInNs::Values(module_def_id) => ModuleDef::from(module_def_id).module(db), + ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).module(db), + } +} + +fn module_with_matching_name( + db: &RootDatabase, + unresolved_first_segment_string: &str, + candidate: ItemInNs, +) -> Option { + let mut current_module = item_module(db, candidate); + while let Some(module) = current_module { + match module.name(db) { + Some(module_name) => { + if module_name.to_string().as_str() == unresolved_first_segment_string { + return Some(module); + } + } + None => {} + } + current_module = module.parent(db); + } + None } fn trait_applicable_defs<'a>( @@ -367,10 +456,20 @@ fn path_import_candidate( ) -> Option { Some(match qualifier { Some(qualifier) => match sema.resolve_path(&qualifier) { - None => ImportCandidate::Path(PathImportCandidate { - unresolved_qualifier: Some(qualifier), - name, - }), + None => { + let qualifier_start = + qualifier.syntax().descendants().find_map(ast::PathSegment::cast)?; + let qualifier_start_path = + qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; + if sema.resolve_path(&qualifier_start_path).is_none() { + ImportCandidate::Path(PathImportCandidate { + qualifier: Qualifier::FirstSegmentUnresolved(qualifier_start, qualifier), + name, + }) + } else { + return None; + } + } Some(hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path))) => { ImportCandidate::TraitAssocItem(TraitImportCandidate { receiver_ty: assoc_item_path.ty(sema.db), @@ -379,6 +478,6 @@ fn path_import_candidate( } Some(_) => return None, }, - None => ImportCandidate::Path(PathImportCandidate { unresolved_qualifier: None, name }), + None => ImportCandidate::Path(PathImportCandidate { qualifier: Qualifier::Absent, name }), }) } diff --git a/crates/ide_db/src/imports_locator.rs b/crates/ide_db/src/imports_locator.rs index 502e8281a3..480cbf1ea8 100644 --- a/crates/ide_db/src/imports_locator.rs +++ b/crates/ide_db/src/imports_locator.rs @@ -40,6 +40,7 @@ pub fn find_exact_imports<'a>( )) } +#[derive(Debug)] pub enum AssocItemSearch { Include, Exclude, From 582cee2cdf5355b681f14bbb33bd5c431c284d87 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 Feb 2021 01:06:31 +0200 Subject: [PATCH 06/24] Return more data about located imports --- .../ide_assists/src/handlers/auto_import.rs | 10 +- .../ide_assists/src/handlers/qualify_path.rs | 17 +- .../src/completions/flyimport.rs | 23 +-- crates/ide_completion/src/item.rs | 28 ++- crates/ide_completion/src/lib.rs | 20 +- crates/ide_completion/src/render.rs | 2 +- crates/ide_db/src/helpers/import_assets.rs | 179 +++++++++++------- crates/ide_db/src/imports_locator.rs | 11 +- crates/rust-analyzer/src/handlers.rs | 7 +- 9 files changed, 172 insertions(+), 125 deletions(-) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 5fe3f47fd9..7188724be7 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -92,14 +92,18 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let range = ctx.sema.original_range(&syntax_under_caret).range; let group = import_group_message(import_assets.import_candidate()); let scope = ImportScope::find_insert_use_container(&syntax_under_caret, &ctx.sema)?; - for (import, _) in proposed_imports { + for import in proposed_imports { acc.add_group( &group, AssistId("auto_import", AssistKind::QuickFix), - format!("Import `{}`", &import), + format!("Import `{}`", import.display_path()), range, |builder| { - let rewriter = insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use); + let rewriter = insert_use( + &scope, + mod_path_to_ast(import.import_path()), + ctx.config.insert_use, + ); builder.rewrite(rewriter); }, ); diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index c0463311ef..a40cdd80e0 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -74,17 +74,17 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> }; let group_label = group_label(candidate); - for (import, item) in proposed_imports { + for import in proposed_imports { acc.add_group( &group_label, AssistId("qualify_path", AssistKind::QuickFix), - label(candidate, &import), + label(candidate, import.display_path()), range, |builder| { qualify_candidate.qualify( |replace_with: String| builder.replace(range, replace_with), - import, - item, + import.import_path(), + import.item_to_import(), ) }, ); @@ -100,8 +100,13 @@ enum QualifyCandidate<'db> { } impl QualifyCandidate<'_> { - fn qualify(&self, mut replacer: impl FnMut(String), import: hir::ModPath, item: hir::ItemInNs) { - let import = mod_path_to_ast(&import); + fn qualify( + &self, + mut replacer: impl FnMut(String), + import: &hir::ModPath, + item: hir::ItemInNs, + ) { + let import = mod_path_to_ast(import); match self { QualifyCandidate::QualifierStart(segment, generics) => { let generics = generics.as_ref().map_or_else(String::new, ToString::to_string); diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 64b60bbdd4..8ff76688e1 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -96,21 +96,21 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) let mut all_mod_paths = import_assets .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) .into_iter() - .map(|(mod_path, item_in_ns)| { - let scope_item = match item_in_ns { + .map(|import| { + let proposed_def = match import.item_to_import() { hir::ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), hir::ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), hir::ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), }; - (mod_path, scope_item) + (import, proposed_def) }) .filter(|(_, proposed_def)| !scope_definitions.contains(proposed_def)) .collect::>(); - all_mod_paths.sort_by_cached_key(|(mod_path, _)| { - compute_fuzzy_completion_order_key(mod_path, &user_input_lowercased) + all_mod_paths.sort_by_cached_key(|(import, _)| { + compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased) }); - acc.add_all(all_mod_paths.into_iter().filter_map(|(import_path, definition)| { + acc.add_all(all_mod_paths.into_iter().filter_map(|(import, definition)| { let import_for_trait_assoc_item = match definition { ScopeDef::ModuleDef(module_def) => module_def .as_assoc_item(ctx.db) @@ -118,11 +118,8 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) .is_some(), _ => false, }; - let import_edit = ImportEdit { - import_path, - import_scope: import_scope.clone(), - import_for_trait_assoc_item, - }; + let import_edit = + ImportEdit { import, import_scope: import_scope.clone(), import_for_trait_assoc_item }; render_resolution_with_import(RenderContext::new(ctx), import_edit, &definition) })); Some(()) @@ -186,11 +183,11 @@ fn compute_fuzzy_completion_order_key( user_input_lowercased: &str, ) -> usize { cov_mark::hit!(certain_fuzzy_order_test); - let proposed_import_name = match proposed_mod_path.segments().last() { + let import_name = match proposed_mod_path.segments().last() { Some(name) => name.to_string().to_lowercase(), None => return usize::MAX, }; - match proposed_import_name.match_indices(user_input_lowercased).next() { + match import_name.match_indices(user_input_lowercased).next() { Some((first_matching_index, _)) => first_matching_index, None => usize::MAX, } diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 9b2435c4b6..0390fe2266 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -2,9 +2,10 @@ use std::fmt; -use hir::{Documentation, ModPath, Mutability}; +use hir::{Documentation, Mutability}; use ide_db::{ helpers::{ + import_assets::LocatedImport, insert_use::{self, ImportScope, InsertUseConfig}, mod_path_to_ast, SnippetCap, }, @@ -272,7 +273,7 @@ impl CompletionItem { /// An extra import to add after the completion is applied. #[derive(Debug, Clone)] pub struct ImportEdit { - pub import_path: ModPath, + pub import: LocatedImport, pub import_scope: ImportScope, pub import_for_trait_assoc_item: bool, } @@ -283,8 +284,11 @@ impl ImportEdit { pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option { let _p = profile::span("ImportEdit::to_text_edit"); - let rewriter = - insert_use::insert_use(&self.import_scope, mod_path_to_ast(&self.import_path), cfg); + let rewriter = insert_use::insert_use( + &self.import_scope, + mod_path_to_ast(self.import.import_path()), + cfg, + ); let old_ast = rewriter.rewrite_root()?; let mut import_insert = TextEdit::builder(); algo::diff(&old_ast, &rewriter.rewrite(&old_ast)).into_text_edit(&mut import_insert); @@ -323,19 +327,13 @@ impl Builder { let mut insert_text = self.insert_text; if let Some(import_to_add) = self.import_to_add.as_ref() { + 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_for_trait_assoc_item { - lookup = lookup.or_else(|| Some(label.clone())); - insert_text = insert_text.or_else(|| Some(label.clone())); - label = format!("{} ({})", label, import_to_add.import_path); + label = format!("{} ({})", label, display_path); } else { - let mut import_path_without_last_segment = import_to_add.import_path.to_owned(); - let _ = import_path_without_last_segment.pop_segment(); - - if !import_path_without_last_segment.segments().is_empty() { - lookup = lookup.or_else(|| Some(label.clone())); - insert_text = insert_text.or_else(|| Some(label.clone())); - label = format!("{}::{}", import_path_without_last_segment, label); - } + label = display_path.to_string(); } } diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index b0b809791e..ca2e5e706a 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -13,7 +13,9 @@ mod completions; use completions::flyimport::position_for_import; use ide_db::{ - base_db::FilePosition, helpers::insert_use::ImportScope, imports_locator, RootDatabase, + base_db::FilePosition, + helpers::{import_assets::LocatedImport, insert_use::ImportScope}, + imports_locator, RootDatabase, }; use text_edit::TextEdit; @@ -148,12 +150,16 @@ pub fn resolve_completion_edits( let current_module = ctx.sema.scope(position_for_import).module()?; let current_crate = current_module.krate(); - let import_path = imports_locator::find_exact_imports(&ctx.sema, current_crate, imported_name) - .filter_map(|candidate| { - let item: hir::ItemInNs = candidate.either(Into::into, Into::into); - current_module.find_use_path_prefixed(db, item, config.insert_use.prefix_kind) - }) - .find(|mod_path| mod_path.to_string() == full_import_path)?; + let (import_path, item_to_import) = + imports_locator::find_exact_imports(&ctx.sema, current_crate, imported_name) + .filter_map(|candidate| { + let item: hir::ItemInNs = candidate.either(Into::into, Into::into); + current_module + .find_use_path_prefixed(db, item, config.insert_use.prefix_kind) + .zip(Some(item)) + }) + .find(|(mod_path, _)| mod_path.to_string() == full_import_path)?; + let import = LocatedImport::new(import_path, item_to_import, None); ImportEdit { import_path, import_scope, import_for_trait_assoc_item } .to_text_edit(config.insert_use) diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index dcfac23c55..df26e76426 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -56,7 +56,7 @@ pub(crate) fn render_resolution_with_import<'a>( ScopeDef::ModuleDef(ModuleDef::Function(f)) => f.name(ctx.completion.db).to_string(), ScopeDef::ModuleDef(ModuleDef::Const(c)) => c.name(ctx.completion.db)?.to_string(), ScopeDef::ModuleDef(ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db).to_string(), - _ => import_edit.import_path.segments().last()?.to_string(), + _ => import_edit.import.display_path().segments().last()?.to_string(), }; Render::new(ctx).render_resolution(local_name, Some(import_edit), resolution).map(|mut item| { item.completion_kind = CompletionKind::Magic; diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index dc3b92a643..d8bf61aaaf 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -117,6 +117,42 @@ impl ImportAssets { } } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct LocatedImport { + import_path: ModPath, + item_to_import: ItemInNs, + import_display_override: Option<(ModPath, ItemInNs)>, +} + +impl LocatedImport { + pub fn new( + import_path: ModPath, + item_to_import: ItemInNs, + import_display_override: Option<(ModPath, ItemInNs)>, + ) -> Self { + Self { import_path, item_to_import, import_display_override } + } + + pub fn display_path(&self) -> &ModPath { + self.import_display_override + .as_ref() + .map(|(mod_path, _)| mod_path) + .unwrap_or(&self.import_path) + } + + pub fn import_path(&self) -> &ModPath { + &self.import_path + } + + pub fn item_to_display(&self) -> ItemInNs { + self.import_display_override.as_ref().map(|&(_, item)| item).unwrap_or(self.item_to_import) + } + + pub fn item_to_import(&self) -> ItemInNs { + self.item_to_import + } +} + impl ImportAssets { pub fn import_candidate(&self) -> &ImportCandidate { &self.import_candidate @@ -134,16 +170,13 @@ impl ImportAssets { &self, sema: &Semantics, prefix_kind: PrefixKind, - ) -> Vec<(hir::ModPath, hir::ItemInNs)> { + ) -> Vec { let _p = profile::span("import_assets::search_for_imports"); self.search_for(sema, Some(prefix_kind)) } /// This may return non-absolute paths if a part of the returned path is already imported into scope. - pub fn search_for_relative_paths( - &self, - sema: &Semantics, - ) -> Vec<(hir::ModPath, hir::ItemInNs)> { + pub fn search_for_relative_paths(&self, sema: &Semantics) -> Vec { let _p = profile::span("import_assets::search_for_relative_paths"); self.search_for(sema, None) } @@ -152,7 +185,7 @@ impl ImportAssets { &self, sema: &Semantics, prefixed: Option, - ) -> Vec<(hir::ModPath, hir::ItemInNs)> { + ) -> Vec { let current_crate = self.module_with_candidate.krate(); let imports_for_candidate_name = match self.name_to_import() { @@ -181,61 +214,53 @@ impl ImportAssets { } }; - let mut res = self - .applicable_defs(sema, prefixed, imports_for_candidate_name) - .filter(|(use_path, _)| use_path.len() > 1) - .collect::>(); - res.sort_by_cached_key(|(path, _)| path.clone()); - res + self.applicable_defs(sema.db, prefixed, imports_for_candidate_name) + .into_iter() + .filter(|import| import.import_path().len() > 1) + .collect() } - fn applicable_defs<'a>( - &'a self, - sema: &'a Semantics, + fn applicable_defs( + &self, + db: &RootDatabase, prefixed: Option, - unfiltered_defs: impl Iterator> + 'a, - ) -> Box + 'a> { + unfiltered_defs: impl Iterator>, + ) -> FxHashSet { let current_crate = self.module_with_candidate.krate(); - let db = sema.db; + + let import_path_locator = + |item| get_mod_path(db, item, &self.module_with_candidate, prefixed); match &self.import_candidate { - ImportCandidate::Path(path_candidate) => Box::new( - path_applicable_items( - db, - path_candidate, - &self.module_with_candidate, - prefixed, - unfiltered_defs, - ) - .into_iter(), + ImportCandidate::Path(path_candidate) => { + path_applicable_imports(db, path_candidate, import_path_locator, unfiltered_defs) + } + ImportCandidate::TraitAssocItem(trait_candidate) => trait_applicable_items( + db, + current_crate, + trait_candidate, + true, + import_path_locator, + unfiltered_defs, ), - ImportCandidate::TraitAssocItem(trait_candidate) => Box::new( - trait_applicable_defs(db, current_crate, trait_candidate, true, unfiltered_defs) - .into_iter() - .filter_map(move |item_to_search| { - get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) - .zip(Some(item_to_search)) - }), - ), - ImportCandidate::TraitMethod(trait_candidate) => Box::new( - trait_applicable_defs(db, current_crate, trait_candidate, false, unfiltered_defs) - .into_iter() - .filter_map(move |item_to_search| { - get_mod_path(db, item_to_search, &self.module_with_candidate, prefixed) - .zip(Some(item_to_search)) - }), + ImportCandidate::TraitMethod(trait_candidate) => trait_applicable_items( + db, + current_crate, + trait_candidate, + false, + import_path_locator, + unfiltered_defs, ), } } } -fn path_applicable_items<'a>( - db: &'a RootDatabase, - path_candidate: &'a PathImportCandidate, - module_with_candidate: &hir::Module, - prefixed: Option, - unfiltered_defs: impl Iterator> + 'a, -) -> FxHashSet<(ModPath, ItemInNs)> { +fn path_applicable_imports( + db: &RootDatabase, + path_candidate: &PathImportCandidate, + import_path_locator: impl Fn(ItemInNs) -> Option, + unfiltered_defs: impl Iterator>, +) -> FxHashSet { let applicable_items = unfiltered_defs .filter_map(|def| { let (assoc_original, candidate) = match def { @@ -256,14 +281,15 @@ fn path_applicable_items<'a>( Some((assoc_original, candidate)) }) .filter_map(|(assoc_original, candidate)| { - get_mod_path(db, candidate, module_with_candidate, prefixed) - .zip(Some((assoc_original, candidate))) + import_path_locator(candidate).zip(Some((assoc_original, candidate))) }); let (unresolved_first_segment, unresolved_qualifier) = match &path_candidate.qualifier { Qualifier::Absent => { return applicable_items - .map(|(candidate_path, (_, candidate))| (candidate_path, candidate)) + .map(|(candidate_path, (_, candidate))| { + LocatedImport::new(candidate_path, candidate, None) + }) .collect(); } Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier), @@ -283,19 +309,22 @@ fn path_applicable_items<'a>( .filter_map(|(candidate_path, (assoc_original, candidate))| { if let Some(assoc_original) = assoc_original { if item_name(db, candidate)?.to_string() == unresolved_first_segment_string { - return Some((candidate_path, ItemInNs::from(assoc_original))); + return Some(LocatedImport::new( + candidate_path.clone(), + ItemInNs::from(assoc_original), + Some((candidate_path, candidate)), + )); } } let matching_module = module_with_matching_name(db, &unresolved_first_segment_string, candidate)?; - let path = get_mod_path( - db, - ItemInNs::from(ModuleDef::from(matching_module)), - module_with_candidate, - prefixed, - )?; - Some((path, candidate)) + let item = ItemInNs::from(ModuleDef::from(matching_module)); + Some(LocatedImport::new( + import_path_locator(item)?, + item, + Some((candidate_path, candidate)), + )) }) .collect() } @@ -336,13 +365,14 @@ fn module_with_matching_name( None } -fn trait_applicable_defs<'a>( - db: &'a RootDatabase, +fn trait_applicable_items( + db: &RootDatabase, current_crate: Crate, trait_candidate: &TraitImportCandidate, trait_assoc_item: bool, - unfiltered_defs: impl Iterator> + 'a, -) -> FxHashSet { + import_path_locator: impl Fn(ItemInNs) -> Option, + unfiltered_defs: impl Iterator>, +) -> FxHashSet { let mut required_assoc_items = FxHashSet::default(); let trait_candidates = unfiltered_defs @@ -357,7 +387,7 @@ fn trait_applicable_defs<'a>( }) .collect(); - let mut applicable_traits = FxHashSet::default(); + let mut located_imports = FxHashSet::default(); if trait_assoc_item { trait_candidate.receiver_ty.iterate_path_candidates( @@ -372,8 +402,13 @@ fn trait_applicable_defs<'a>( return None; } } - applicable_traits - .insert(ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?))); + + let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?)); + located_imports.insert(LocatedImport::new( + import_path_locator(item)?, + item, + None, + )); } None::<()> }, @@ -387,15 +422,19 @@ fn trait_applicable_defs<'a>( |_, function| { let assoc = function.as_assoc_item(db)?; if required_assoc_items.contains(&assoc) { - applicable_traits - .insert(ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?))); + let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?)); + located_imports.insert(LocatedImport::new( + import_path_locator(item)?, + item, + None, + )); } None::<()> }, ) }; - applicable_traits + located_imports } fn get_mod_path( diff --git a/crates/ide_db/src/imports_locator.rs b/crates/ide_db/src/imports_locator.rs index 480cbf1ea8..fd700e04ff 100644 --- a/crates/ide_db/src/imports_locator.rs +++ b/crates/ide_db/src/imports_locator.rs @@ -17,8 +17,8 @@ use rustc_hash::FxHashSet; pub(crate) const DEFAULT_QUERY_SEARCH_LIMIT: usize = 40; -pub fn find_exact_imports<'a>( - sema: &Semantics<'a, RootDatabase>, +pub fn find_exact_imports( + sema: &Semantics<'_, RootDatabase>, krate: Crate, name_to_import: String, ) -> Box>> { @@ -48,7 +48,7 @@ pub enum AssocItemSearch { } pub fn find_similar_imports<'a>( - sema: &Semantics<'a, RootDatabase>, + sema: &'a Semantics<'a, RootDatabase>, krate: Crate, fuzzy_search_string: String, assoc_item_search: AssocItemSearch, @@ -77,12 +77,11 @@ pub fn find_similar_imports<'a>( local_query.limit(limit); } - let db = sema.db; Box::new(find_imports(sema, krate, local_query, external_query).filter( move |import_candidate| match assoc_item_search { AssocItemSearch::Include => true, - AssocItemSearch::Exclude => !is_assoc_item(import_candidate, db), - AssocItemSearch::AssocItemsOnly => is_assoc_item(import_candidate, db), + AssocItemSearch::Exclude => !is_assoc_item(import_candidate, sema.db), + AssocItemSearch::AssocItemsOnly => is_assoc_item(import_candidate, sema.db), }, )) } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 4f6f250d6c..d479d826fb 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -1534,14 +1534,13 @@ fn fill_resolve_data( position: &TextDocumentPositionParams, ) -> Option<()> { let import_edit = item.import_to_add()?; - let full_import_path = import_edit.import_path.to_string(); - let imported_name = import_edit.import_path.segments().last()?.to_string(); + let import_path = import_edit.import.import_path(); *resolve_data = Some( to_value(CompletionResolveData { position: position.to_owned(), - full_import_path, - imported_name, + full_import_path: import_path.to_string(), + imported_name: import_path.segments().last()?.to_string(), import_for_trait_assoc_item: import_edit.import_for_trait_assoc_item, }) .unwrap(), From d386481fac65e988fa4d13c1bc8d4ddb2bc490c6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 Feb 2021 01:53:59 +0200 Subject: [PATCH 07/24] Fix some tests --- .../src/completions/flyimport.rs | 65 ++++++++++- crates/ide_completion/src/render.rs | 7 +- crates/ide_db/src/helpers.rs | 10 +- crates/ide_db/src/helpers/import_assets.rs | 110 +++++++++++------- 4 files changed, 143 insertions(+), 49 deletions(-) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 8ff76688e1..e33fc4b78a 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -97,7 +97,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) .into_iter() .map(|import| { - let proposed_def = match import.item_to_import() { + let proposed_def = match import.item_to_display() { hir::ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), hir::ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), hir::ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), @@ -809,7 +809,7 @@ fn main() { #[test] fn unresolved_assoc_item_container() { check_edit( - "Item", + "TEST_ASSOC", r#" mod foo { pub struct Item; @@ -820,7 +820,7 @@ mod foo { } fn main() { - Item::TEST_A$0; + Item::TEST_A$0 } "#, r#" @@ -844,7 +844,7 @@ fn main() { #[test] fn unresolved_assoc_item_container_with_path() { check_edit( - "Item", + "TEST_ASSOC", r#" mod foo { pub mod bar { @@ -857,7 +857,7 @@ mod foo { } fn main() { - bar::Item::TEST_A$0; + bar::Item::TEST_A$0 } "#, r#" @@ -876,6 +876,61 @@ mod foo { fn main() { bar::Item::TEST_ASSOC } +"#, + ); + } + + #[test] + fn unresolved_assoc_item_container_and_trait_with_path() { + check_edit( + "TEST_ASSOC", + r#" +mod foo { + pub mod bar { + pub trait SomeTrait { + const TEST_ASSOC: usize; + } + } + + pub mod baz { + use super::bar::SomeTrait; + + pub struct Item; + + impl SomeTrait for Item { + const TEST_ASSOC: usize = 3; + } + } +} + +fn main() { + baz::Item::TEST_A$0 +} +"#, + r#" +use foo::{bar::SomeTrait, baz}; + +mod foo { + pub mod bar { + pub trait SomeTrait { + const TEST_ASSOC: usize; + } + } + + pub mod baz { + use super::bar::SomeTrait; + + pub struct Item; + + impl SomeTrait for Item { + const TEST_ASSOC: usize = 3; + } + } +} + +fn main() { + baz::Item::TEST_ASSOC +} "#, ); } diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index df26e76426..4bddc3957d 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -13,7 +13,10 @@ mod builder_ext; use hir::{ AsAssocItem, Documentation, HasAttrs, HirDisplay, ModuleDef, Mutability, ScopeDef, Type, }; -use ide_db::{helpers::SnippetCap, RootDatabase, SymbolKind}; +use ide_db::{ + helpers::{item_name, SnippetCap}, + RootDatabase, SymbolKind, +}; use syntax::TextRange; use crate::{ @@ -56,7 +59,7 @@ pub(crate) fn render_resolution_with_import<'a>( ScopeDef::ModuleDef(ModuleDef::Function(f)) => f.name(ctx.completion.db).to_string(), ScopeDef::ModuleDef(ModuleDef::Const(c)) => c.name(ctx.completion.db)?.to_string(), ScopeDef::ModuleDef(ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db).to_string(), - _ => import_edit.import.display_path().segments().last()?.to_string(), + _ => item_name(ctx.db(), import_edit.import.item_to_display())?.to_string(), }; Render::new(ctx).render_resolution(local_name, Some(import_edit), resolution).map(|mut item| { item.completion_kind = CompletionKind::Magic; diff --git a/crates/ide_db/src/helpers.rs b/crates/ide_db/src/helpers.rs index 3ff77400bb..3c95d3cffa 100644 --- a/crates/ide_db/src/helpers.rs +++ b/crates/ide_db/src/helpers.rs @@ -2,11 +2,19 @@ pub mod insert_use; pub mod import_assets; -use hir::{Crate, Enum, Module, ScopeDef, Semantics, Trait}; +use hir::{Crate, Enum, ItemInNs, MacroDef, Module, ModuleDef, Name, ScopeDef, Semantics, Trait}; use syntax::ast::{self, make}; use crate::RootDatabase; +pub fn item_name(db: &RootDatabase, item: ItemInNs) -> Option { + match item { + ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).name(db), + ItemInNs::Values(module_def_id) => ModuleDef::from(module_def_id).name(db), + ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).name(db), + } +} + /// Converts the mod path struct into its ast representation. pub fn mod_path_to_ast(path: &hir::ModPath) -> ast::Path { let _p = profile::span("mod_path_to_ast"); diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index d8bf61aaaf..3d79f97711 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,8 +1,8 @@ //! Look up accessible paths for items. use either::Either; use hir::{ - AsAssocItem, AssocItem, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, Name, - PrefixKind, Semantics, + AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module, + ModuleDef, PathResolution, PrefixKind, Semantics, Type, }; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -12,6 +12,8 @@ use crate::{ RootDatabase, }; +use super::item_name; + #[derive(Debug)] pub enum ImportCandidate { // A path, qualified (`std::collections::HashMap`) or not (`HashMap`). @@ -28,7 +30,7 @@ pub enum ImportCandidate { #[derive(Debug)] pub struct TraitImportCandidate { - pub receiver_ty: hir::Type, + pub receiver_ty: Type, pub name: NameToImport, } @@ -62,7 +64,7 @@ impl NameToImport { #[derive(Debug)] pub struct ImportAssets { import_candidate: ImportCandidate, - module_with_candidate: hir::Module, + module_with_candidate: Module, } impl ImportAssets { @@ -104,7 +106,7 @@ impl ImportAssets { pub fn for_fuzzy_method_call( module_with_method_call: Module, - receiver_ty: hir::Type, + receiver_ty: Type, fuzzy_method_name: String, ) -> Option { Some(Self { @@ -184,7 +186,7 @@ impl ImportAssets { fn search_for( &self, sema: &Semantics, - prefixed: Option, + prefixed: Option, ) -> Vec { let current_crate = self.module_with_candidate.krate(); @@ -223,7 +225,7 @@ impl ImportAssets { fn applicable_defs( &self, db: &RootDatabase, - prefixed: Option, + prefixed: Option, unfiltered_defs: impl Iterator>, ) -> FxHashSet { let current_crate = self.module_with_candidate.krate(); @@ -266,10 +268,10 @@ fn path_applicable_imports( let (assoc_original, candidate) = match def { Either::Left(module_def) => match module_def.as_assoc_item(db) { Some(assoc_item) => match assoc_item.container(db) { - hir::AssocItemContainer::Trait(trait_) => { + AssocItemContainer::Trait(trait_) => { (Some(module_def), ItemInNs::from(ModuleDef::from(trait_))) } - hir::AssocItemContainer::Impl(impl_) => ( + AssocItemContainer::Impl(impl_) => ( Some(module_def), ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?)), ), @@ -296,6 +298,7 @@ fn path_applicable_imports( }; // TODO kb need to remove turbofish from the qualifier, maybe use the segments instead? + // TODO kb sorting is changed now, return back? let unresolved_qualifier_string = unresolved_qualifier.to_string(); let unresolved_first_segment_string = unresolved_first_segment.to_string(); @@ -305,38 +308,35 @@ fn path_applicable_imports( candidate_path_string.contains(&unresolved_qualifier_string) && candidate_path_string.contains(&unresolved_first_segment_string) }) - // TODO kb need to adjust the return type: I get the results rendered rather badly .filter_map(|(candidate_path, (assoc_original, candidate))| { - if let Some(assoc_original) = assoc_original { - if item_name(db, candidate)?.to_string() == unresolved_first_segment_string { - return Some(LocatedImport::new( - candidate_path.clone(), - ItemInNs::from(assoc_original), - Some((candidate_path, candidate)), - )); - } - } + let found_segment_resolution = item_name(db, candidate) + .map(|name| name.to_string() == unresolved_first_segment_string) + .unwrap_or(false); + let (import_path, item_to_import) = if found_segment_resolution { + (candidate_path.clone(), candidate) + } else { + let matching_module = + module_with_matching_name(db, &unresolved_first_segment_string, candidate)?; + let module_item = ItemInNs::from(ModuleDef::from(matching_module)); + (import_path_locator(module_item)?, module_item) + }; - let matching_module = - module_with_matching_name(db, &unresolved_first_segment_string, candidate)?; - let item = ItemInNs::from(ModuleDef::from(matching_module)); - Some(LocatedImport::new( - import_path_locator(item)?, - item, - Some((candidate_path, candidate)), - )) + Some(match assoc_original { + Some(assoc_original) => LocatedImport::new( + import_path.clone(), + item_to_import, + Some((import_path, ItemInNs::from(assoc_original))), + ), + None => LocatedImport::new( + import_path, + item_to_import, + if found_segment_resolution { None } else { Some((candidate_path, candidate)) }, + ), + }) }) .collect() } -fn item_name(db: &RootDatabase, item: ItemInNs) -> Option { - match item { - ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).name(db), - ItemInNs::Values(module_def_id) => ModuleDef::from(module_def_id).name(db), - ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).name(db), - } -} - fn item_module(db: &RootDatabase, item: ItemInNs) -> Option { match item { ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).module(db), @@ -404,10 +404,20 @@ fn trait_applicable_items( } let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?)); + let item_path = import_path_locator(item)?; + + let assoc_item = assoc_to_item(assoc); + let assoc_item_path = match assoc.container(db) { + AssocItemContainer::Trait(_) => item_path.clone(), + AssocItemContainer::Impl(impl_) => import_path_locator(ItemInNs::from( + ModuleDef::from(impl_.target_ty(db).as_adt()?), + ))?, + }; + located_imports.insert(LocatedImport::new( - import_path_locator(item)?, + item_path, item, - None, + Some((assoc_item_path, assoc_item)), )); } None::<()> @@ -423,10 +433,20 @@ 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 item_path = import_path_locator(item)?; + + let assoc_item = assoc_to_item(assoc); + let assoc_item_path = match assoc.container(db) { + AssocItemContainer::Trait(_) => item_path.clone(), + AssocItemContainer::Impl(impl_) => import_path_locator(ItemInNs::from( + ModuleDef::from(impl_.target_ty(db).as_adt()?), + ))?, + }; + located_imports.insert(LocatedImport::new( - import_path_locator(item)?, + item_path, item, - None, + Some((assoc_item_path, assoc_item)), )); } None::<()> @@ -437,11 +457,19 @@ fn trait_applicable_items( located_imports } +fn assoc_to_item(assoc: AssocItem) -> ItemInNs { + match assoc { + AssocItem::Function(f) => ItemInNs::from(ModuleDef::from(f)), + AssocItem::Const(c) => ItemInNs::from(ModuleDef::from(c)), + AssocItem::TypeAlias(t) => ItemInNs::from(ModuleDef::from(t)), + } +} + fn get_mod_path( db: &RootDatabase, item_to_search: ItemInNs, module_with_candidate: &Module, - prefixed: Option, + prefixed: Option, ) -> Option { if let Some(prefix_kind) = prefixed { module_with_candidate.find_use_path_prefixed(db, item_to_search, prefix_kind) @@ -509,7 +537,7 @@ fn path_import_candidate( return None; } } - Some(hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path))) => { + Some(PathResolution::Def(ModuleDef::Adt(assoc_item_path))) => { ImportCandidate::TraitAssocItem(TraitImportCandidate { receiver_ty: assoc_item_path.ty(sema.db), name, From 9482353fa8e1e88cb720a029b9bb6304819c7399 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 28 Feb 2021 09:57:54 +0200 Subject: [PATCH 08/24] Properly handle turbofishes in qualifiers --- crates/hir_def/src/path.rs | 4 ++++ crates/ide_assists/src/handlers/auto_import.rs | 2 +- crates/ide_assists/src/handlers/qualify_path.rs | 2 +- crates/ide_completion/src/completions/flyimport.rs | 2 +- crates/ide_db/src/helpers/import_assets.rs | 12 +++++++----- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs index 0e60dc2b63..1dc085199f 100644 --- a/crates/hir_def/src/path.rs +++ b/crates/hir_def/src/path.rs @@ -44,6 +44,10 @@ pub enum ImportAlias { } impl ModPath { + pub fn from_src_unhygienic(path: ast::Path) -> Option { + lower::lower_path(path, &Hygiene::new_unhygienic()).map(|it| it.mod_path) + } + pub fn from_src(path: ast::Path, hygiene: &Hygiene) -> Option { lower::lower_path(path, hygiene).map(|it| it.mod_path) } diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 7188724be7..e9993a7cc9 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -242,7 +242,7 @@ mod tests { } ", r" - use PubMod3::PubStruct; + use PubMod1::PubStruct; PubStruct diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index a40cdd80e0..2611784486 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -313,7 +313,7 @@ mod tests { } ", r" - PubMod3::PubStruct + PubMod1::PubStruct pub mod PubMod1 { pub struct PubStruct; diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index e33fc4b78a..af49fdd262 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -669,8 +669,8 @@ fn main() { } "#, expect![[r#" - ct SPECIAL_CONST (dep::test_mod::TestTrait) DEPRECATED fn weird_function() (dep::test_mod::TestTrait) -> () DEPRECATED + ct SPECIAL_CONST (dep::test_mod::TestTrait) DEPRECATED "#]], ); } diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 3d79f97711..2e7a183d1c 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -43,7 +43,7 @@ pub struct PathImportCandidate { #[derive(Debug)] pub enum Qualifier { Absent, - FirstSegmentUnresolved(ast::PathSegment, ast::Path), + FirstSegmentUnresolved(ast::NameRef, ModPath), } #[derive(Debug)] @@ -297,8 +297,7 @@ fn path_applicable_imports( Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier), }; - // TODO kb need to remove turbofish from the qualifier, maybe use the segments instead? - // TODO kb sorting is changed now, return back? + // TODO kb zz.syntax().ast_node() <- two options are now proposed despite the trait being imported let unresolved_qualifier_string = unresolved_qualifier.to_string(); let unresolved_first_segment_string = unresolved_first_segment.to_string(); @@ -525,12 +524,15 @@ fn path_import_candidate( Some(qualifier) => match sema.resolve_path(&qualifier) { None => { let qualifier_start = - qualifier.syntax().descendants().find_map(ast::PathSegment::cast)?; + qualifier.syntax().descendants().find_map(ast::NameRef::cast)?; let qualifier_start_path = qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; if sema.resolve_path(&qualifier_start_path).is_none() { ImportCandidate::Path(PathImportCandidate { - qualifier: Qualifier::FirstSegmentUnresolved(qualifier_start, qualifier), + qualifier: Qualifier::FirstSegmentUnresolved( + qualifier_start, + ModPath::from_src_unhygienic(qualifier)?, + ), name, }) } else { From 89d410cef571f5fa7631b17e2fbe52a8f8f03990 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 28 Feb 2021 10:32:15 +0200 Subject: [PATCH 09/24] Do not propose already imported imports --- crates/hir/src/semantics.rs | 2 +- .../ide_assists/src/handlers/auto_import.rs | 4 +- .../src/completions/flyimport.rs | 23 +++------- crates/ide_db/src/helpers/import_assets.rs | 43 +++++++++++++++---- 4 files changed, 45 insertions(+), 27 deletions(-) diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 945638cc56..69370ef3d8 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -774,7 +774,7 @@ fn find_root(node: &SyntaxNode) -> SyntaxNode { /// /// Note that if you are wondering "what does this specific existing name mean?", /// you'd better use the `resolve_` family of methods. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct SemanticsScope<'a> { pub db: &'a dyn HirDatabase, file_id: HirFileId, diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index e9993a7cc9..1825475893 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -111,7 +111,9 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> Some(()) } -pub(super) fn find_importable_node(ctx: &AssistContext) -> Option<(ImportAssets, SyntaxNode)> { +pub(super) fn find_importable_node<'a>( + ctx: &'a AssistContext, +) -> Option<(ImportAssets<'a>, SyntaxNode)> { if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::() { ImportAssets::for_exact_path(&path_under_caret, &ctx.sema) .zip(Some(path_under_caret.syntax().clone())) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index af49fdd262..d6adf70b13 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -48,12 +48,11 @@ //! Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corredponding //! capability enabled. -use hir::{AsAssocItem, ModPath, ScopeDef}; +use hir::{AsAssocItem, ItemInNs, ModPath, ScopeDef}; use ide_db::helpers::{ import_assets::{ImportAssets, ImportCandidate}, insert_use::ImportScope, }; -use rustc_hash::FxHashSet; use syntax::{AstNode, SyntaxNode, T}; use crate::{ @@ -92,19 +91,17 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) &ctx.sema, )?; - let scope_definitions = scope_definitions(ctx); let mut all_mod_paths = import_assets .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) .into_iter() .map(|import| { let proposed_def = match import.item_to_display() { - hir::ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), - hir::ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), - hir::ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), + ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), }; (import, proposed_def) }) - .filter(|(_, proposed_def)| !scope_definitions.contains(proposed_def)) .collect::>(); all_mod_paths.sort_by_cached_key(|(import, _)| { compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased) @@ -125,14 +122,6 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) Some(()) } -fn scope_definitions(ctx: &CompletionContext) -> FxHashSet { - let mut scope_definitions = FxHashSet::default(); - ctx.scope.process_all_names(&mut |_, scope_def| { - scope_definitions.insert(scope_def); - }); - scope_definitions -} - pub(crate) fn position_for_import<'a>( ctx: &'a CompletionContext, import_candidate: Option<&ImportCandidate>, @@ -150,13 +139,14 @@ pub(crate) fn position_for_import<'a>( }) } -fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option { +fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option> { let current_module = ctx.scope.module()?; if let Some(dot_receiver) = &ctx.dot_receiver { ImportAssets::for_fuzzy_method_call( current_module, ctx.sema.type_of_expr(dot_receiver)?, fuzzy_name, + ctx.scope.clone(), ) } else { let fuzzy_name_length = fuzzy_name.len(); @@ -165,6 +155,7 @@ fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option { import_candidate: ImportCandidate, module_with_candidate: Module, + scope: SemanticsScope<'a>, } -impl ImportAssets { +impl<'a> ImportAssets<'a> { pub fn for_method_call( method_call: &ast::MethodCallExpr, - sema: &Semantics, + sema: &'a Semantics, ) -> Option { + let scope = sema.scope(method_call.syntax()); Some(Self { import_candidate: ImportCandidate::for_method_call(sema, method_call)?, - module_with_candidate: sema.scope(method_call.syntax()).module()?, + module_with_candidate: scope.module()?, + scope, }) } pub fn for_exact_path( fully_qualified_path: &ast::Path, - sema: &Semantics, + sema: &'a Semantics, ) -> Option { let syntax_under_caret = fully_qualified_path.syntax(); if syntax_under_caret.ancestors().find_map(ast::Use::cast).is_some() { return None; } + let scope = sema.scope(syntax_under_caret); Some(Self { import_candidate: ImportCandidate::for_regular_path(sema, fully_qualified_path)?, - module_with_candidate: sema.scope(syntax_under_caret).module()?, + module_with_candidate: scope.module()?, + scope, }) } @@ -97,10 +102,12 @@ impl ImportAssets { qualifier: Option, fuzzy_name: String, sema: &Semantics, + scope: SemanticsScope<'a>, ) -> Option { Some(Self { import_candidate: ImportCandidate::for_fuzzy_path(qualifier, fuzzy_name, sema)?, module_with_candidate, + scope, }) } @@ -108,6 +115,7 @@ impl ImportAssets { module_with_method_call: Module, receiver_ty: Type, fuzzy_method_name: String, + scope: SemanticsScope<'a>, ) -> Option { Some(Self { import_candidate: ImportCandidate::TraitMethod(TraitImportCandidate { @@ -115,6 +123,7 @@ impl ImportAssets { name: NameToImport::Fuzzy(fuzzy_method_name), }), module_with_candidate: module_with_method_call, + scope, }) } } @@ -155,7 +164,7 @@ impl LocatedImport { } } -impl ImportAssets { +impl<'a> ImportAssets<'a> { pub fn import_candidate(&self) -> &ImportCandidate { &self.import_candidate } @@ -189,6 +198,7 @@ impl ImportAssets { prefixed: Option, ) -> Vec { let current_crate = self.module_with_candidate.krate(); + let scope_definitions = self.scope_definitions(); let imports_for_candidate_name = match self.name_to_import() { NameToImport::Exact(exact_name) => { @@ -219,9 +229,25 @@ impl ImportAssets { self.applicable_defs(sema.db, prefixed, imports_for_candidate_name) .into_iter() .filter(|import| import.import_path().len() > 1) + .filter(|import| { + let proposed_def = match import.item_to_import() { + ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), + }; + !scope_definitions.contains(&proposed_def) + }) .collect() } + fn scope_definitions(&self) -> FxHashSet { + let mut scope_definitions = FxHashSet::default(); + self.scope.process_all_names(&mut |_, scope_def| { + scope_definitions.insert(scope_def); + }); + scope_definitions + } + fn applicable_defs( &self, db: &RootDatabase, @@ -297,7 +323,6 @@ fn path_applicable_imports( Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier), }; - // TODO kb zz.syntax().ast_node() <- two options are now proposed despite the trait being imported let unresolved_qualifier_string = unresolved_qualifier.to_string(); let unresolved_first_segment_string = unresolved_first_segment.to_string(); From e74c55bb4adcad001b0f7373ebff795fc2aaeb1b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 1 Mar 2021 00:05:22 +0200 Subject: [PATCH 10/24] Refactor the import location --- crates/hir/src/lib.rs | 11 + .../src/completions/flyimport.rs | 65 +----- crates/ide_db/src/helpers/import_assets.rs | 218 +++++++++--------- 3 files changed, 129 insertions(+), 165 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 62692c2c18..c4691d34c2 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1115,6 +1115,7 @@ pub enum AssocItem { Const(Const), TypeAlias(TypeAlias), } +#[derive(Debug)] pub enum AssocItemContainer { Trait(Trait), Impl(Impl), @@ -2148,6 +2149,16 @@ impl ScopeDef { } } +impl From for ScopeDef { + fn from(item: ItemInNs) -> Self { + match item { + ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), + ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), + } + } +} + pub trait HasVisibility { fn visibility(&self, db: &dyn HirDatabase) -> Visibility; fn is_visible_from(&self, db: &dyn HirDatabase, module: Module) -> bool { diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index d6adf70b13..55439d0e59 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -95,20 +95,20 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) .into_iter() .map(|import| { - let proposed_def = match import.item_to_display() { + let def_to_display = match import.item_to_display() { ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), }; - (import, proposed_def) + (import, def_to_display) }) .collect::>(); all_mod_paths.sort_by_cached_key(|(import, _)| { compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased) }); - acc.add_all(all_mod_paths.into_iter().filter_map(|(import, definition)| { - let import_for_trait_assoc_item = match definition { + acc.add_all(all_mod_paths.into_iter().filter_map(|(import, def_to_display)| { + let import_for_trait_assoc_item = match def_to_display { ScopeDef::ModuleDef(module_def) => module_def .as_assoc_item(ctx.db) .and_then(|assoc| assoc.containing_trait(ctx.db)) @@ -117,7 +117,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) }; let import_edit = ImportEdit { import, import_scope: import_scope.clone(), import_for_trait_assoc_item }; - render_resolution_with_import(RenderContext::new(ctx), import_edit, &definition) + render_resolution_with_import(RenderContext::new(ctx), import_edit, &def_to_display) })); Some(()) } @@ -867,61 +867,6 @@ mod foo { fn main() { bar::Item::TEST_ASSOC } -"#, - ); - } - - #[test] - fn unresolved_assoc_item_container_and_trait_with_path() { - check_edit( - "TEST_ASSOC", - r#" -mod foo { - pub mod bar { - pub trait SomeTrait { - const TEST_ASSOC: usize; - } - } - - pub mod baz { - use super::bar::SomeTrait; - - pub struct Item; - - impl SomeTrait for Item { - const TEST_ASSOC: usize = 3; - } - } -} - -fn main() { - baz::Item::TEST_A$0 -} -"#, - r#" -use foo::{bar::SomeTrait, baz}; - -mod foo { - pub mod bar { - pub trait SomeTrait { - const TEST_ASSOC: usize; - } - } - - pub mod baz { - use super::bar::SomeTrait; - - pub struct Item; - - impl SomeTrait for Item { - const TEST_ASSOC: usize = 3; - } - } -} - -fn main() { - baz::Item::TEST_ASSOC -} "#, ); } diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index b25786928e..2909ecd450 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -130,25 +130,23 @@ impl<'a> ImportAssets<'a> { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct LocatedImport { + // TODO kb extract both into a separate struct + add another field: `assoc_item_name: Optional` import_path: ModPath, item_to_import: ItemInNs, - import_display_override: Option<(ModPath, ItemInNs)>, + data_to_display: Option<(ModPath, ItemInNs)>, } impl LocatedImport { pub fn new( import_path: ModPath, item_to_import: ItemInNs, - import_display_override: Option<(ModPath, ItemInNs)>, + data_to_display: Option<(ModPath, ItemInNs)>, ) -> Self { - Self { import_path, item_to_import, import_display_override } + Self { import_path, item_to_import, data_to_display } } pub fn display_path(&self) -> &ModPath { - self.import_display_override - .as_ref() - .map(|(mod_path, _)| mod_path) - .unwrap_or(&self.import_path) + self.data_to_display.as_ref().map(|(mod_pathh, _)| mod_pathh).unwrap_or(&self.import_path) } pub fn import_path(&self) -> &ModPath { @@ -156,7 +154,7 @@ impl LocatedImport { } pub fn item_to_display(&self) -> ItemInNs { - self.import_display_override.as_ref().map(|&(_, item)| item).unwrap_or(self.item_to_import) + self.data_to_display.as_ref().map(|&(_, item)| item).unwrap_or(self.item_to_import) } pub fn item_to_import(&self) -> ItemInNs { @@ -200,7 +198,7 @@ impl<'a> ImportAssets<'a> { let current_crate = self.module_with_candidate.krate(); let scope_definitions = self.scope_definitions(); - let imports_for_candidate_name = match self.name_to_import() { + let defs_for_candidate_name = match self.name_to_import() { NameToImport::Exact(exact_name) => { imports_locator::find_exact_imports(sema, current_crate, exact_name.clone()) } @@ -226,7 +224,7 @@ impl<'a> ImportAssets<'a> { } }; - self.applicable_defs(sema.db, prefixed, imports_for_candidate_name) + self.applicable_defs(sema.db, prefixed, defs_for_candidate_name) .into_iter() .filter(|import| import.import_path().len() > 1) .filter(|import| { @@ -252,32 +250,31 @@ impl<'a> ImportAssets<'a> { &self, db: &RootDatabase, prefixed: Option, - unfiltered_defs: impl Iterator>, + defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { let current_crate = self.module_with_candidate.krate(); - let import_path_locator = - |item| get_mod_path(db, item, &self.module_with_candidate, prefixed); + let mod_path = |item| get_mod_path(db, item, &self.module_with_candidate, prefixed); match &self.import_candidate { ImportCandidate::Path(path_candidate) => { - path_applicable_imports(db, path_candidate, import_path_locator, unfiltered_defs) + path_applicable_imports(db, path_candidate, mod_path, defs_for_candidate_name) } ImportCandidate::TraitAssocItem(trait_candidate) => trait_applicable_items( db, current_crate, trait_candidate, true, - import_path_locator, - unfiltered_defs, + mod_path, + defs_for_candidate_name, ), ImportCandidate::TraitMethod(trait_candidate) => trait_applicable_items( db, current_crate, trait_candidate, false, - import_path_locator, - unfiltered_defs, + mod_path, + defs_for_candidate_name, ), } } @@ -286,103 +283,114 @@ impl<'a> ImportAssets<'a> { fn path_applicable_imports( db: &RootDatabase, path_candidate: &PathImportCandidate, - import_path_locator: impl Fn(ItemInNs) -> Option, - unfiltered_defs: impl Iterator>, + mod_path: impl Fn(ItemInNs) -> Option + Copy, + defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { - let applicable_items = unfiltered_defs - .filter_map(|def| { - let (assoc_original, candidate) = match def { - Either::Left(module_def) => match module_def.as_assoc_item(db) { - Some(assoc_item) => match assoc_item.container(db) { - AssocItemContainer::Trait(trait_) => { - (Some(module_def), ItemInNs::from(ModuleDef::from(trait_))) - } - AssocItemContainer::Impl(impl_) => ( - Some(module_def), - ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?)), - ), - }, - None => (None, ItemInNs::from(module_def)), - }, - Either::Right(macro_def) => (None, ItemInNs::from(macro_def)), - }; - Some((assoc_original, candidate)) - }) - .filter_map(|(assoc_original, candidate)| { - import_path_locator(candidate).zip(Some((assoc_original, candidate))) - }); + let items_for_candidate_name = + defs_for_candidate_name.map(|def| def.either(ItemInNs::from, ItemInNs::from)); let (unresolved_first_segment, unresolved_qualifier) = match &path_candidate.qualifier { Qualifier::Absent => { - return applicable_items - .map(|(candidate_path, (_, candidate))| { - LocatedImport::new(candidate_path, candidate, None) - }) + return items_for_candidate_name + .filter_map(|item| Some(LocatedImport::new(mod_path(item)?, item, None))) .collect(); } - Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier), + Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => { + (first_segment.to_string(), qualifier.to_string()) + } }; - let unresolved_qualifier_string = unresolved_qualifier.to_string(); - let unresolved_first_segment_string = unresolved_first_segment.to_string(); - - applicable_items - .filter(|(candidate_path, _)| { - let candidate_path_string = candidate_path.to_string(); - candidate_path_string.contains(&unresolved_qualifier_string) - && candidate_path_string.contains(&unresolved_first_segment_string) - }) - .filter_map(|(candidate_path, (assoc_original, candidate))| { - let found_segment_resolution = item_name(db, candidate) - .map(|name| name.to_string() == unresolved_first_segment_string) - .unwrap_or(false); - let (import_path, item_to_import) = if found_segment_resolution { - (candidate_path.clone(), candidate) - } else { - let matching_module = - module_with_matching_name(db, &unresolved_first_segment_string, candidate)?; - let module_item = ItemInNs::from(ModuleDef::from(matching_module)); - (import_path_locator(module_item)?, module_item) - }; - - Some(match assoc_original { - Some(assoc_original) => LocatedImport::new( - import_path.clone(), - item_to_import, - Some((import_path, ItemInNs::from(assoc_original))), - ), - None => LocatedImport::new( - import_path, - item_to_import, - if found_segment_resolution { None } else { Some((candidate_path, candidate)) }, - ), - }) + items_for_candidate_name + .filter_map(|item| { + import_for_item(db, mod_path, &unresolved_first_segment, &unresolved_qualifier, item) }) .collect() } -fn item_module(db: &RootDatabase, item: ItemInNs) -> Option { - match item { +fn import_for_item( + db: &RootDatabase, + mod_path: impl Fn(ItemInNs) -> Option, + unresolved_first_segment: &str, + unresolved_qualifier: &str, + original_item: ItemInNs, +) -> Option { + let (item_candidate, trait_to_import) = match original_item { + ItemInNs::Types(module_def_id) | ItemInNs::Values(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), + } + } + ItemInNs::Macros(_) => (original_item, None), + }; + let import_path_candidate = mod_path(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)?; + let data_to_display = Some((import_path_candidate.clone(), original_item)); + Some(match (segment_import == item_candidate, trait_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, data_to_display) + } + (true, None) => LocatedImport::new(import_path_candidate, item_candidate, data_to_display), + (false, None) => { + LocatedImport::new(mod_path(segment_import)?, segment_import, data_to_display) + } + }) +} + +fn find_import_for_segment( + db: &RootDatabase, + original_item: ItemInNs, + unresolved_first_segment: &str, +) -> Option { + let segment_is_name = item_name(db, original_item) + .map(|name| name.to_string() == unresolved_first_segment) + .unwrap_or(false); + + Some(if segment_is_name { + original_item + } else { + let matching_module = + module_with_segment_name(db, &unresolved_first_segment, original_item)?; + ItemInNs::from(ModuleDef::from(matching_module)) + }) +} + +fn module_with_segment_name( + db: &RootDatabase, + segment_name: &str, + candidate: ItemInNs, +) -> Option { + let mut current_module = match candidate { ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).module(db), ItemInNs::Values(module_def_id) => ModuleDef::from(module_def_id).module(db), ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).module(db), - } -} - -fn module_with_matching_name( - db: &RootDatabase, - unresolved_first_segment_string: &str, - candidate: ItemInNs, -) -> Option { - let mut current_module = item_module(db, candidate); + }; while let Some(module) = current_module { - match module.name(db) { - Some(module_name) => { - if module_name.to_string().as_str() == unresolved_first_segment_string { - return Some(module); - } + if let Some(module_name) = module.name(db) { + if module_name.to_string() == segment_name { + return Some(module); } - None => {} } current_module = module.parent(db); } @@ -394,12 +402,12 @@ fn trait_applicable_items( current_crate: Crate, trait_candidate: &TraitImportCandidate, trait_assoc_item: bool, - import_path_locator: impl Fn(ItemInNs) -> Option, - unfiltered_defs: impl Iterator>, + mod_path: impl Fn(ItemInNs) -> Option, + defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { let mut required_assoc_items = FxHashSet::default(); - let trait_candidates = unfiltered_defs + let trait_candidates = defs_for_candidate_name .filter_map(|input| match input { Either::Left(module_def) => module_def.as_assoc_item(db), _ => None, @@ -428,12 +436,12 @@ fn trait_applicable_items( } let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?)); - let item_path = import_path_locator(item)?; + let item_path = mod_path(item)?; let assoc_item = assoc_to_item(assoc); let assoc_item_path = match assoc.container(db) { AssocItemContainer::Trait(_) => item_path.clone(), - AssocItemContainer::Impl(impl_) => import_path_locator(ItemInNs::from( + AssocItemContainer::Impl(impl_) => mod_path(ItemInNs::from( ModuleDef::from(impl_.target_ty(db).as_adt()?), ))?, }; @@ -457,12 +465,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 item_path = import_path_locator(item)?; + let item_path = mod_path(item)?; let assoc_item = assoc_to_item(assoc); let assoc_item_path = match assoc.container(db) { AssocItemContainer::Trait(_) => item_path.clone(), - AssocItemContainer::Impl(impl_) => import_path_locator(ItemInNs::from( + AssocItemContainer::Impl(impl_) => mod_path(ItemInNs::from( ModuleDef::from(impl_.target_ty(db).as_adt()?), ))?, }; From e214c3a6bd7f74b42d38663b959fc4f0d113c90c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 1 Mar 2021 13:55:47 +0200 Subject: [PATCH 11/24] Simplify --- .../src/completions/flyimport.rs | 43 ++++++++----------- crates/ide_db/src/helpers/import_assets.rs | 18 +++----- 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 55439d0e59..c6b83da3d1 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -48,7 +48,7 @@ //! Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corredponding //! capability enabled. -use hir::{AsAssocItem, ItemInNs, ModPath, ScopeDef}; +use hir::{AsAssocItem, ModPath, ModuleDef, ScopeDef}; use ide_db::helpers::{ import_assets::{ImportAssets, ImportCandidate}, insert_use::ImportScope, @@ -91,33 +91,26 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) &ctx.sema, )?; - let mut all_mod_paths = import_assets - .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) - .into_iter() - .map(|import| { - let def_to_display = match import.item_to_display() { - ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), - ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), - ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), - }; - (import, def_to_display) - }) - .collect::>(); - all_mod_paths.sort_by_cached_key(|(import, _)| { + let mut all_imports = + import_assets.search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind); + all_imports.sort_by_cached_key(|import| { compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased) }); - acc.add_all(all_mod_paths.into_iter().filter_map(|(import, def_to_display)| { - let import_for_trait_assoc_item = match def_to_display { - ScopeDef::ModuleDef(module_def) => module_def - .as_assoc_item(ctx.db) - .and_then(|assoc| assoc.containing_trait(ctx.db)) - .is_some(), - _ => false, - }; - let import_edit = - ImportEdit { import, import_scope: import_scope.clone(), import_for_trait_assoc_item }; - render_resolution_with_import(RenderContext::new(ctx), import_edit, &def_to_display) + acc.add_all(all_imports.into_iter().filter_map(|import| { + let import_for_trait_assoc_item = import + .item_to_display() + .as_module_def_id() + .and_then(|module_def_id| { + ModuleDef::from(module_def_id).as_assoc_item(ctx.db)?.containing_trait(ctx.db) + }) + .is_some(); + let def_to_display = ScopeDef::from(import.item_to_display()); + render_resolution_with_import( + RenderContext::new(ctx), + ImportEdit { import, import_scope: import_scope.clone(), import_for_trait_assoc_item }, + &def_to_display, + ) })); Some(()) } diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 2909ecd450..4e352d546f 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -130,7 +130,6 @@ impl<'a> ImportAssets<'a> { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct LocatedImport { - // TODO kb extract both into a separate struct + add another field: `assoc_item_name: Optional` import_path: ModPath, item_to_import: ItemInNs, data_to_display: Option<(ModPath, ItemInNs)>, @@ -146,7 +145,7 @@ impl LocatedImport { } pub fn display_path(&self) -> &ModPath { - self.data_to_display.as_ref().map(|(mod_pathh, _)| mod_pathh).unwrap_or(&self.import_path) + self.data_to_display.as_ref().map(|(mod_path, _)| mod_path).unwrap_or(&self.import_path) } pub fn import_path(&self) -> &ModPath { @@ -227,14 +226,7 @@ impl<'a> ImportAssets<'a> { self.applicable_defs(sema.db, prefixed, defs_for_candidate_name) .into_iter() .filter(|import| import.import_path().len() > 1) - .filter(|import| { - let proposed_def = match import.item_to_import() { - ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()), - ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()), - ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()), - }; - !scope_definitions.contains(&proposed_def) - }) + .filter(|import| !scope_definitions.contains(&ScopeDef::from(import.item_to_import()))) .collect() } @@ -314,8 +306,8 @@ fn import_for_item( unresolved_qualifier: &str, original_item: ItemInNs, ) -> Option { - let (item_candidate, trait_to_import) = match original_item { - ItemInNs::Types(module_def_id) | ItemInNs::Values(module_def_id) => { + 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_)) => { @@ -328,7 +320,7 @@ fn import_for_item( None => (original_item, None), } } - ItemInNs::Macros(_) => (original_item, None), + None => (original_item, None), }; let import_path_candidate = mod_path(item_candidate)?; From 821e8369d994ebca39ccbf9b449c604d0c910efc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 1 Mar 2021 14:14:24 +0200 Subject: [PATCH 12/24] Update the docs --- .../src/completions/flyimport.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index c6b83da3d1..1ef6f8afbc 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -21,6 +21,45 @@ //! ``` //! //! Also completes associated items, that require trait imports. +//! If any unresolved and/or partially-qualified path predeces the input, it will be taken into account: only the items with import string +//! containing this whole path will be considered and the corresponding path import will be added: +//! +//! ``` +//! mod foo { +//! pub mod bar { +//! pub struct Item; +//! +//! impl Item { +//! pub const TEST_ASSOC: usize = 3; +//! } +//! } +//! } +//! +//! fn main() { +//! bar::Item::TEST_A$0 +//! } +//! ``` +//! -> +//! ``` +//! use foo::bar; +//! +//! mod foo { +//! pub mod bar { +//! pub struct Item; +//! +//! impl Item { +//! pub const TEST_ASSOC: usize = 3; +//! } +//! } +//! } +//! +//! fn main() { +//! bar::Item::TEST_ASSOC +//! } +//! ``` +//! +//! NOTE: currently, if an assoc item comes from a trait that's not currently imported and it also has an unresolved and/or partially-qualified path, +//! no imports will be proposed. //! //! .Fuzzy search details //! From 4d4ac1d4fa0aba107a27d3fd2d209304dfe69b9f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 1 Mar 2021 14:27:07 +0200 Subject: [PATCH 13/24] Profile import_assets better --- crates/ide_db/src/helpers/import_assets.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 4e352d546f..a30a4dd9de 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -244,6 +244,7 @@ impl<'a> ImportAssets<'a> { prefixed: Option, defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { + 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); @@ -278,6 +279,8 @@ fn path_applicable_imports( mod_path: impl Fn(ItemInNs) -> Option + Copy, defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { + let _p = profile::span("import_assets::path_applicable_imports"); + let items_for_candidate_name = defs_for_candidate_name.map(|def| def.either(ItemInNs::from, ItemInNs::from)); @@ -306,6 +309,7 @@ fn import_for_item( unresolved_qualifier: &str, 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)) @@ -397,6 +401,7 @@ fn trait_applicable_items( mod_path: impl Fn(ItemInNs) -> Option, defs_for_candidate_name: impl Iterator>, ) -> FxHashSet { + let _p = profile::span("import_assets::trait_applicable_items"); let mut required_assoc_items = FxHashSet::default(); let trait_candidates = defs_for_candidate_name From 33c83e72b9b48177a6171fd06a26676679963a4d Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 3 Mar 2021 01:26:53 +0200 Subject: [PATCH 14/24] Work towards better import labels --- crates/hir_def/src/import_map.rs | 23 +++ crates/ide/src/lib.rs | 2 - .../ide_assists/src/handlers/auto_import.rs | 13 +- .../ide_assists/src/handlers/qualify_path.rs | 22 ++- .../replace_derive_with_manual_impl.rs | 10 +- .../src/completions/flyimport.rs | 101 ++++++------ crates/ide_completion/src/item.rs | 41 ++--- crates/ide_completion/src/lib.rs | 17 +- crates/ide_completion/src/render.rs | 14 +- crates/ide_db/src/helpers/import_assets.rs | 155 +++++++----------- .../{imports_locator.rs => items_locator.rs} | 75 +++++---- crates/ide_db/src/lib.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 5 +- 13 files changed, 242 insertions(+), 238 deletions(-) rename crates/ide_db/src/{imports_locator.rs => items_locator.rs} (62%) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 369bc3350b..07ee7bdfd7 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -1094,4 +1094,27 @@ mod tests { expect![[r#""#]], ); } + + #[test] + fn search_with_path() { + check_search( + r#" +//- /main.rs crate:main deps:dep +//- /dep.rs crate:dep +pub mod foo { + pub mod bar { + pub mod baz { + pub trait Display { + fn fmt(); + } + } + } +}"#, + "main", + Query::new("baz::fmt".to_string()).search_mode(SearchMode::Fuzzy), + expect![[r#" + dep::foo::bar::baz::Display::fmt (a) + "#]], + ); + } } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index b600178ee1..f83ed65d5a 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -478,7 +478,6 @@ impl Analysis { position: FilePosition, full_import_path: &str, imported_name: String, - import_for_trait_assoc_item: bool, ) -> Cancelable> { Ok(self .with_db(|db| { @@ -488,7 +487,6 @@ impl Analysis { position, full_import_path, imported_name, - import_for_trait_assoc_item, ) })? .unwrap_or_default()) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 1825475893..f3c969eeec 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -93,17 +93,18 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let group = import_group_message(import_assets.import_candidate()); let scope = ImportScope::find_insert_use_container(&syntax_under_caret, &ctx.sema)?; for import in proposed_imports { + let name = match import.original_item_name(ctx.db()) { + Some(name) => name, + None => continue, + }; acc.add_group( &group, AssistId("auto_import", AssistKind::QuickFix), - format!("Import `{}`", import.display_path()), + format!("Import `{}`", name), range, |builder| { - let rewriter = insert_use( - &scope, - mod_path_to_ast(import.import_path()), - ctx.config.insert_use, - ); + let rewriter = + insert_use(&scope, mod_path_to_ast(&import.import_path), ctx.config.insert_use); builder.rewrite(rewriter); }, ); diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index 2611784486..407ba47bea 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -2,7 +2,7 @@ use std::iter; use hir::AsAssocItem; use ide_db::helpers::{ - import_assets::{ImportCandidate, Qualifier}, + import_assets::{ImportCandidate, LocatedImport, Qualifier}, mod_path_to_ast, }; use ide_db::RootDatabase; @@ -78,13 +78,13 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> acc.add_group( &group_label, AssistId("qualify_path", AssistKind::QuickFix), - label(candidate, import.display_path()), + label(ctx.db(), candidate, &import), range, |builder| { qualify_candidate.qualify( |replace_with: String| builder.replace(range, replace_with), - import.import_path(), - import.item_to_import(), + &import.import_path, + import.item_to_import, ) }, ); @@ -197,17 +197,21 @@ fn group_label(candidate: &ImportCandidate) -> GroupLabel { GroupLabel(format!("Qualify {}", name)) } -fn label(candidate: &ImportCandidate, import: &hir::ModPath) -> String { +fn label(db: &RootDatabase, candidate: &ImportCandidate, import: &LocatedImport) -> String { + let display_path = match import.original_item_name(db) { + Some(display_path) => display_path.to_string(), + None => "{unknown}".to_string(), + }; match candidate { ImportCandidate::Path(candidate) => { if !matches!(candidate.qualifier, Qualifier::Absent) { - format!("Qualify with `{}`", &import) + format!("Qualify with `{}`", display_path) } else { - format!("Qualify as `{}`", &import) + format!("Qualify as `{}`", display_path) } } - ImportCandidate::TraitAssocItem(_) => format!("Qualify `{}`", &import), - ImportCandidate::TraitMethod(_) => format!("Qualify with cast as `{}`", &import), + ImportCandidate::TraitAssocItem(_) => format!("Qualify `{}`", display_path), + ImportCandidate::TraitMethod(_) => format!("Qualify with cast as `{}`", display_path), } } diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index c69bc5cacd..93a03e8b25 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs @@ -1,5 +1,6 @@ +use hir::ModuleDef; use ide_db::helpers::mod_path_to_ast; -use ide_db::imports_locator; +use ide_db::items_locator; use itertools::Itertools; use syntax::{ ast::{self, make, AstNode, NameOwner}, @@ -64,13 +65,14 @@ pub(crate) fn replace_derive_with_manual_impl( let current_module = ctx.sema.scope(annotated_name.syntax()).module()?; let current_crate = current_module.krate(); - let found_traits = imports_locator::find_exact_imports( + let found_traits = items_locator::with_for_exact_name( &ctx.sema, current_crate, trait_token.text().to_string(), ) - .filter_map(|candidate: either::Either| match candidate { - either::Either::Left(hir::ModuleDef::Trait(trait_)) => Some(trait_), + .into_iter() + .filter_map(|item| match ModuleDef::from(item.as_module_def_id()?) { + ModuleDef::Trait(trait_) => Some(trait_), _ => None, }) .flat_map(|trait_| { diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 1ef6f8afbc..c1e3f091fb 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -87,11 +87,12 @@ //! Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corredponding //! capability enabled. -use hir::{AsAssocItem, ModPath, ModuleDef, ScopeDef}; +use hir::ModPath; use ide_db::helpers::{ import_assets::{ImportAssets, ImportCandidate}, insert_use::ImportScope, }; +use itertools::Itertools; use syntax::{AstNode, SyntaxNode, T}; use crate::{ @@ -130,27 +131,23 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) &ctx.sema, )?; - let mut all_imports = - import_assets.search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind); - all_imports.sort_by_cached_key(|import| { - compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased) - }); - - acc.add_all(all_imports.into_iter().filter_map(|import| { - let import_for_trait_assoc_item = import - .item_to_display() - .as_module_def_id() - .and_then(|module_def_id| { - ModuleDef::from(module_def_id).as_assoc_item(ctx.db)?.containing_trait(ctx.db) + acc.add_all( + import_assets + .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind) + .into_iter() + .sorted_by_key(|located_import| { + compute_fuzzy_completion_order_key( + &located_import.import_path, + &user_input_lowercased, + ) }) - .is_some(); - let def_to_display = ScopeDef::from(import.item_to_display()); - render_resolution_with_import( - RenderContext::new(ctx), - ImportEdit { import, import_scope: import_scope.clone(), import_for_trait_assoc_item }, - &def_to_display, - ) - })); + .filter_map(|import| { + render_resolution_with_import( + RenderContext::new(ctx), + ImportEdit { import, import_scope: import_scope.clone() }, + ) + }), + ); Some(()) } @@ -190,6 +187,7 @@ fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option, label: String, insert_text: Option, @@ -322,19 +322,22 @@ impl Builder { pub(crate) fn build(self) -> CompletionItem { let _p = profile::span("item::Builder::build"); - let mut label = self.label; - let mut lookup = self.lookup; - let mut insert_text = self.insert_text; + 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() { - 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_for_trait_assoc_item { - label = format!("{} ({})", label, display_path); - } else { - label = display_path.to_string(); - } + 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 text_edit = match self.text_edit { @@ -438,8 +441,8 @@ impl Builder { } } -impl<'a> Into for Builder { - fn into(self) -> CompletionItem { - self.build() - } -} +// impl<'a> Into for Builder { +// fn into(self) -> CompletionItem { +// self.build() +// } +// } diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index ca2e5e706a..d19368de09 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -15,7 +15,7 @@ use completions::flyimport::position_for_import; use ide_db::{ base_db::FilePosition, helpers::{import_assets::LocatedImport, insert_use::ImportScope}, - imports_locator, RootDatabase, + items_locator, RootDatabase, }; use text_edit::TextEdit; @@ -141,7 +141,6 @@ pub fn resolve_completion_edits( position: FilePosition, full_import_path: &str, imported_name: String, - import_for_trait_assoc_item: bool, ) -> Option> { let ctx = CompletionContext::new(db, position, config)?; let position_for_import = position_for_import(&ctx, None)?; @@ -151,19 +150,17 @@ pub fn resolve_completion_edits( let current_crate = current_module.krate(); let (import_path, item_to_import) = - imports_locator::find_exact_imports(&ctx.sema, current_crate, imported_name) + items_locator::with_for_exact_name(&ctx.sema, current_crate, imported_name) + .into_iter() .filter_map(|candidate| { - let item: hir::ItemInNs = candidate.either(Into::into, Into::into); current_module - .find_use_path_prefixed(db, item, config.insert_use.prefix_kind) - .zip(Some(item)) + .find_use_path_prefixed(db, candidate, config.insert_use.prefix_kind) + .zip(Some(candidate)) }) .find(|(mod_path, _)| mod_path.to_string() == full_import_path)?; - let import = LocatedImport::new(import_path, item_to_import, None); + let import = LocatedImport::new(import_path, item_to_import, item_to_import); - ImportEdit { import_path, import_scope, import_for_trait_assoc_item } - .to_text_edit(config.insert_use) - .map(|edit| vec![edit]) + ImportEdit { import, import_scope }.to_text_edit(config.insert_use).map(|edit| vec![edit]) } #[cfg(test)] diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 4bddc3957d..fae5685e22 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -53,18 +53,20 @@ pub(crate) fn render_resolution<'a>( pub(crate) fn render_resolution_with_import<'a>( ctx: RenderContext<'a>, import_edit: ImportEdit, - resolution: &ScopeDef, ) -> Option { + let resolution = ScopeDef::from(import_edit.import.original_item); let local_name = match resolution { ScopeDef::ModuleDef(ModuleDef::Function(f)) => f.name(ctx.completion.db).to_string(), ScopeDef::ModuleDef(ModuleDef::Const(c)) => c.name(ctx.completion.db)?.to_string(), ScopeDef::ModuleDef(ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db).to_string(), - _ => item_name(ctx.db(), import_edit.import.item_to_display())?.to_string(), + _ => item_name(ctx.db(), import_edit.import.original_item)?.to_string(), }; - Render::new(ctx).render_resolution(local_name, Some(import_edit), resolution).map(|mut item| { - item.completion_kind = CompletionKind::Magic; - item - }) + Render::new(ctx).render_resolution(local_name, Some(import_edit), &resolution).map( + |mut item| { + item.completion_kind = CompletionKind::Magic; + item + }, + ) } /// Interface for data and methods required for items rendering. diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index a30a4dd9de..8d16c011e7 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,14 +1,13 @@ //! Look up accessible paths for items. -use either::Either; use hir::{ AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module, - ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type, + ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type, }; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; use crate::{ - imports_locator::{self, AssocItemSearch, DEFAULT_QUERY_SEARCH_LIMIT}, + items_locator::{self, AssocItemSearch, DEFAULT_QUERY_SEARCH_LIMIT}, RootDatabase, }; @@ -130,34 +129,23 @@ impl<'a> ImportAssets<'a> { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct LocatedImport { - import_path: ModPath, - item_to_import: ItemInNs, - data_to_display: Option<(ModPath, ItemInNs)>, + pub import_path: ModPath, + pub item_to_import: ItemInNs, + pub original_item: ItemInNs, } impl LocatedImport { - pub fn new( - import_path: ModPath, - item_to_import: ItemInNs, - data_to_display: Option<(ModPath, ItemInNs)>, - ) -> Self { - Self { import_path, item_to_import, data_to_display } + pub fn new(import_path: ModPath, item_to_import: ItemInNs, original_item: ItemInNs) -> Self { + Self { import_path, item_to_import, original_item } } - pub fn display_path(&self) -> &ModPath { - self.data_to_display.as_ref().map(|(mod_path, _)| mod_path).unwrap_or(&self.import_path) - } - - pub fn import_path(&self) -> &ModPath { - &self.import_path - } - - pub fn item_to_display(&self) -> ItemInNs { - self.data_to_display.as_ref().map(|&(_, item)| item).unwrap_or(self.item_to_import) - } - - pub fn item_to_import(&self) -> ItemInNs { - self.item_to_import + pub fn original_item_name(&self, db: &RootDatabase) -> Option { + match self.original_item { + ItemInNs::Types(module_def_id) | ItemInNs::Values(module_def_id) => { + ModuleDef::from(module_def_id).name(db) + } + ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).name(db), + } } } @@ -166,25 +154,20 @@ impl<'a> ImportAssets<'a> { &self.import_candidate } - fn name_to_import(&self) -> &NameToImport { - match &self.import_candidate { - ImportCandidate::Path(candidate) => &candidate.name, - ImportCandidate::TraitAssocItem(candidate) - | ImportCandidate::TraitMethod(candidate) => &candidate.name, - } - } - pub fn search_for_imports( &self, sema: &Semantics, prefix_kind: PrefixKind, - ) -> Vec { + ) -> FxHashSet { let _p = profile::span("import_assets::search_for_imports"); self.search_for(sema, Some(prefix_kind)) } /// This may return non-absolute paths if a part of the returned path is already imported into scope. - pub fn search_for_relative_paths(&self, sema: &Semantics) -> Vec { + pub fn search_for_relative_paths( + &self, + sema: &Semantics, + ) -> FxHashSet { let _p = profile::span("import_assets::search_for_relative_paths"); self.search_for(sema, None) } @@ -193,14 +176,13 @@ impl<'a> ImportAssets<'a> { &self, sema: &Semantics, prefixed: Option, - ) -> Vec { - let current_crate = self.module_with_candidate.krate(); - let scope_definitions = self.scope_definitions(); - - let defs_for_candidate_name = match self.name_to_import() { - NameToImport::Exact(exact_name) => { - imports_locator::find_exact_imports(sema, current_crate, exact_name.clone()) - } + ) -> FxHashSet { + let items_with_candidate_name = match self.name_to_import() { + NameToImport::Exact(exact_name) => items_locator::with_for_exact_name( + sema, + self.module_with_candidate.krate(), + exact_name.clone(), + ), // FIXME: ideally, we should avoid using `fst` for seacrhing trait imports for assoc items: // instead, we need to look up all trait impls for a certain struct and search through them only // see https://github.com/rust-analyzer/rust-analyzer/pull/7293#issuecomment-761585032 @@ -213,9 +195,9 @@ impl<'a> ImportAssets<'a> { (AssocItemSearch::Include, Some(DEFAULT_QUERY_SEARCH_LIMIT)) }; - imports_locator::find_similar_imports( + items_locator::with_similar_name( sema, - current_crate, + self.module_with_candidate.krate(), fuzzy_name.clone(), assoc_item_search, limit, @@ -223,10 +205,11 @@ impl<'a> ImportAssets<'a> { } }; - self.applicable_defs(sema.db, prefixed, defs_for_candidate_name) + let scope_definitions = self.scope_definitions(); + self.applicable_defs(sema.db, prefixed, items_with_candidate_name) .into_iter() - .filter(|import| import.import_path().len() > 1) - .filter(|import| !scope_definitions.contains(&ScopeDef::from(import.item_to_import()))) + .filter(|import| import.import_path.len() > 1) + .filter(|import| !scope_definitions.contains(&ScopeDef::from(import.item_to_import))) .collect() } @@ -238,11 +221,19 @@ impl<'a> ImportAssets<'a> { scope_definitions } + fn name_to_import(&self) -> &NameToImport { + match &self.import_candidate { + ImportCandidate::Path(candidate) => &candidate.name, + ImportCandidate::TraitAssocItem(candidate) + | ImportCandidate::TraitMethod(candidate) => &candidate.name, + } + } + fn applicable_defs( &self, db: &RootDatabase, prefixed: Option, - defs_for_candidate_name: impl Iterator>, + items_with_candidate_name: FxHashSet, ) -> FxHashSet { let _p = profile::span("import_assets::applicable_defs"); let current_crate = self.module_with_candidate.krate(); @@ -251,7 +242,7 @@ impl<'a> ImportAssets<'a> { match &self.import_candidate { ImportCandidate::Path(path_candidate) => { - path_applicable_imports(db, path_candidate, mod_path, defs_for_candidate_name) + path_applicable_imports(db, path_candidate, mod_path, items_with_candidate_name) } ImportCandidate::TraitAssocItem(trait_candidate) => trait_applicable_items( db, @@ -259,7 +250,7 @@ impl<'a> ImportAssets<'a> { trait_candidate, true, mod_path, - defs_for_candidate_name, + items_with_candidate_name, ), ImportCandidate::TraitMethod(trait_candidate) => trait_applicable_items( db, @@ -267,7 +258,7 @@ impl<'a> ImportAssets<'a> { trait_candidate, false, mod_path, - defs_for_candidate_name, + items_with_candidate_name, ), } } @@ -277,17 +268,15 @@ fn path_applicable_imports( db: &RootDatabase, path_candidate: &PathImportCandidate, mod_path: impl Fn(ItemInNs) -> Option + Copy, - defs_for_candidate_name: impl Iterator>, + items_with_candidate_name: FxHashSet, ) -> FxHashSet { let _p = profile::span("import_assets::path_applicable_imports"); - let items_for_candidate_name = - defs_for_candidate_name.map(|def| def.either(ItemInNs::from, ItemInNs::from)); - let (unresolved_first_segment, unresolved_qualifier) = match &path_candidate.qualifier { Qualifier::Absent => { - return items_for_candidate_name - .filter_map(|item| Some(LocatedImport::new(mod_path(item)?, item, None))) + return items_with_candidate_name + .into_iter() + .filter_map(|item| Some(LocatedImport::new(mod_path(item)?, item, item))) .collect(); } Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => { @@ -295,7 +284,8 @@ fn path_applicable_imports( } }; - items_for_candidate_name + items_with_candidate_name + .into_iter() .filter_map(|item| { import_for_item(db, mod_path, &unresolved_first_segment, &unresolved_qualifier, item) }) @@ -336,7 +326,6 @@ fn import_for_item( } let segment_import = find_import_for_segment(db, item_candidate, &unresolved_first_segment)?; - let data_to_display = Some((import_path_candidate.clone(), original_item)); Some(match (segment_import == item_candidate, trait_to_import) { (true, Some(_)) => { // FIXME we should be able to import both the trait and the segment, @@ -345,11 +334,11 @@ fn import_for_item( return None; } (false, Some(trait_to_import)) => { - LocatedImport::new(mod_path(trait_to_import)?, trait_to_import, data_to_display) + LocatedImport::new(mod_path(trait_to_import)?, trait_to_import, original_item) } - (true, None) => LocatedImport::new(import_path_candidate, item_candidate, data_to_display), + (true, None) => LocatedImport::new(import_path_candidate, item_candidate, original_item), (false, None) => { - LocatedImport::new(mod_path(segment_import)?, segment_import, data_to_display) + LocatedImport::new(mod_path(segment_import)?, segment_import, original_item) } }) } @@ -399,16 +388,14 @@ fn trait_applicable_items( trait_candidate: &TraitImportCandidate, trait_assoc_item: bool, mod_path: impl Fn(ItemInNs) -> Option, - defs_for_candidate_name: impl Iterator>, + items_with_candidate_name: FxHashSet, ) -> FxHashSet { let _p = profile::span("import_assets::trait_applicable_items"); let mut required_assoc_items = FxHashSet::default(); - let trait_candidates = defs_for_candidate_name - .filter_map(|input| match input { - Either::Left(module_def) => module_def.as_assoc_item(db), - _ => None, - }) + let trait_candidates = items_with_candidate_name + .into_iter() + .filter_map(|input| ModuleDef::from(input.as_module_def_id()?).as_assoc_item(db)) .filter_map(|assoc| { let assoc_item_trait = assoc.containing_trait(db)?; required_assoc_items.insert(assoc); @@ -433,20 +420,10 @@ fn trait_applicable_items( } let item = ItemInNs::from(ModuleDef::from(assoc.containing_trait(db)?)); - let item_path = mod_path(item)?; - - let assoc_item = assoc_to_item(assoc); - let assoc_item_path = match assoc.container(db) { - AssocItemContainer::Trait(_) => item_path.clone(), - AssocItemContainer::Impl(impl_) => mod_path(ItemInNs::from( - ModuleDef::from(impl_.target_ty(db).as_adt()?), - ))?, - }; - located_imports.insert(LocatedImport::new( - item_path, + mod_path(item)?, item, - Some((assoc_item_path, assoc_item)), + assoc_to_item(assoc), )); } None::<()> @@ -462,20 +439,10 @@ 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 item_path = mod_path(item)?; - - let assoc_item = assoc_to_item(assoc); - let assoc_item_path = match assoc.container(db) { - AssocItemContainer::Trait(_) => item_path.clone(), - AssocItemContainer::Impl(impl_) => mod_path(ItemInNs::from( - ModuleDef::from(impl_.target_ty(db).as_adt()?), - ))?, - }; - located_imports.insert(LocatedImport::new( - item_path, + mod_path(item)?, item, - Some((assoc_item_path, assoc_item)), + assoc_to_item(assoc), )); } None::<()> diff --git a/crates/ide_db/src/imports_locator.rs b/crates/ide_db/src/items_locator.rs similarity index 62% rename from crates/ide_db/src/imports_locator.rs rename to crates/ide_db/src/items_locator.rs index fd700e04ff..b81c14618a 100644 --- a/crates/ide_db/src/imports_locator.rs +++ b/crates/ide_db/src/items_locator.rs @@ -1,9 +1,10 @@ //! This module contains an import search functionality that is provided to the assists module. //! Later, this should be moved away to a separate crate that is accessible from the assists module. +use either::Either; use hir::{ import_map::{self, ImportKind}, - AsAssocItem, Crate, MacroDef, ModuleDef, Semantics, + AsAssocItem, Crate, ItemInNs, ModuleDef, Semantics, }; use syntax::{ast, AstNode, SyntaxKind::NAME}; @@ -12,32 +13,31 @@ use crate::{ symbol_index::{self, FileSymbol}, RootDatabase, }; -use either::Either; use rustc_hash::FxHashSet; pub(crate) const DEFAULT_QUERY_SEARCH_LIMIT: usize = 40; -pub fn find_exact_imports( +pub fn with_for_exact_name( sema: &Semantics<'_, RootDatabase>, krate: Crate, - name_to_import: String, -) -> Box>> { + exact_name: String, +) -> FxHashSet { let _p = profile::span("find_exact_imports"); - Box::new(find_imports( + find_items( sema, krate, { - let mut local_query = symbol_index::Query::new(name_to_import.clone()); + let mut local_query = symbol_index::Query::new(exact_name.clone()); local_query.exact(); local_query.limit(DEFAULT_QUERY_SEARCH_LIMIT); local_query }, - import_map::Query::new(name_to_import) + import_map::Query::new(exact_name) .limit(DEFAULT_QUERY_SEARCH_LIMIT) .name_only() .search_mode(import_map::SearchMode::Equals) .case_sensitive(), - )) + ) } #[derive(Debug)] @@ -47,13 +47,13 @@ pub enum AssocItemSearch { AssocItemsOnly, } -pub fn find_similar_imports<'a>( - sema: &'a Semantics<'a, RootDatabase>, +pub fn with_similar_name( + sema: &Semantics<'_, RootDatabase>, krate: Crate, fuzzy_search_string: String, assoc_item_search: AssocItemSearch, limit: Option, -) -> Box> + 'a> { +) -> FxHashSet { let _p = profile::span("find_similar_imports"); let mut external_query = import_map::Query::new(fuzzy_search_string.clone()) @@ -77,36 +77,39 @@ pub fn find_similar_imports<'a>( local_query.limit(limit); } - Box::new(find_imports(sema, krate, local_query, external_query).filter( - move |import_candidate| match assoc_item_search { + find_items(sema, krate, local_query, external_query) + .into_iter() + .filter(move |&item| match assoc_item_search { AssocItemSearch::Include => true, - AssocItemSearch::Exclude => !is_assoc_item(import_candidate, sema.db), - AssocItemSearch::AssocItemsOnly => is_assoc_item(import_candidate, sema.db), - }, - )) + AssocItemSearch::Exclude => !is_assoc_item(item, sema.db), + AssocItemSearch::AssocItemsOnly => is_assoc_item(item, sema.db), + }) + .collect() } -fn is_assoc_item(import_candidate: &Either, db: &RootDatabase) -> bool { - match import_candidate { - Either::Left(ModuleDef::Function(function)) => function.as_assoc_item(db).is_some(), - Either::Left(ModuleDef::Const(const_)) => const_.as_assoc_item(db).is_some(), - Either::Left(ModuleDef::TypeAlias(type_alias)) => type_alias.as_assoc_item(db).is_some(), - _ => false, - } +fn is_assoc_item(item: ItemInNs, db: &RootDatabase) -> bool { + item.as_module_def_id() + .and_then(|module_def_id| ModuleDef::from(module_def_id).as_assoc_item(db)) + .is_some() } -fn find_imports<'a>( - sema: &Semantics<'a, RootDatabase>, +fn find_items( + sema: &Semantics<'_, RootDatabase>, krate: Crate, local_query: symbol_index::Query, external_query: import_map::Query, -) -> impl Iterator> { +) -> FxHashSet { let _p = profile::span("find_similar_imports"); let db = sema.db; // Query dependencies first. - let mut candidates: FxHashSet<_> = - krate.query_external_importables(db, external_query).collect(); + let mut candidates = krate + .query_external_importables(db, external_query) + .map(|external_importable| match external_importable { + Either::Left(module_def) => ItemInNs::from(module_def), + Either::Right(macro_def) => ItemInNs::from(macro_def), + }) + .collect::>(); // Query the local crate using the symbol index. let local_results = symbol_index::crate_symbols(db, krate.into(), local_query); @@ -114,19 +117,19 @@ fn find_imports<'a>( candidates.extend( local_results .into_iter() - .filter_map(|import_candidate| get_name_definition(sema, &import_candidate)) + .filter_map(|local_candidate| get_name_definition(sema, &local_candidate)) .filter_map(|name_definition_to_import| match name_definition_to_import { - Definition::ModuleDef(module_def) => Some(Either::Left(module_def)), - Definition::Macro(macro_def) => Some(Either::Right(macro_def)), + Definition::ModuleDef(module_def) => Some(ItemInNs::from(module_def)), + Definition::Macro(macro_def) => Some(ItemInNs::from(macro_def)), _ => None, }), ); - candidates.into_iter() + candidates } -fn get_name_definition<'a>( - sema: &Semantics<'a, RootDatabase>, +fn get_name_definition( + sema: &Semantics<'_, RootDatabase>, import_candidate: &FileSymbol, ) -> Option { let _p = profile::span("get_name_definition"); diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 6eb34b06b7..88ee4a87d5 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -8,7 +8,7 @@ pub mod line_index; pub mod symbol_index; pub mod defs; pub mod search; -pub mod imports_locator; +pub mod items_locator; pub mod source_change; pub mod ty_filter; pub mod traits; diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index d479d826fb..2c4c339cb7 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -697,7 +697,6 @@ pub(crate) fn handle_completion_resolve( FilePosition { file_id, offset }, &resolve_data.full_import_path, resolve_data.imported_name, - resolve_data.import_for_trait_assoc_item, )? .into_iter() .flat_map(|edit| edit.into_iter().map(|indel| to_proto::text_edit(&line_index, indel))) @@ -1525,7 +1524,6 @@ struct CompletionResolveData { position: lsp_types::TextDocumentPositionParams, full_import_path: String, imported_name: String, - import_for_trait_assoc_item: bool, } fn fill_resolve_data( @@ -1534,14 +1532,13 @@ fn fill_resolve_data( position: &TextDocumentPositionParams, ) -> Option<()> { let import_edit = item.import_to_add()?; - let import_path = import_edit.import.import_path(); + let import_path = &import_edit.import.import_path; *resolve_data = Some( to_value(CompletionResolveData { position: position.to_owned(), full_import_path: import_path.to_string(), imported_name: import_path.segments().last()?.to_string(), - import_for_trait_assoc_item: import_edit.import_for_trait_assoc_item, }) .unwrap(), ); From 24a5d3b19dfa3e076df8b7413d0cc4a547aeb7d7 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 3 Mar 2021 23:55:21 +0200 Subject: [PATCH 15/24] Fix the completion labels and tests --- .../src/completions/flyimport.rs | 34 ++++--- crates/ide_completion/src/item.rs | 39 ++++---- crates/ide_completion/src/lib.rs | 7 +- crates/ide_db/src/helpers/import_assets.rs | 99 +++++++++++++------ 4 files changed, 115 insertions(+), 64 deletions(-) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index c1e3f091fb..c5b3c9e277 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 d01620500c..44e4a6dfd8 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; + let mut label = self.label; + let mut lookup = self.lookup; + let mut 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(); - // } + 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 d19368de09..5470914fbf 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 8d16c011e7..b3e90717a5 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::<()> From 5b7d928075f3bedf71f754444c1532976d52eae4 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 3 Mar 2021 23:59:56 +0200 Subject: [PATCH 16/24] Enforce the located imports' order --- crates/ide_assists/src/handlers/auto_import.rs | 2 +- crates/ide_assists/src/handlers/qualify_path.rs | 2 +- crates/ide_db/src/helpers/import_assets.rs | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index f3c969eeec..eb8d35e957 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -245,7 +245,7 @@ mod tests { } ", r" - use PubMod1::PubStruct; + use PubMod3::PubStruct; PubStruct diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index 407ba47bea..b36dd38230 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -317,7 +317,7 @@ mod tests { } ", r" - PubMod1::PubStruct + PubMod3::PubStruct pub mod PubMod1 { pub struct PubStruct; diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index b3e90717a5..b78d1969d2 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -3,6 +3,7 @@ use hir::{ AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module, ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type, }; +use itertools::Itertools; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -164,16 +165,13 @@ impl<'a> ImportAssets<'a> { &self, sema: &Semantics, prefix_kind: PrefixKind, - ) -> FxHashSet { + ) -> Vec { let _p = profile::span("import_assets::search_for_imports"); self.search_for(sema, Some(prefix_kind)) } /// This may return non-absolute paths if a part of the returned path is already imported into scope. - pub fn search_for_relative_paths( - &self, - sema: &Semantics, - ) -> FxHashSet { + pub fn search_for_relative_paths(&self, sema: &Semantics) -> Vec { let _p = profile::span("import_assets::search_for_relative_paths"); self.search_for(sema, None) } @@ -182,7 +180,7 @@ impl<'a> ImportAssets<'a> { &self, sema: &Semantics, prefixed: Option, - ) -> FxHashSet { + ) -> Vec { let items_with_candidate_name = match self.name_to_import() { NameToImport::Exact(exact_name) => items_locator::with_for_exact_name( sema, @@ -216,6 +214,7 @@ impl<'a> ImportAssets<'a> { .into_iter() .filter(|import| import.import_path.len() > 1) .filter(|import| !scope_definitions.contains(&ScopeDef::from(import.item_to_import))) + .sorted_by_key(|import| import.import_path.clone()) .collect() } From 6ca6f101c1b3a795ff6578bae6e01cb4b818e14c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 4 Mar 2021 00:10:20 +0200 Subject: [PATCH 17/24] Test for fuzzy unresolved path maatch --- .../src/completions/flyimport.rs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index c5b3c9e277..efb91fe0e0 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -921,4 +921,26 @@ fn main() { }"#, ); } + + #[test] + fn fuzzy_unresolved_path() { + check( + r#" +mod foo { + pub mod bar { + pub struct Item; + + impl Item { + pub const TEST_ASSOC: usize = 3; + } + } +} + +fn main() { + let zz = "sdsd"; + bar::Ass$0 +}"#, + expect![[]], + ) + } } From 84c575a21201cdbeb391ff2cfae2fbbccaa76f8a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 5 Mar 2021 00:11:07 +0200 Subject: [PATCH 18/24] Restrict fuzzy qualifiers for now --- crates/hir_def/src/import_map.rs | 23 ---------- .../src/completions/flyimport.rs | 7 ++- crates/ide_db/src/helpers/import_assets.rs | 43 ++++++++++--------- 3 files changed, 26 insertions(+), 47 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 07ee7bdfd7..369bc3350b 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -1094,27 +1094,4 @@ mod tests { expect![[r#""#]], ); } - - #[test] - fn search_with_path() { - check_search( - r#" -//- /main.rs crate:main deps:dep -//- /dep.rs crate:dep -pub mod foo { - pub mod bar { - pub mod baz { - pub trait Display { - fn fmt(); - } - } - } -}"#, - "main", - Query::new("baz::fmt".to_string()).search_mode(SearchMode::Fuzzy), - expect![[r#" - dep::foo::bar::baz::Display::fmt (a) - "#]], - ); - } } diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index efb91fe0e0..8a11cba411 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -21,8 +21,9 @@ //! ``` //! //! Also completes associated items, that require trait imports. -//! If any unresolved and/or partially-qualified path predeces the input, it will be taken into account: only the items with import string -//! containing this whole path will be considered and the corresponding path import will be added: +//! If any unresolved and/or partially-qualified path predeces the input, it will be taken into account. +//! Currently, only the imports with their import path ending with the whole qialifier will be proposed +//! (no fuzzy matching for qualifier). //! //! ``` //! mod foo { @@ -187,7 +188,6 @@ fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option { @@ -358,19 +360,15 @@ fn import_for_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::Types(_) | ItemInNs::Values(_) => match item_as_assoc(db, item) { + 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, }) } @@ -427,7 +425,7 @@ fn trait_applicable_items( let trait_candidates = items_with_candidate_name .into_iter() - .filter_map(|input| ModuleDef::from(input.as_module_def_id()?).as_assoc_item(db)) + .filter_map(|input| item_as_assoc(db, input)) .filter_map(|assoc| { let assoc_item_trait = assoc.containing_trait(db)?; required_assoc_items.insert(assoc); @@ -583,3 +581,8 @@ fn path_import_candidate( None => ImportCandidate::Path(PathImportCandidate { qualifier: Qualifier::Absent, name }), }) } + +fn item_as_assoc(db: &RootDatabase, item: ItemInNs) -> Option { + item.as_module_def_id() + .and_then(|module_def_id| ModuleDef::from(module_def_id).as_assoc_item(db)) +} From c56b59d37781f6eb7caee76de2c8a1bbc9d169dc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 5 Mar 2021 00:29:39 +0200 Subject: [PATCH 19/24] Cleanup --- crates/ide_completion/src/item.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 44e4a6dfd8..06f16a2c7c 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -441,9 +441,3 @@ impl Builder { self } } - -// impl<'a> Into for Builder { -// fn into(self) -> CompletionItem { -// self.build() -// } -// } From db61d4ea13113cd6c4e0661075ea9b2f739be862 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 7 Mar 2021 11:58:43 +0200 Subject: [PATCH 20/24] Rebase leftovers --- crates/ide_completion/src/item.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 06f16a2c7c..9b039e3e5f 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -283,11 +283,8 @@ impl ImportEdit { pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option { let _p = profile::span("ImportEdit::to_text_edit"); - let rewriter = insert_use::insert_use( - &self.scope, - mod_path_to_ast(&self.import.import_path), - cfg, - ); + let rewriter = + insert_use::insert_use(&self.scope, mod_path_to_ast(&self.import.import_path), cfg); let old_ast = rewriter.rewrite_root()?; let mut import_insert = TextEdit::builder(); algo::diff(&old_ast, &rewriter.rewrite(&old_ast)).into_text_edit(&mut import_insert); From dccbb38d2e28bfeb53f31c13de3b83e72f1a476c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 8 Mar 2021 00:25:45 +0200 Subject: [PATCH 21/24] Less lifetines: derive SemanticsScope in place --- crates/hir/src/semantics.rs | 2 +- .../ide_assists/src/handlers/auto_import.rs | 4 +- .../replace_derive_with_manual_impl.rs | 31 ++++++------ .../src/completions/flyimport.rs | 11 +++-- crates/ide_completion/src/lib.rs | 2 +- crates/ide_db/src/helpers/import_assets.rs | 47 +++++++++---------- crates/ide_db/src/items_locator.rs | 2 +- 7 files changed, 49 insertions(+), 50 deletions(-) diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 69370ef3d8..945638cc56 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -774,7 +774,7 @@ fn find_root(node: &SyntaxNode) -> SyntaxNode { /// /// Note that if you are wondering "what does this specific existing name mean?", /// you'd better use the `resolve_` family of methods. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct SemanticsScope<'a> { pub db: &'a dyn HirDatabase, file_id: HirFileId, diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index eb8d35e957..5546c3a4e1 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -112,9 +112,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> Some(()) } -pub(super) fn find_importable_node<'a>( - ctx: &'a AssistContext, -) -> Option<(ImportAssets<'a>, SyntaxNode)> { +pub(super) fn find_importable_node(ctx: &AssistContext) -> Option<(ImportAssets, SyntaxNode)> { if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::() { ImportAssets::for_exact_path(&path_under_caret, &ctx.sema) .zip(Some(path_under_caret.syntax().clone())) diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index 93a03e8b25..88fe2fe904 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs @@ -65,23 +65,20 @@ pub(crate) fn replace_derive_with_manual_impl( let current_module = ctx.sema.scope(annotated_name.syntax()).module()?; let current_crate = current_module.krate(); - let found_traits = items_locator::with_for_exact_name( - &ctx.sema, - current_crate, - trait_token.text().to_string(), - ) - .into_iter() - .filter_map(|item| match ModuleDef::from(item.as_module_def_id()?) { - ModuleDef::Trait(trait_) => Some(trait_), - _ => None, - }) - .flat_map(|trait_| { - current_module - .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_)) - .as_ref() - .map(mod_path_to_ast) - .zip(Some(trait_)) - }); + let found_traits = + items_locator::with_exact_name(&ctx.sema, current_crate, trait_token.text().to_string()) + .into_iter() + .filter_map(|item| match ModuleDef::from(item.as_module_def_id()?) { + ModuleDef::Trait(trait_) => Some(trait_), + _ => None, + }) + .flat_map(|trait_| { + current_module + .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_)) + .as_ref() + .map(mod_path_to_ast) + .zip(Some(trait_)) + }); let mut no_traits_found = true; for (trait_path, trait_) in found_traits.inspect(|_| no_traits_found = false) { diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 8a11cba411..391a11c91c 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -169,23 +169,28 @@ pub(crate) fn position_for_import<'a>( }) } -fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option> { +fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option { let current_module = ctx.scope.module()?; if let Some(dot_receiver) = &ctx.dot_receiver { ImportAssets::for_fuzzy_method_call( current_module, ctx.sema.type_of_expr(dot_receiver)?, fuzzy_name, - ctx.scope.clone(), + dot_receiver.syntax().clone(), ) } else { let fuzzy_name_length = fuzzy_name.len(); + let approximate_node = match current_module.definition_source(ctx.db).value { + hir::ModuleSource::SourceFile(s) => s.syntax().clone(), + hir::ModuleSource::Module(m) => m.syntax().clone(), + hir::ModuleSource::BlockExpr(b) => b.syntax().clone(), + }; let assets_for_path = ImportAssets::for_fuzzy_path( current_module, ctx.path_qual.clone(), fuzzy_name, &ctx.sema, - ctx.scope.clone(), + approximate_node, )?; if matches!(assets_for_path.import_candidate(), ImportCandidate::Path(_)) diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index 5470914fbf..a0c8c374d0 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -150,7 +150,7 @@ pub fn resolve_completion_edits( let current_crate = current_module.krate(); let (import_path, item_to_import) = - items_locator::with_for_exact_name(&ctx.sema, current_crate, imported_name) + items_locator::with_exact_name(&ctx.sema, current_crate, imported_name) .into_iter() .filter_map(|candidate| { current_module diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 9bdc938774..f2866af136 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,11 +1,11 @@ //! Look up accessible paths for items. use hir::{ AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module, - ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type, + ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, Type, }; use itertools::Itertools; use rustc_hash::FxHashSet; -use syntax::{ast, AstNode}; +use syntax::{ast, AstNode, SyntaxNode}; use crate::{ items_locator::{self, AssocItemSearch, DEFAULT_QUERY_SEARCH_LIMIT}, @@ -62,38 +62,37 @@ impl NameToImport { } #[derive(Debug)] -pub struct ImportAssets<'a> { +pub struct ImportAssets { import_candidate: ImportCandidate, + candidate_node: SyntaxNode, module_with_candidate: Module, - scope: SemanticsScope<'a>, } -impl<'a> ImportAssets<'a> { +impl ImportAssets { pub fn for_method_call( method_call: &ast::MethodCallExpr, - sema: &'a Semantics, + sema: &Semantics, ) -> Option { - let scope = sema.scope(method_call.syntax()); + let candidate_node = method_call.syntax().clone(); Some(Self { import_candidate: ImportCandidate::for_method_call(sema, method_call)?, - module_with_candidate: scope.module()?, - scope, + module_with_candidate: sema.scope(&candidate_node).module()?, + candidate_node, }) } pub fn for_exact_path( fully_qualified_path: &ast::Path, - sema: &'a Semantics, + sema: &Semantics, ) -> Option { - let syntax_under_caret = fully_qualified_path.syntax(); - if syntax_under_caret.ancestors().find_map(ast::Use::cast).is_some() { + let candidate_node = fully_qualified_path.syntax().clone(); + if candidate_node.ancestors().find_map(ast::Use::cast).is_some() { return None; } - let scope = sema.scope(syntax_under_caret); Some(Self { import_candidate: ImportCandidate::for_regular_path(sema, fully_qualified_path)?, - module_with_candidate: scope.module()?, - scope, + module_with_candidate: sema.scope(&candidate_node).module()?, + candidate_node, }) } @@ -102,12 +101,12 @@ impl<'a> ImportAssets<'a> { qualifier: Option, fuzzy_name: String, sema: &Semantics, - scope: SemanticsScope<'a>, + candidate_node: SyntaxNode, ) -> Option { Some(Self { import_candidate: ImportCandidate::for_fuzzy_path(qualifier, fuzzy_name, sema)?, module_with_candidate, - scope, + candidate_node, }) } @@ -115,7 +114,7 @@ impl<'a> ImportAssets<'a> { module_with_method_call: Module, receiver_ty: Type, fuzzy_method_name: String, - scope: SemanticsScope<'a>, + candidate_node: SyntaxNode, ) -> Option { Some(Self { import_candidate: ImportCandidate::TraitMethod(TraitImportCandidate { @@ -123,7 +122,7 @@ impl<'a> ImportAssets<'a> { name: NameToImport::Fuzzy(fuzzy_method_name), }), module_with_candidate: module_with_method_call, - scope, + candidate_node, }) } } @@ -156,7 +155,7 @@ impl LocatedImport { } } -impl<'a> ImportAssets<'a> { +impl ImportAssets { pub fn import_candidate(&self) -> &ImportCandidate { &self.import_candidate } @@ -182,7 +181,7 @@ impl<'a> ImportAssets<'a> { prefixed: Option, ) -> Vec { let items_with_candidate_name = match self.name_to_import() { - NameToImport::Exact(exact_name) => items_locator::with_for_exact_name( + NameToImport::Exact(exact_name) => items_locator::with_exact_name( sema, self.module_with_candidate.krate(), exact_name.clone(), @@ -209,7 +208,7 @@ impl<'a> ImportAssets<'a> { } }; - let scope_definitions = self.scope_definitions(); + let scope_definitions = self.scope_definitions(sema); self.applicable_defs(sema.db, prefixed, items_with_candidate_name) .into_iter() .filter(|import| import.import_path.len() > 1) @@ -218,9 +217,9 @@ impl<'a> ImportAssets<'a> { .collect() } - fn scope_definitions(&self) -> FxHashSet { + fn scope_definitions(&self, sema: &Semantics) -> FxHashSet { let mut scope_definitions = FxHashSet::default(); - self.scope.process_all_names(&mut |_, scope_def| { + sema.scope(&self.candidate_node).process_all_names(&mut |_, scope_def| { scope_definitions.insert(scope_def); }); scope_definitions diff --git a/crates/ide_db/src/items_locator.rs b/crates/ide_db/src/items_locator.rs index b81c14618a..8a7f029353 100644 --- a/crates/ide_db/src/items_locator.rs +++ b/crates/ide_db/src/items_locator.rs @@ -17,7 +17,7 @@ use rustc_hash::FxHashSet; pub(crate) const DEFAULT_QUERY_SEARCH_LIMIT: usize = 40; -pub fn with_for_exact_name( +pub fn with_exact_name( sema: &Semantics<'_, RootDatabase>, krate: Crate, exact_name: String, From 5168ab16e14679e16a472c0ab13b1bbc32dc95f3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 8 Mar 2021 14:09:20 +0200 Subject: [PATCH 22/24] Add rustdocs and use better names --- .../ide_assists/src/handlers/auto_import.rs | 8 +- .../ide_assists/src/handlers/qualify_path.rs | 14 ++-- crates/ide_db/src/helpers/import_assets.rs | 82 ++++++++++++------- 3 files changed, 64 insertions(+), 40 deletions(-) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 5546c3a4e1..7caee8df04 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -1,7 +1,7 @@ use ide_db::helpers::{ import_assets::{ImportAssets, ImportCandidate}, insert_use::{insert_use, ImportScope}, - mod_path_to_ast, + item_name, mod_path_to_ast, }; use syntax::{ast, AstNode, SyntaxNode}; @@ -93,7 +93,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let group = import_group_message(import_assets.import_candidate()); let scope = ImportScope::find_insert_use_container(&syntax_under_caret, &ctx.sema)?; for import in proposed_imports { - let name = match import.original_item_name(ctx.db()) { + let name = match item_name(ctx.db(), import.original_item) { Some(name) => name, None => continue, }; @@ -130,10 +130,10 @@ fn import_group_message(import_candidate: &ImportCandidate) -> GroupLabel { let name = match import_candidate { ImportCandidate::Path(candidate) => format!("Import {}", candidate.name.text()), ImportCandidate::TraitAssocItem(candidate) => { - format!("Import a trait for item {}", candidate.name.text()) + format!("Import a trait for item {}", candidate.assoc_item_name.text()) } ImportCandidate::TraitMethod(candidate) => { - format!("Import a trait for method {}", candidate.name.text()) + format!("Import a trait for method {}", candidate.assoc_item_name.text()) } }; GroupLabel(name) diff --git a/crates/ide_assists/src/handlers/qualify_path.rs b/crates/ide_assists/src/handlers/qualify_path.rs index b36dd38230..272874ae39 100644 --- a/crates/ide_assists/src/handlers/qualify_path.rs +++ b/crates/ide_assists/src/handlers/qualify_path.rs @@ -2,8 +2,8 @@ use std::iter; use hir::AsAssocItem; use ide_db::helpers::{ - import_assets::{ImportCandidate, LocatedImport, Qualifier}, - mod_path_to_ast, + import_assets::{ImportCandidate, LocatedImport}, + item_name, mod_path_to_ast, }; use ide_db::RootDatabase; use syntax::{ @@ -48,7 +48,7 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let qualify_candidate = match candidate { ImportCandidate::Path(candidate) => { - if !matches!(candidate.qualifier, Qualifier::Absent) { + if candidate.qualifier.is_some() { cov_mark::hit!(qualify_path_qualifier_start); let path = ast::Path::cast(syntax_under_caret)?; let (prev_segment, segment) = (path.qualifier()?.segment()?, path.segment()?); @@ -191,20 +191,22 @@ fn item_as_trait(db: &RootDatabase, item: hir::ItemInNs) -> Option { fn group_label(candidate: &ImportCandidate) -> GroupLabel { let name = match candidate { ImportCandidate::Path(it) => &it.name, - ImportCandidate::TraitAssocItem(it) | ImportCandidate::TraitMethod(it) => &it.name, + ImportCandidate::TraitAssocItem(it) | ImportCandidate::TraitMethod(it) => { + &it.assoc_item_name + } } .text(); GroupLabel(format!("Qualify {}", name)) } fn label(db: &RootDatabase, candidate: &ImportCandidate, import: &LocatedImport) -> String { - let display_path = match import.original_item_name(db) { + let display_path = match item_name(db, import.original_item) { Some(display_path) => display_path.to_string(), None => "{unknown}".to_string(), }; match candidate { ImportCandidate::Path(candidate) => { - if !matches!(candidate.qualifier, Qualifier::Absent) { + if candidate.qualifier.is_some() { format!("Qualify with `{}`", display_path) } else { format!("Qualify as `{}`", display_path) diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index f2866af136..3d9df463da 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -1,7 +1,7 @@ //! Look up accessible paths for items. use hir::{ AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module, - ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, Type, + ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, Type, }; use itertools::Itertools; use rustc_hash::FxHashSet; @@ -14,11 +14,16 @@ use crate::{ use super::item_name; +/// A candidate for import, derived during various IDE activities: +/// * completion with imports on the fly proposals +/// * completion edit resolve requests +/// * assists +/// * etc. #[derive(Debug)] pub enum ImportCandidate { - // A path, qualified (`std::collections::HashMap`) or not (`HashMap`). + /// A path, qualified (`std::collections::HashMap`) or not (`HashMap`). Path(PathImportCandidate), - /// A trait associated function (with no self parameter) or associated constant. + /// A trait associated function (with no self parameter) or an associated constant. /// For 'test_mod::TestEnum::test_function', `ty` is the `test_mod::TestEnum` expression type /// and `name` is the `test_function` TraitAssocItem(TraitImportCandidate), @@ -28,27 +33,40 @@ pub enum ImportCandidate { TraitMethod(TraitImportCandidate), } +/// A trait import needed for a given associated item access. +/// For `some::path::SomeStruct::ASSOC_`, contains the +/// type of `some::path::SomeStruct` and `ASSOC_` as the item name. #[derive(Debug)] pub struct TraitImportCandidate { + /// A type of the item that has the associated item accessed at. pub receiver_ty: Type, - pub name: NameToImport, + /// The associated item name that the trait to import should contain. + pub assoc_item_name: NameToImport, } +/// Path import for a given name, qualified or not. #[derive(Debug)] pub struct PathImportCandidate { - pub qualifier: Qualifier, + /// Optional qualifier before name. + pub qualifier: Option, + /// The name the item (struct, trait, enum, etc.) should have. pub name: NameToImport, } +/// A qualifier that has a first segment and it's unresolved. #[derive(Debug)] -pub enum Qualifier { - Absent, - FirstSegmentUnresolved(ast::NameRef, ModPath), +pub struct FirstSegmentUnresolved { + fist_segment: ast::NameRef, + full_qualifier: ModPath, } +/// A name that will be used during item lookups. #[derive(Debug)] pub enum NameToImport { + /// Requires items with names that exactly match the given string, case-sensitive. Exact(String), + /// Requires items with names that case-insensitively contain all letters from the string, + /// in the same order, but not necessary adjacent. Fuzzy(String), } @@ -61,6 +79,7 @@ impl NameToImport { } } +/// A struct to find imports in the project, given a certain name (or its part) and the context. #[derive(Debug)] pub struct ImportAssets { import_candidate: ImportCandidate, @@ -119,7 +138,7 @@ impl ImportAssets { Some(Self { import_candidate: ImportCandidate::TraitMethod(TraitImportCandidate { receiver_ty, - name: NameToImport::Fuzzy(fuzzy_method_name), + assoc_item_name: NameToImport::Fuzzy(fuzzy_method_name), }), module_with_candidate: module_with_method_call, candidate_node, @@ -127,11 +146,22 @@ impl ImportAssets { } } +/// An import (not necessary the only one) that corresponds a certain given [`PathImportCandidate`]. +/// (the structure is not entirely correct, since there can be situations requiring two imports, see FIXME below for the details) #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct LocatedImport { + /// The path to use in the `use` statement for a given candidate to be imported. pub import_path: ModPath, + /// An item that will be imported with the import path given. pub item_to_import: ItemInNs, + /// The path import candidate, resolved. + /// + /// Not necessary matches the import: + /// For any associated constant from the trait, we try to access as `some::path::SomeStruct::ASSOC_` + /// the original item is the associated constant, but the import has to be a trait that + /// defines this constant. pub original_item: ItemInNs, + /// A path of the original item. pub original_path: Option, } @@ -144,15 +174,6 @@ impl LocatedImport { ) -> Self { Self { import_path, item_to_import, original_item, original_path } } - - pub fn original_item_name(&self, db: &RootDatabase) -> Option { - match self.original_item { - ItemInNs::Types(module_def_id) | ItemInNs::Values(module_def_id) => { - ModuleDef::from(module_def_id).name(db) - } - ItemInNs::Macros(macro_def_id) => MacroDef::from(macro_def_id).name(db), - } - } } impl ImportAssets { @@ -229,7 +250,7 @@ impl ImportAssets { match &self.import_candidate { ImportCandidate::Path(candidate) => &candidate.name, ImportCandidate::TraitAssocItem(candidate) - | ImportCandidate::TraitMethod(candidate) => &candidate.name, + | ImportCandidate::TraitMethod(candidate) => &candidate.assoc_item_name, } } @@ -279,7 +300,7 @@ fn path_applicable_imports( let _p = profile::span("import_assets::path_applicable_imports"); let (unresolved_first_segment, unresolved_qualifier) = match &path_candidate.qualifier { - Qualifier::Absent => { + None => { return items_with_candidate_name .into_iter() .filter_map(|item| { @@ -287,9 +308,10 @@ fn path_applicable_imports( }) .collect(); } - Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => { - (first_segment.to_string(), qualifier.to_string()) - } + Some(first_segment_unresolved) => ( + first_segment_unresolved.fist_segment.to_string(), + first_segment_unresolved.full_qualifier.to_string(), + ), }; items_with_candidate_name @@ -516,7 +538,7 @@ impl ImportCandidate { Some(_) => None, None => Some(Self::TraitMethod(TraitImportCandidate { receiver_ty: sema.type_of_expr(&method_call.receiver()?)?, - name: NameToImport::Exact(method_call.name_ref()?.to_string()), + assoc_item_name: NameToImport::Exact(method_call.name_ref()?.to_string()), })), } } @@ -559,10 +581,10 @@ fn path_import_candidate( qualifier_start.syntax().ancestors().find_map(ast::Path::cast)?; if sema.resolve_path(&qualifier_start_path).is_none() { ImportCandidate::Path(PathImportCandidate { - qualifier: Qualifier::FirstSegmentUnresolved( - qualifier_start, - ModPath::from_src_unhygienic(qualifier)?, - ), + qualifier: Some(FirstSegmentUnresolved { + fist_segment: qualifier_start, + full_qualifier: ModPath::from_src_unhygienic(qualifier)?, + }), name, }) } else { @@ -572,12 +594,12 @@ fn path_import_candidate( Some(PathResolution::Def(ModuleDef::Adt(assoc_item_path))) => { ImportCandidate::TraitAssocItem(TraitImportCandidate { receiver_ty: assoc_item_path.ty(sema.db), - name, + assoc_item_name: name, }) } Some(_) => return None, }, - None => ImportCandidate::Path(PathImportCandidate { qualifier: Qualifier::Absent, name }), + None => ImportCandidate::Path(PathImportCandidate { qualifier: None, name }), }) } From 778deb38fe7e1bac8833934224d26f44eb80a6cc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 8 Mar 2021 14:59:54 +0200 Subject: [PATCH 23/24] Better strip turbofishes --- crates/hir_def/src/path.rs | 4 -- crates/ide_db/src/helpers/import_assets.rs | 8 ++-- crates/syntax/src/ast/make.rs | 4 ++ crates/syntax/src/lib.rs | 1 + crates/syntax/src/utils.rs | 43 ++++++++++++++++++++++ 5 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 crates/syntax/src/utils.rs diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs index 1dc085199f..0e60dc2b63 100644 --- a/crates/hir_def/src/path.rs +++ b/crates/hir_def/src/path.rs @@ -44,10 +44,6 @@ pub enum ImportAlias { } impl ModPath { - pub fn from_src_unhygienic(path: ast::Path) -> Option { - lower::lower_path(path, &Hygiene::new_unhygienic()).map(|it| it.mod_path) - } - pub fn from_src(path: ast::Path, hygiene: &Hygiene) -> Option { lower::lower_path(path, hygiene).map(|it| it.mod_path) } diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 3d9df463da..e03ccd3515 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -5,7 +5,7 @@ use hir::{ }; use itertools::Itertools; use rustc_hash::FxHashSet; -use syntax::{ast, AstNode, SyntaxNode}; +use syntax::{ast, utils::path_to_string_stripping_turbo_fish, AstNode, SyntaxNode}; use crate::{ items_locator::{self, AssocItemSearch, DEFAULT_QUERY_SEARCH_LIMIT}, @@ -57,7 +57,7 @@ pub struct PathImportCandidate { #[derive(Debug)] pub struct FirstSegmentUnresolved { fist_segment: ast::NameRef, - full_qualifier: ModPath, + full_qualifier: ast::Path, } /// A name that will be used during item lookups. @@ -310,7 +310,7 @@ fn path_applicable_imports( } Some(first_segment_unresolved) => ( first_segment_unresolved.fist_segment.to_string(), - first_segment_unresolved.full_qualifier.to_string(), + path_to_string_stripping_turbo_fish(&first_segment_unresolved.full_qualifier), ), }; @@ -583,7 +583,7 @@ fn path_import_candidate( ImportCandidate::Path(PathImportCandidate { qualifier: Some(FirstSegmentUnresolved { fist_segment: qualifier_start, - full_qualifier: ModPath::from_src_unhygienic(qualifier)?, + full_qualifier: qualifier, }), name, }) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index b6c5de6588..70ba8adb48 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -91,6 +91,10 @@ pub fn path_from_segments( }) } +pub fn path_from_text(text: &str) -> ast::Path { + ast_from_text(&format!("fn main() {{ let test = {}; }}", text)) +} + pub fn glob_use_tree() -> ast::UseTree { ast_from_text("use *;") } diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index 11294c5b22..09e212e8c0 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -37,6 +37,7 @@ pub mod algo; pub mod ast; #[doc(hidden)] pub mod fuzz; +pub mod utils; use std::{marker::PhantomData, sync::Arc}; diff --git a/crates/syntax/src/utils.rs b/crates/syntax/src/utils.rs new file mode 100644 index 0000000000..f4c02518b4 --- /dev/null +++ b/crates/syntax/src/utils.rs @@ -0,0 +1,43 @@ +//! A set of utils methods to reuse on other abstraction levels + +use itertools::Itertools; + +use crate::{ast, match_ast, AstNode}; + +pub fn path_to_string_stripping_turbo_fish(path: &ast::Path) -> String { + path.syntax() + .children() + .filter_map(|node| { + match_ast! { + match node { + ast::PathSegment(it) => { + Some(it.name_ref()?.to_string()) + }, + ast::Path(it) => { + Some(path_to_string_stripping_turbo_fish(&it)) + }, + _ => None, + } + } + }) + .join("::") +} + +#[cfg(test)] +mod tests { + use super::path_to_string_stripping_turbo_fish; + use crate::ast::make; + + #[test] + fn turbofishes_are_stripped() { + assert_eq!("Vec", path_to_string_stripping_turbo_fish(&make::path_from_text("Vec::")),); + assert_eq!( + "Vec::new", + path_to_string_stripping_turbo_fish(&make::path_from_text("Vec::::new")), + ); + assert_eq!( + "Vec::new", + path_to_string_stripping_turbo_fish(&make::path_from_text("Vec::new()")), + ); + } +} From 867fdf8f03a25862c122614688c38f5e26e08e1f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Mar 2021 14:54:50 +0300 Subject: [PATCH 24/24] Improve compilation speed --- crates/syntax/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml index c39095def0..33bde099bc 100644 --- a/crates/syntax/Cargo.toml +++ b/crates/syntax/Cargo.toml @@ -25,10 +25,10 @@ serde = { version = "1.0.106", features = ["derive"] } stdx = { path = "../stdx", version = "0.0.0" } text_edit = { path = "../text_edit", version = "0.0.0" } parser = { path = "../parser", version = "0.0.0" } -test_utils = { path = "../test_utils", version = "0.0.0" } profile = { path = "../profile", version = "0.0.0" } [dev-dependencies] +test_utils = { path = "../test_utils" } walkdir = "2.3.1" rayon = "1" expect-test = "1.1"