Merge assoc_items_only and exclude_import_kinds into assoc_mode

Author: lowr

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<ImportKind>,
 }
 
 impl Query {
@@ -331,24 +325,23 @@ impl Query {
         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,25 +352,17 @@ impl Query {
         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,
         import: &ImportInfo,
         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<ImportKind> {
-    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 @@ mod tests {
         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 @@ mod tests {
             "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 @@ mod tests {
             "#]],
         );
     }
-
-    #[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#""#]],
-        );
-    }
 }

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()? {

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

crates/ide-db/src/imports/import_assets.rs

@@ -13,7 +13,7 @@ use syntax::{
 
 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))

crates/ide-db/src/items_locator.rs

@@ -3,34 +3,22 @@
 //! 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};
 
 /// 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<usize>,
 ) -> impl Iterator<Item = ItemInNs> + '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<Item = ItemInNs> + '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)
 }