From 1bfc3a50c0febbfa9b56de01997d081814d98e39 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 31 Dec 2020 02:18:15 +0200 Subject: Add a basic test for the trait fuzzy import --- crates/hir_def/src/import_map.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 30b22f51d..810a4a268 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -748,6 +748,30 @@ mod tests { ); } + #[test] + fn fuzzy_import_trait() { + let ra_fixture = r#" + //- /main.rs crate:main deps:dep + //- /dep.rs crate:dep + pub mod fmt { + pub trait Display { + fn fmttt(); + } + } + "#; + + check_search( + ra_fixture, + "main", + Query::new("fmt".to_string()).search_mode(SearchMode::Fuzzy), + expect![[r#" + dep::fmt (t) + dep::fmt::Display (t) + dep::fmt::Display::fmttt (f) + "#]], + ); + } + #[test] fn search_mode() { let ra_fixture = r#" -- cgit v1.2.3 From 63d83fa38584fa0537f896bee5d90709c1352030 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 2 Jan 2021 02:05:09 +0200 Subject: Add associated data into fst --- crates/hir_def/src/import_map.rs | 94 +++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 810a4a268..6283c1f1a 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -7,9 +7,7 @@ use fst::{self, Streamer}; use hir_expand::name::Name; use indexmap::{map::Entry, IndexMap}; use itertools::Itertools; -use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; -use smallvec::SmallVec; -use syntax::SmolStr; +use rustc_hash::{FxHashSet, FxHasher}; use crate::{ db::DefDatabase, item_scope::ItemInNs, visibility::Visibility, AssocItemId, ModuleDefId, @@ -25,6 +23,8 @@ pub struct ImportInfo { pub path: ImportPath, /// The module containing this item. pub container: ModuleId, + /// Whether the import is a trait associated item or not. + pub is_assoc_item: bool, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -64,10 +64,6 @@ pub struct ImportMap { /// the index of the first one. importables: Vec, fst: fst::Map>, - - /// Maps names of associated items to the item's ID. Only includes items whose defining trait is - /// exported. - assoc_map: FxHashMap>, } impl ImportMap { @@ -108,14 +104,22 @@ impl ImportMap { for item in per_ns.iter_items() { let path = mk_path(); + let path_len = path.len(); + let import_info = ImportInfo { path, container: module, is_assoc_item: false }; + + // If we've added a path to a trait, add the trait's associated items to the assoc map. + if let Some(ModuleDefId::TraitId(tr)) = item.as_module_def_id() { + import_map.collect_trait_assoc_items(db, tr, &import_info); + } + match import_map.map.entry(item) { Entry::Vacant(entry) => { - entry.insert(ImportInfo { path, container: module }); + entry.insert(import_info); } Entry::Occupied(mut entry) => { // If the new path is shorter, prefer that one. - if path.len() < entry.get().path.len() { - *entry.get_mut() = ImportInfo { path, container: module }; + if path_len < entry.get().path.len() { + *entry.get_mut() = import_info; } else { continue; } @@ -128,11 +132,6 @@ impl ImportMap { if let Some(ModuleDefId::ModuleId(mod_id)) = item.as_module_def_id() { worklist.push((mod_id, mk_path())); } - - // If we've added a path to a trait, add the trait's methods to the method map. - if let Some(ModuleDefId::TraitId(tr)) = item.as_module_def_id() { - import_map.collect_trait_methods(db, tr); - } } } } @@ -153,12 +152,10 @@ impl ImportMap { } } - let start = last_batch_start; - last_batch_start = idx + 1; + let key = fst_path(&importables[last_batch_start].1.path); + builder.insert(key, last_batch_start as u64).unwrap(); - let key = fst_path(&importables[start].1.path); - - builder.insert(key, start as u64).unwrap(); + last_batch_start = idx + 1; } import_map.fst = fst::Map::new(builder.into_inner().unwrap()).unwrap(); @@ -176,10 +173,22 @@ impl ImportMap { self.map.get(&item) } - fn collect_trait_methods(&mut self, db: &dyn DefDatabase, tr: TraitId) { - let data = db.trait_data(tr); - for (name, item) in data.items.iter() { - self.assoc_map.entry(name.to_string().into()).or_default().push(*item); + fn collect_trait_assoc_items( + &mut self, + db: &dyn DefDatabase, + tr: TraitId, + import_info: &ImportInfo, + ) { + for (assoc_item_name, item) in db.trait_data(tr).items.iter() { + let assoc_item = ItemInNs::Types(match item.clone() { + AssocItemId::FunctionId(f) => f.into(), + AssocItemId::ConstId(c) => c.into(), + AssocItemId::TypeAliasId(t) => t.into(), + }); + let mut assoc_item_info = import_info.to_owned(); + assoc_item_info.path.segments.push(assoc_item_name.to_owned()); + assoc_item_info.is_assoc_item = true; + self.map.insert(assoc_item, assoc_item_info); } } } @@ -304,11 +313,11 @@ impl Query { } } -fn contains_query(query: &Query, input_path: &ImportPath, enforce_lowercase: bool) -> bool { - let mut input = if query.name_only { - input_path.segments.last().unwrap().to_string() +fn import_matches_query(import: &ImportInfo, query: &Query, enforce_lowercase: bool) -> bool { + let mut input = if import.is_assoc_item || query.name_only { + import.path.segments.last().unwrap().to_string() } else { - input_path.to_string() + import.path.to_string() }; if enforce_lowercase || !query.case_sensitive { input.make_ascii_lowercase(); @@ -366,13 +375,13 @@ pub fn search_dependencies<'a>( let import_map = &import_maps[indexed_value.index]; let importables = &import_map.importables[indexed_value.value as usize..]; - // Path shared by the importable items in this group. - let common_importables_path = &import_map.map[&importables[0]].path; - if !contains_query(&query, common_importables_path, true) { + let common_importable_data = &import_map.map[&importables[0]]; + if !import_matches_query(common_importable_data, &query, true) { continue; } - let common_importables_path_fst = fst_path(common_importables_path); + // Path shared by the importable items in this group. + let common_importables_path_fst = fst_path(&common_importable_data.path); // Add the items from this `ModPath` group. Those are all subsequent items in // `importables` whose paths match `path`. let iter = importables @@ -387,7 +396,7 @@ pub fn search_dependencies<'a>( }) .filter(|item| { !query.case_sensitive // we've already checked the common importables path case-insensitively - || contains_query(&query, &import_map.map[item].path, false) + || import_matches_query(&import_map.map[item], &query, false) }); res.extend(iter); @@ -398,19 +407,6 @@ pub fn search_dependencies<'a>( } } - // Add all exported associated items whose names match the query (exactly). - for map in &import_maps { - if let Some(v) = map.assoc_map.get(&*query.query) { - res.extend(v.iter().map(|&assoc| { - ItemInNs::Types(match assoc { - AssocItemId::FunctionId(it) => it.into(), - AssocItemId::ConstId(it) => it.into(), - AssocItemId::TypeAliasId(it) => it.into(), - }) - })); - } - } - res } @@ -755,7 +751,7 @@ mod tests { //- /dep.rs crate:dep pub mod fmt { pub trait Display { - fn fmttt(); + fn format(); } } "#; @@ -767,7 +763,7 @@ mod tests { expect![[r#" dep::fmt (t) dep::fmt::Display (t) - dep::fmt::Display::fmttt (f) + dep::fmt::Display::format (f) "#]], ); } @@ -808,8 +804,8 @@ mod tests { dep::Fmt (v) dep::Fmt (m) dep::fmt::Display (t) - dep::format (f) dep::fmt::Display::fmt (f) + dep::format (f) "#]], ); -- cgit v1.2.3 From d27dea86b7dd4123f75ad176037b3c754eddfa65 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 3 Jan 2021 12:08:08 +0200 Subject: Properly check assoc items lookup --- crates/hir_def/src/import_map.rs | 105 ++++++++++++++++++++++----------------- crates/hir_def/src/item_scope.rs | 14 +++++- 2 files changed, 73 insertions(+), 46 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 6283c1f1a..0ba8198b5 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -428,9 +428,8 @@ fn item_import_kind(item: ItemInNs) -> Option { mod tests { use base_db::{fixture::WithFixture, SourceDatabase, Upcast}; use expect_test::{expect, Expect}; - use stdx::format_to; - use crate::{data::FunctionData, test_db::TestDB, AssocContainerId, Lookup}; + use crate::{test_db::TestDB, AssocContainerId, Lookup}; use super::*; @@ -447,46 +446,55 @@ mod tests { let actual = search_dependencies(db.upcast(), krate, query) .into_iter() - .filter_map(|item| { - let mark = match item { - ItemInNs::Types(ModuleDefId::FunctionId(_)) - | ItemInNs::Values(ModuleDefId::FunctionId(_)) => "f", - ItemInNs::Types(_) => "t", - ItemInNs::Values(_) => "v", - ItemInNs::Macros(_) => "m", + .filter_map(|dependency| { + let dependency_krate = dependency.krate(db.upcast())?; + let dependency_imports = db.import_map(dependency_krate); + + let (path, mark) = match assoc_item_path(&db, &dependency_imports, dependency) { + Some(assoc_item_path) => (assoc_item_path, "a"), + None => ( + dependency_imports.path_of(dependency)?.to_string(), + match dependency { + ItemInNs::Types(_) => "t", + ItemInNs::Values(_) => "v", + ItemInNs::Macros(_) => "m", + }, + ), }; - item.krate(db.upcast()).map(|krate| { - let map = db.import_map(krate); - - let path = match assoc_to_trait(&db, item) { - Some(trait_) => { - let mut full_path = map.path_of(trait_).unwrap().to_string(); - if let ItemInNs::Types(ModuleDefId::FunctionId(function_id)) - | ItemInNs::Values(ModuleDefId::FunctionId(function_id)) = item - { - format_to!( - full_path, - "::{}", - FunctionData::fn_data_query(&db, function_id).name, - ); - } - full_path - } - None => map.path_of(item).unwrap().to_string(), - }; - - format!( - "{}::{} ({})\n", - crate_graph[krate].display_name.as_ref().unwrap(), - path, - mark - ) - }) + + Some(format!( + "{}::{} ({})\n", + crate_graph[dependency_krate].display_name.as_ref()?, + path, + mark + )) }) .collect::(); expect.assert_eq(&actual) } + fn assoc_item_path( + db: &dyn DefDatabase, + dependency_imports: &ImportMap, + dependency: ItemInNs, + ) -> Option { + let dependency_assoc_item_id = dependency.as_assoc_item_id()?; + let trait_ = assoc_to_trait(db, dependency)?; + if let ModuleDefId::TraitId(tr) = trait_.as_module_def_id()? { + let trait_data = db.trait_data(tr); + let assoc_item_name = + trait_data.items.iter().find_map(|(assoc_item_name, assoc_item_id)| { + if &dependency_assoc_item_id == assoc_item_id { + Some(assoc_item_name) + } else { + None + } + })?; + return Some(format!("{}::{}", dependency_imports.path_of(trait_)?, assoc_item_name)); + } + None + } + fn assoc_to_trait(db: &dyn DefDatabase, item: ItemInNs) -> Option { let assoc: AssocItemId = match item { ItemInNs::Types(it) | ItemInNs::Values(it) => match it { @@ -745,13 +753,17 @@ mod tests { } #[test] - fn fuzzy_import_trait() { + fn fuzzy_import_trait_and_assoc_items() { let ra_fixture = r#" //- /main.rs crate:main deps:dep //- /dep.rs crate:dep pub mod fmt { pub trait Display { - fn format(); + type FmtTypeAlias; + const FMT_CONST: bool; + + fn format_function(); + fn format_method(&self); } } "#; @@ -763,7 +775,10 @@ mod tests { expect![[r#" dep::fmt (t) dep::fmt::Display (t) - dep::fmt::Display::format (f) + dep::fmt::Display::FMT_CONST (a) + dep::fmt::Display::FmtTypeAlias (a) + dep::fmt::Display::format_function (a) + dep::fmt::Display::format_method (a) "#]], ); } @@ -804,8 +819,8 @@ mod tests { dep::Fmt (v) dep::Fmt (m) dep::fmt::Display (t) - dep::fmt::Display::fmt (f) - dep::format (f) + dep::fmt::Display::fmt (a) + dep::format (v) "#]], ); @@ -818,7 +833,7 @@ mod tests { dep::Fmt (t) dep::Fmt (v) dep::Fmt (m) - dep::fmt::Display::fmt (f) + dep::fmt::Display::fmt (a) "#]], ); @@ -832,7 +847,7 @@ mod tests { dep::Fmt (v) dep::Fmt (m) dep::fmt::Display (t) - dep::fmt::Display::fmt (f) + dep::fmt::Display::fmt (a) "#]], ); } @@ -873,7 +888,7 @@ mod tests { dep::Fmt (v) dep::Fmt (m) dep::fmt::Display (t) - dep::fmt::Display::fmt (f) + dep::fmt::Display::fmt (a) "#]], ); @@ -886,7 +901,7 @@ mod tests { dep::Fmt (t) dep::Fmt (v) dep::Fmt (m) - dep::fmt::Display::fmt (f) + dep::fmt::Display::fmt (a) "#]], ); } diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index 62ab3b2bd..4da1c3ab0 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -10,11 +10,11 @@ use once_cell::sync::Lazy; use rustc_hash::{FxHashMap, FxHashSet}; use test_utils::mark; -use crate::ModuleId; use crate::{ db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, HasModule, ImplId, LocalModuleId, Lookup, MacroDefId, ModuleDefId, TraitId, }; +use crate::{AssocItemId, ModuleId}; #[derive(Copy, Clone)] pub(crate) enum ImportType { @@ -349,6 +349,18 @@ impl ItemInNs { } } + pub fn as_assoc_item_id(self) -> Option { + match self { + ItemInNs::Types(ModuleDefId::FunctionId(id)) + | ItemInNs::Values(ModuleDefId::FunctionId(id)) => Some(id.into()), + ItemInNs::Types(ModuleDefId::ConstId(id)) + | ItemInNs::Values(ModuleDefId::ConstId(id)) => Some(id.into()), + ItemInNs::Types(ModuleDefId::TypeAliasId(id)) + | ItemInNs::Values(ModuleDefId::TypeAliasId(id)) => Some(id.into()), + _ => None, + } + } + /// Returns the crate defining this item (or `None` if `self` is built-in). pub fn krate(&self, db: &dyn DefDatabase) -> Option { Some(match self { -- cgit v1.2.3 From 8721574a85399cd74319e93c84dc19c7ed7c6605 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 3 Jan 2021 12:24:50 +0200 Subject: Simplify --- crates/hir_def/src/import_map.rs | 49 +++++++++++++++++++++++++++++----------- crates/hir_def/src/item_scope.rs | 15 +----------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 0ba8198b5..a8bffe7ce 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -24,7 +24,7 @@ pub struct ImportInfo { /// The module containing this item. pub container: ModuleId, /// Whether the import is a trait associated item or not. - pub is_assoc_item: bool, + pub is_trait_assoc_item: bool, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -105,11 +105,16 @@ impl ImportMap { for item in per_ns.iter_items() { let path = mk_path(); let path_len = path.len(); - let import_info = ImportInfo { path, container: module, is_assoc_item: false }; + let import_info = + ImportInfo { path, container: module, is_trait_assoc_item: false }; - // If we've added a path to a trait, add the trait's associated items to the assoc map. if let Some(ModuleDefId::TraitId(tr)) = item.as_module_def_id() { - import_map.collect_trait_assoc_items(db, tr, &import_info); + import_map.collect_trait_assoc_items( + db, + tr, + matches!(item, ItemInNs::Types(_)), + &import_info, + ); } match import_map.map.entry(item) { @@ -177,17 +182,24 @@ impl ImportMap { &mut self, db: &dyn DefDatabase, tr: TraitId, - import_info: &ImportInfo, + is_type_in_ns: bool, + original_import_info: &ImportInfo, ) { - for (assoc_item_name, item) in db.trait_data(tr).items.iter() { - let assoc_item = ItemInNs::Types(match item.clone() { + for (assoc_item_name, item) in &db.trait_data(tr).items { + let module_def_id = match *item { AssocItemId::FunctionId(f) => f.into(), AssocItemId::ConstId(c) => c.into(), AssocItemId::TypeAliasId(t) => t.into(), - }); - let mut assoc_item_info = import_info.to_owned(); + }; + let assoc_item = if is_type_in_ns { + ItemInNs::Types(module_def_id) + } else { + ItemInNs::Values(module_def_id) + }; + + let mut assoc_item_info = original_import_info.to_owned(); assoc_item_info.path.segments.push(assoc_item_name.to_owned()); - assoc_item_info.is_assoc_item = true; + assoc_item_info.is_trait_assoc_item = true; self.map.insert(assoc_item, assoc_item_info); } } @@ -314,7 +326,7 @@ impl Query { } fn import_matches_query(import: &ImportInfo, query: &Query, enforce_lowercase: bool) -> bool { - let mut input = if import.is_assoc_item || query.name_only { + let mut input = if import.is_trait_assoc_item || query.name_only { import.path.segments.last().unwrap().to_string() } else { import.path.to_string() @@ -455,6 +467,8 @@ mod tests { None => ( dependency_imports.path_of(dependency)?.to_string(), match dependency { + ItemInNs::Types(ModuleDefId::FunctionId(_)) + | ItemInNs::Values(ModuleDefId::FunctionId(_)) => "f", ItemInNs::Types(_) => "t", ItemInNs::Values(_) => "v", ItemInNs::Macros(_) => "m", @@ -478,7 +492,16 @@ mod tests { dependency_imports: &ImportMap, dependency: ItemInNs, ) -> Option { - let dependency_assoc_item_id = dependency.as_assoc_item_id()?; + let dependency_assoc_item_id = match dependency { + ItemInNs::Types(ModuleDefId::FunctionId(id)) + | ItemInNs::Values(ModuleDefId::FunctionId(id)) => AssocItemId::from(id), + ItemInNs::Types(ModuleDefId::ConstId(id)) + | ItemInNs::Values(ModuleDefId::ConstId(id)) => AssocItemId::from(id), + ItemInNs::Types(ModuleDefId::TypeAliasId(id)) + | ItemInNs::Values(ModuleDefId::TypeAliasId(id)) => AssocItemId::from(id), + _ => return None, + }; + let trait_ = assoc_to_trait(db, dependency)?; if let ModuleDefId::TraitId(tr) = trait_.as_module_def_id()? { let trait_data = db.trait_data(tr); @@ -820,7 +843,7 @@ mod tests { dep::Fmt (m) dep::fmt::Display (t) dep::fmt::Display::fmt (a) - dep::format (v) + dep::format (f) "#]], ); diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index 4da1c3ab0..2750e1c91 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -12,9 +12,8 @@ use test_utils::mark; use crate::{ db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, HasModule, ImplId, - LocalModuleId, Lookup, MacroDefId, ModuleDefId, TraitId, + LocalModuleId, Lookup, MacroDefId, ModuleDefId, ModuleId, TraitId, }; -use crate::{AssocItemId, ModuleId}; #[derive(Copy, Clone)] pub(crate) enum ImportType { @@ -349,18 +348,6 @@ impl ItemInNs { } } - pub fn as_assoc_item_id(self) -> Option { - match self { - ItemInNs::Types(ModuleDefId::FunctionId(id)) - | ItemInNs::Values(ModuleDefId::FunctionId(id)) => Some(id.into()), - ItemInNs::Types(ModuleDefId::ConstId(id)) - | ItemInNs::Values(ModuleDefId::ConstId(id)) => Some(id.into()), - ItemInNs::Types(ModuleDefId::TypeAliasId(id)) - | ItemInNs::Values(ModuleDefId::TypeAliasId(id)) => Some(id.into()), - _ => None, - } - } - /// Returns the crate defining this item (or `None` if `self` is built-in). pub fn krate(&self, db: &dyn DefDatabase) -> Option { Some(match self { -- cgit v1.2.3 From ec316cb21117dbefacd67af2f33779fb2d66fdea Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 3 Jan 2021 15:58:15 +0200 Subject: Ignore associated items during unqialified path fuzzy completions --- crates/completion/src/completions/unqualified_path.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index 81a6d00e2..068b6b407 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -3,7 +3,7 @@ use std::iter; use either::Either; -use hir::{Adt, ModPath, ModuleDef, ScopeDef, Type}; +use hir::{Adt, AsAssocItem, ModPath, ModuleDef, ScopeDef, Type}; use ide_db::helpers::insert_use::ImportScope; use ide_db::imports_locator; use syntax::AstNode; @@ -143,6 +143,14 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<() potential_import_name, true, ) + .filter(|import_candidate| match import_candidate { + Either::Left(ModuleDef::Function(function)) => function.as_assoc_item(ctx.db).is_none(), + Either::Left(ModuleDef::Const(const_)) => const_.as_assoc_item(ctx.db).is_none(), + Either::Left(ModuleDef::TypeAlias(type_alias)) => { + type_alias.as_assoc_item(ctx.db).is_none() + } + _ => true, + }) .filter_map(|import_candidate| { Some(match import_candidate { Either::Left(module_def) => { -- cgit v1.2.3 From ed1ef3ae13a8f7bc2144bd06d0271d9a6e3205a0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 3 Jan 2021 16:16:09 +0200 Subject: Do not collect trait type aliases --- crates/hir_def/src/import_map.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index a8bffe7ce..dd3a7198f 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -186,10 +186,12 @@ impl ImportMap { original_import_info: &ImportInfo, ) { for (assoc_item_name, item) in &db.trait_data(tr).items { - let module_def_id = match *item { - AssocItemId::FunctionId(f) => f.into(), - AssocItemId::ConstId(c) => c.into(), - AssocItemId::TypeAliasId(t) => t.into(), + let module_def_id = match item { + AssocItemId::FunctionId(f) => ModuleDefId::from(*f), + AssocItemId::ConstId(c) => ModuleDefId::from(*c), + // cannot use associated type aliases directly: need a `::TypeAlias` + // qualifier, ergo no need to store it for imports in import_map + AssocItemId::TypeAliasId(_) => continue, }; let assoc_item = if is_type_in_ns { ItemInNs::Types(module_def_id) @@ -799,7 +801,6 @@ mod tests { dep::fmt (t) dep::fmt::Display (t) dep::fmt::Display::FMT_CONST (a) - dep::fmt::Display::FmtTypeAlias (a) dep::fmt::Display::format_function (a) dep::fmt::Display::format_method (a) "#]], -- cgit v1.2.3 From ca42a52051e69d441b3f32ba4268286a8f0d8685 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 4 Jan 2021 18:33:05 +0200 Subject: Code review fixes --- .../completion/src/completions/unqualified_path.rs | 11 +--- crates/hir_def/src/import_map.rs | 60 +++++++++++----------- crates/ide_db/src/imports_locator.rs | 21 ++++++-- 3 files changed, 50 insertions(+), 42 deletions(-) diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index 068b6b407..f376ded57 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -3,7 +3,7 @@ use std::iter; use either::Either; -use hir::{Adt, AsAssocItem, ModPath, ModuleDef, ScopeDef, Type}; +use hir::{Adt, ModPath, ModuleDef, ScopeDef, Type}; use ide_db::helpers::insert_use::ImportScope; use ide_db::imports_locator; use syntax::AstNode; @@ -142,15 +142,8 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<() Some(40), potential_import_name, true, + true, ) - .filter(|import_candidate| match import_candidate { - Either::Left(ModuleDef::Function(function)) => function.as_assoc_item(ctx.db).is_none(), - Either::Left(ModuleDef::Const(const_)) => const_.as_assoc_item(ctx.db).is_none(), - Either::Left(ModuleDef::TypeAlias(type_alias)) => { - type_alias.as_assoc_item(ctx.db).is_none() - } - _ => true, - }) .filter_map(|import_candidate| { Some(match import_candidate { Either::Left(module_def) => { diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index dd3a7198f..1325a93d1 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -199,7 +199,7 @@ impl ImportMap { ItemInNs::Values(module_def_id) }; - let mut assoc_item_info = original_import_info.to_owned(); + let mut assoc_item_info = original_import_info.clone(); assoc_item_info.path.segments.push(assoc_item_name.to_owned()); assoc_item_info.is_trait_assoc_item = true; self.map.insert(assoc_item, assoc_item_info); @@ -325,38 +325,38 @@ impl Query { self.exclude_import_kinds.insert(import_kind); self } -} -fn import_matches_query(import: &ImportInfo, query: &Query, enforce_lowercase: bool) -> bool { - let mut input = if import.is_trait_assoc_item || query.name_only { - import.path.segments.last().unwrap().to_string() - } else { - import.path.to_string() - }; - if enforce_lowercase || !query.case_sensitive { - input.make_ascii_lowercase(); - } + fn import_matches(&self, import: &ImportInfo, enforce_lowercase: bool) -> bool { + let mut input = if import.is_trait_assoc_item || self.name_only { + import.path.segments.last().unwrap().to_string() + } else { + import.path.to_string() + }; + if enforce_lowercase || !self.case_sensitive { + input.make_ascii_lowercase(); + } - let query_string = - if !enforce_lowercase && query.case_sensitive { &query.query } else { &query.lowercased }; - - match query.search_mode { - SearchMode::Equals => &input == query_string, - SearchMode::Contains => input.contains(query_string), - SearchMode::Fuzzy => { - let mut unchecked_query_chars = query_string.chars(); - let mut mismatching_query_char = unchecked_query_chars.next(); - - for input_char in input.chars() { - match mismatching_query_char { - None => return true, - Some(matching_query_char) if matching_query_char == input_char => { - mismatching_query_char = unchecked_query_chars.next(); + let query_string = + if !enforce_lowercase && self.case_sensitive { &self.query } else { &self.lowercased }; + + match self.search_mode { + SearchMode::Equals => &input == query_string, + SearchMode::Contains => input.contains(query_string), + SearchMode::Fuzzy => { + let mut unchecked_query_chars = query_string.chars(); + let mut mismatching_query_char = unchecked_query_chars.next(); + + for input_char in input.chars() { + match mismatching_query_char { + None => return true, + Some(matching_query_char) if matching_query_char == input_char => { + mismatching_query_char = unchecked_query_chars.next(); + } + _ => (), } - _ => (), } + mismatching_query_char.is_none() } - mismatching_query_char.is_none() } } } @@ -390,7 +390,7 @@ pub fn search_dependencies<'a>( let importables = &import_map.importables[indexed_value.value as usize..]; let common_importable_data = &import_map.map[&importables[0]]; - if !import_matches_query(common_importable_data, &query, true) { + if !query.import_matches(common_importable_data, true) { continue; } @@ -410,7 +410,7 @@ pub fn search_dependencies<'a>( }) .filter(|item| { !query.case_sensitive // we've already checked the common importables path case-insensitively - || import_matches_query(&import_map.map[item], &query, false) + || query.import_matches(&import_map.map[item], false) }); res.extend(iter); diff --git a/crates/ide_db/src/imports_locator.rs b/crates/ide_db/src/imports_locator.rs index 0f4c2ca47..0782ab070 100644 --- a/crates/ide_db/src/imports_locator.rs +++ b/crates/ide_db/src/imports_locator.rs @@ -1,7 +1,7 @@ //! This module contains an import search funcionality 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 hir::{import_map, Crate, MacroDef, ModuleDef, Semantics}; +use hir::{import_map, AsAssocItem, Crate, MacroDef, ModuleDef, Semantics}; use syntax::{ast, AstNode, SyntaxKind::NAME}; use crate::{ @@ -40,8 +40,9 @@ pub fn find_similar_imports<'a>( krate: Crate, limit: Option, fuzzy_search_string: String, + ignore_assoc_items: bool, name_only: bool, -) -> impl Iterator> { +) -> impl Iterator> + 'a { let _p = profile::span("find_similar_imports"); let mut external_query = import_map::Query::new(fuzzy_search_string.clone()) @@ -57,7 +58,21 @@ pub fn find_similar_imports<'a>( external_query = external_query.limit(limit); } - find_imports(sema, krate, local_query, external_query) + let db = sema.db; + find_imports(sema, krate, local_query, external_query).filter(move |import_candidate| { + if ignore_assoc_items { + match import_candidate { + Either::Left(ModuleDef::Function(function)) => function.as_assoc_item(db).is_none(), + Either::Left(ModuleDef::Const(const_)) => const_.as_assoc_item(db).is_none(), + Either::Left(ModuleDef::TypeAlias(type_alias)) => { + type_alias.as_assoc_item(db).is_none() + } + _ => true, + } + } else { + true + } + }) } fn find_imports<'a>( -- cgit v1.2.3 From 27b3b13824f223b69558c84de264267bd3fee9c2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 4 Jan 2021 22:01:35 +0200 Subject: Small helpers --- crates/completion/src/completions/unqualified_path.rs | 2 +- crates/hir_def/src/import_map.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index f376ded57..2f41a3f96 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -124,8 +124,8 @@ fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &T // 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. fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - let _p = profile::span("fuzzy_completion"); let potential_import_name = ctx.token.to_string(); + let _p = profile::span("fuzzy_completion").detail(|| potential_import_name.clone()); if potential_import_name.len() < 2 { return None; diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 1325a93d1..59206cab8 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -8,6 +8,7 @@ use hir_expand::name::Name; use indexmap::{map::Entry, IndexMap}; use itertools::Itertools; use rustc_hash::{FxHashSet, FxHasher}; +use test_utils::mark; use crate::{ db::DefDatabase, item_scope::ItemInNs, visibility::Visibility, AssocItemId, ModuleDefId, @@ -185,6 +186,7 @@ impl ImportMap { is_type_in_ns: bool, original_import_info: &ImportInfo, ) { + mark::hit!(type_aliases_ignored); for (assoc_item_name, item) in &db.trait_data(tr).items { let module_def_id = match item { AssocItemId::FunctionId(f) => ModuleDefId::from(*f), @@ -442,6 +444,7 @@ fn item_import_kind(item: ItemInNs) -> Option { mod tests { use base_db::{fixture::WithFixture, SourceDatabase, Upcast}; use expect_test::{expect, Expect}; + use test_utils::mark; use crate::{test_db::TestDB, AssocContainerId, Lookup}; @@ -779,6 +782,7 @@ mod tests { #[test] fn fuzzy_import_trait_and_assoc_items() { + mark::check!(type_aliases_ignored); let ra_fixture = r#" //- /main.rs crate:main deps:dep //- /dep.rs crate:dep -- cgit v1.2.3 From 543e950e305aa7bd7e0b185753752d0561c70459 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 5 Jan 2021 14:03:58 +0200 Subject: Move the test mark --- crates/hir_def/src/import_map.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 59206cab8..e5368b293 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -186,14 +186,16 @@ impl ImportMap { is_type_in_ns: bool, original_import_info: &ImportInfo, ) { - mark::hit!(type_aliases_ignored); for (assoc_item_name, item) in &db.trait_data(tr).items { let module_def_id = match item { AssocItemId::FunctionId(f) => ModuleDefId::from(*f), AssocItemId::ConstId(c) => ModuleDefId::from(*c), // cannot use associated type aliases directly: need a `::TypeAlias` // qualifier, ergo no need to store it for imports in import_map - AssocItemId::TypeAliasId(_) => continue, + AssocItemId::TypeAliasId(_) => { + mark::hit!(type_aliases_ignored); + continue; + } }; let assoc_item = if is_type_in_ns { ItemInNs::Types(module_def_id) -- cgit v1.2.3