From 8cd4e9f7ec2bc2b0663f488e5d0f7ad922911266 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Thu, 29 Jun 2023 18:39:37 +0900 Subject: [PATCH] Merge `assoc_items_only` and `exclude_import_kinds` into `assoc_mode` --- crates/hir-def/src/import_map.rs | 125 ++++-------------- .../replace_derive_with_manual_impl.rs | 2 +- crates/ide-completion/src/lib.rs | 2 +- crates/ide-db/src/imports/import_assets.rs | 8 +- crates/ide-db/src/items_locator.rs | 55 +++----- 5 files changed, 49 insertions(+), 143 deletions(-) diff --git a/crates/hir-def/src/import_map.rs b/crates/hir-def/src/import_map.rs index 1124302182b..dd5f6e575cd 100644 --- a/crates/hir-def/src/import_map.rs +++ b/crates/hir-def/src/import_map.rs @@ -286,22 +286,6 @@ fn fst_path(db: &dyn DefDatabase, path: &ImportPath) -> String { s } -#[derive(Debug, Eq, PartialEq, Hash)] -pub enum ImportKind { - Module, - Function, - Adt, - EnumVariant, - Const, - Static, - Trait, - TraitAlias, - TypeAlias, - BuiltinType, - AssociatedItem, - Macro, -} - /// A way to match import map contents against the search query. #[derive(Debug)] pub enum SearchMode { @@ -314,15 +298,25 @@ pub enum SearchMode { Fuzzy, } +/// Three possible ways to search for the name in associated and/or other items. +#[derive(Debug, Clone, Copy)] +pub enum AssocSearchMode { + /// Search for the name in both associated and other items. + Include, + /// Search for the name in other items only. + Exclude, + /// Search for the name in the associated items only. + AssocItemsOnly, +} + #[derive(Debug)] pub struct Query { query: String, lowercased: String, - assoc_items_only: bool, search_mode: SearchMode, + assoc_mode: AssocSearchMode, case_sensitive: bool, limit: usize, - exclude_import_kinds: FxHashSet, } impl Query { @@ -331,24 +325,23 @@ pub fn new(query: String) -> Self { Self { query, lowercased, - assoc_items_only: false, search_mode: SearchMode::Contains, + assoc_mode: AssocSearchMode::Include, case_sensitive: false, limit: usize::max_value(), - exclude_import_kinds: FxHashSet::default(), } } - /// Matches only the entries that are associated items, ignoring the rest. - pub fn assoc_items_only(self) -> Self { - Self { assoc_items_only: true, ..self } - } - /// Specifies the way to search for the entries using the query. pub fn search_mode(self, search_mode: SearchMode) -> Self { Self { search_mode, ..self } } + /// Specifies whether we want to include associated items in the result. + pub fn assoc_search_mode(self, assoc_mode: AssocSearchMode) -> Self { + Self { assoc_mode, ..self } + } + /// Limits the returned number of items to `limit`. pub fn limit(self, limit: usize) -> Self { Self { limit, ..self } @@ -359,12 +352,6 @@ pub fn case_sensitive(self) -> Self { Self { case_sensitive: true, ..self } } - /// Do not include imports of the specified kind in the search results. - pub fn exclude_import_kind(mut self, import_kind: ImportKind) -> Self { - self.exclude_import_kinds.insert(import_kind); - self - } - fn import_matches( &self, db: &dyn DefDatabase, @@ -372,12 +359,10 @@ fn import_matches( enforce_lowercase: bool, ) -> bool { let _p = profile::span("import_map::Query::import_matches"); - if import.is_trait_assoc_item { - if self.exclude_import_kinds.contains(&ImportKind::AssociatedItem) { - return false; - } - } else if self.assoc_items_only { - return false; + match (import.is_trait_assoc_item, self.assoc_mode) { + (true, AssocSearchMode::Exclude) => return false, + (false, AssocSearchMode::AssocItemsOnly) => return false, + _ => {} } let mut input = import.path.segments.last().unwrap().display(db.upcast()).to_string(); @@ -458,10 +443,6 @@ pub fn search_dependencies( .take_while(|item| { common_importables_path_fst == fst_path(db, &import_map.map[item].path) }) - .filter(|&item| match item_import_kind(item) { - Some(import_kind) => !query.exclude_import_kinds.contains(&import_kind), - None => true, - }) .filter(|item| { !query.case_sensitive // we've already checked the common importables path case-insensitively || query.import_matches(db, &import_map.map[item], false) @@ -476,22 +457,6 @@ pub fn search_dependencies( res } -fn item_import_kind(item: ItemInNs) -> Option { - Some(match item.as_module_def_id()? { - ModuleDefId::ModuleId(_) => ImportKind::Module, - ModuleDefId::FunctionId(_) => ImportKind::Function, - ModuleDefId::AdtId(_) => ImportKind::Adt, - ModuleDefId::EnumVariantId(_) => ImportKind::EnumVariant, - ModuleDefId::ConstId(_) => ImportKind::Const, - ModuleDefId::StaticId(_) => ImportKind::Static, - ModuleDefId::TraitId(_) => ImportKind::Trait, - ModuleDefId::TraitAliasId(_) => ImportKind::TraitAlias, - ModuleDefId::TypeAliasId(_) => ImportKind::TypeAlias, - ModuleDefId::BuiltinType(_) => ImportKind::BuiltinType, - ModuleDefId::MacroId(_) => ImportKind::Macro, - }) -} - #[cfg(test)] mod tests { use base_db::{fixture::WithFixture, SourceDatabase, Upcast}; @@ -888,7 +853,9 @@ pub trait Display { check_search( ra_fixture, "main", - Query::new("fmt".to_string()).search_mode(SearchMode::Fuzzy).assoc_items_only(), + Query::new("fmt".to_string()) + .search_mode(SearchMode::Fuzzy) + .assoc_search_mode(AssocSearchMode::AssocItemsOnly), expect![[r#" dep::fmt::Display::FMT_CONST (a) dep::fmt::Display::format_function (a) @@ -901,21 +868,11 @@ pub trait Display { "main", Query::new("fmt".to_string()) .search_mode(SearchMode::Fuzzy) - .exclude_import_kind(ImportKind::AssociatedItem), + .assoc_search_mode(AssocSearchMode::Exclude), expect![[r#" dep::fmt (t) "#]], ); - - check_search( - ra_fixture, - "main", - Query::new("fmt".to_string()) - .search_mode(SearchMode::Fuzzy) - .assoc_items_only() - .exclude_import_kind(ImportKind::AssociatedItem), - expect![[r#""#]], - ); } #[test] @@ -1101,34 +1058,4 @@ pub fn no() {} "#]], ); } - - #[test] - fn search_exclusions() { - let ra_fixture = r#" - //- /main.rs crate:main deps:dep - //- /dep.rs crate:dep - - pub struct fmt; - pub struct FMT; - "#; - - check_search( - ra_fixture, - "main", - Query::new("FMT".to_string()), - expect![[r#" - dep::FMT (t) - dep::FMT (v) - dep::fmt (t) - dep::fmt (v) - "#]], - ); - - check_search( - ra_fixture, - "main", - Query::new("FMT".to_string()).exclude_import_kind(ImportKind::Adt), - expect![[r#""#]], - ); - } } 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 3bdd795bea8..c03bc2f41d5 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 @@ -73,7 +73,7 @@ pub(crate) fn replace_derive_with_manual_impl( &ctx.sema, current_crate, NameToImport::exact_case_sensitive(path.segments().last()?.to_string()), - items_locator::AssocItemSearch::Exclude, + items_locator::AssocSearchMode::Exclude, Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT.inner()), ) .filter_map(|item| match item.as_module_def()? { diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index 106d4e1e52f..2eaa42040a0 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -231,7 +231,7 @@ pub fn resolve_completion_edits( &sema, current_crate, NameToImport::exact_case_sensitive(imported_name), - items_locator::AssocItemSearch::Include, + items_locator::AssocSearchMode::Include, Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT.inner()), ); let import = items_with_name diff --git a/crates/ide-db/src/imports/import_assets.rs b/crates/ide-db/src/imports/import_assets.rs index 901d592c691..e52dc356775 100644 --- a/crates/ide-db/src/imports/import_assets.rs +++ b/crates/ide-db/src/imports/import_assets.rs @@ -13,7 +13,7 @@ use crate::{ helpers::item_name, - items_locator::{self, AssocItemSearch, DEFAULT_QUERY_SEARCH_LIMIT}, + items_locator::{self, AssocSearchMode, DEFAULT_QUERY_SEARCH_LIMIT}, RootDatabase, }; @@ -317,7 +317,7 @@ fn path_applicable_imports( // * improve the associated completion item matching and/or scoring to ensure no noisy completions appear // // see also an ignored test under FIXME comment in the qualify_path.rs module - AssocItemSearch::Exclude, + AssocSearchMode::Exclude, Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()), ) .filter_map(|item| { @@ -334,7 +334,7 @@ fn path_applicable_imports( sema, current_crate, path_candidate.name.clone(), - AssocItemSearch::Include, + AssocSearchMode::Include, Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()), ) .filter_map(|item| { @@ -483,7 +483,7 @@ fn trait_applicable_items( sema, current_crate, trait_candidate.assoc_item_name.clone(), - AssocItemSearch::AssocItemsOnly, + AssocSearchMode::AssocItemsOnly, Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()), ) .filter_map(|input| item_as_assoc(db, input)) diff --git a/crates/ide-db/src/items_locator.rs b/crates/ide-db/src/items_locator.rs index 5ba6df3694f..8fab33f38be 100644 --- a/crates/ide-db/src/items_locator.rs +++ b/crates/ide-db/src/items_locator.rs @@ -3,10 +3,7 @@ //! The main reason for this module to exist is the fact that project's items and dependencies' items //! are located in different caches, with different APIs. use either::Either; -use hir::{ - import_map::{self, ImportKind}, - AsAssocItem, Crate, ItemInNs, Semantics, -}; +use hir::{import_map, AsAssocItem, Crate, ItemInNs, Semantics}; use limit::Limit; use crate::{imports::import_assets::NameToImport, symbol_index, RootDatabase}; @@ -14,23 +11,14 @@ /// A value to use, when uncertain which limit to pick. pub static DEFAULT_QUERY_SEARCH_LIMIT: Limit = Limit::new(40); -/// Three possible ways to search for the name in associated and/or other items. -#[derive(Debug, Clone, Copy)] -pub enum AssocItemSearch { - /// Search for the name in both associated and other items. - Include, - /// Search for the name in other items only. - Exclude, - /// Search for the name in the associated items only. - AssocItemsOnly, -} +pub use import_map::AssocSearchMode; /// Searches for importable items with the given name in the crate and its dependencies. pub fn items_with_name<'a>( sema: &'a Semantics<'_, RootDatabase>, krate: Crate, name: NameToImport, - assoc_item_search: AssocItemSearch, + assoc_item_search: AssocSearchMode, limit: Option, ) -> impl Iterator + 'a { let _p = profile::span("items_with_name").detail(|| { @@ -60,16 +48,8 @@ pub fn items_with_name<'a>( let mut local_query = symbol_index::Query::new(fuzzy_search_string.clone()); let mut external_query = import_map::Query::new(fuzzy_search_string.clone()) - .search_mode(import_map::SearchMode::Fuzzy); - match assoc_item_search { - AssocItemSearch::Include => {} - AssocItemSearch::Exclude => { - external_query = external_query.exclude_import_kind(ImportKind::AssociatedItem); - } - AssocItemSearch::AssocItemsOnly => { - external_query = external_query.assoc_items_only(); - } - } + .search_mode(import_map::SearchMode::Fuzzy) + .assoc_search_mode(assoc_item_search); if fuzzy_search_string.to_lowercase() != fuzzy_search_string { local_query.case_sensitive(); @@ -91,13 +71,15 @@ pub fn items_with_name<'a>( fn find_items<'a>( sema: &'a Semantics<'_, RootDatabase>, krate: Crate, - assoc_item_search: AssocItemSearch, + assoc_item_search: AssocSearchMode, local_query: symbol_index::Query, external_query: import_map::Query, ) -> impl Iterator + 'a { let _p = profile::span("find_items"); let db = sema.db; + // NOTE: `external_query` includes `assoc_item_search`, so we don't need to + // filter on our own. let external_importables = krate.query_external_importables(db, external_query).map(|external_importable| { match external_importable { @@ -110,18 +92,15 @@ fn find_items<'a>( let local_results = local_query .search(&symbol_index::crate_symbols(db, krate)) .into_iter() - .filter_map(|local_candidate| match local_candidate.def { - hir::ModuleDef::Macro(macro_def) => Some(ItemInNs::Macros(macro_def)), - def => Some(ItemInNs::from(def)), + .filter(move |candidate| match assoc_item_search { + AssocSearchMode::Include => true, + AssocSearchMode::Exclude => candidate.def.as_assoc_item(db).is_none(), + AssocSearchMode::AssocItemsOnly => candidate.def.as_assoc_item(db).is_some(), + }) + .map(|local_candidate| match local_candidate.def { + hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), + def => ItemInNs::from(def), }); - external_importables.chain(local_results).filter(move |&item| match assoc_item_search { - AssocItemSearch::Include => true, - AssocItemSearch::Exclude => !is_assoc_item(item, sema.db), - AssocItemSearch::AssocItemsOnly => is_assoc_item(item, sema.db), - }) -} - -fn is_assoc_item(item: ItemInNs, db: &RootDatabase) -> bool { - item.as_module_def().and_then(|module_def| module_def.as_assoc_item(db)).is_some() + external_importables.chain(local_results) }