Don't offer qualified path completions for buitlin derives

Veykril · Veykril · commit 2abe19e46aaf · 2022-03-10T21:22:13.000+01:00
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -1811,6 +1811,10 @@ impl Macro {
     pub fn is_attr(&self, db: &dyn HirDatabase) -> bool {
         matches!(self.kind(db), MacroKind::Attr)
     }
+
+    pub fn is_derive(&self, db: &dyn HirDatabase) -> bool {
+        matches!(self.kind(db), MacroKind::Derive)
+    }
 }
 
 impl HasVisibility for Macro {
diff --git a/crates/ide_completion/src/completions/attribute/derive.rs b/crates/ide_completion/src/completions/attribute/derive.rs
@@ -1,32 +1,26 @@
 //! Completion for derives
-use hir::{HasAttrs, Macro, MacroKind};
-use ide_db::{
-    imports::{import_assets::ImportAssets, insert_use::ImportScope},
-    SymbolKind,
-};
+use hir::{HasAttrs, Macro};
+use ide_db::SymbolKind;
 use itertools::Itertools;
-use rustc_hash::FxHashSet;
-use syntax::{SmolStr, SyntaxKind};
+use syntax::SmolStr;
 
 use crate::{
-    completions::flyimport::compute_fuzzy_completion_order_key,
     context::{CompletionContext, PathCompletionCtx, PathKind},
     item::CompletionItem,
-    Completions, ImportEdit,
+    Completions,
 };
 
 pub(crate) fn complete_derive(acc: &mut Completions, ctx: &CompletionContext) {
-    let attr = match (&ctx.path_context, ctx.attr.as_ref()) {
-        (Some(PathCompletionCtx { kind: Some(PathKind::Derive), .. }), Some(attr)) => attr,
+    match ctx.path_context {
+        // FIXME: Enable qualified completions
+        Some(PathCompletionCtx { kind: Some(PathKind::Derive), qualifier: None, .. }) => (),
         _ => return,
-    };
+    }
 
     let core = ctx.famous_defs().core();
-    let existing_derives: FxHashSet<_> =
-        ctx.sema.resolve_derive_macro(attr).into_iter().flatten().flatten().collect();
 
     for (name, mac) in get_derives_in_scope(ctx) {
-        if existing_derives.contains(&mac) {
+        if ctx.existing_derives.contains(&mac) {
             continue;
         }
 
@@ -41,7 +35,7 @@ pub(crate) fn complete_derive(acc: &mut Completions, ctx: &CompletionContext) {
                     let mut components = vec![derive_completion.label];
                     components.extend(derive_completion.dependencies.iter().filter(
                         |&&dependency| {
-                            !existing_derives
+                            !ctx.existing_derives
                                 .iter()
                                 .map(|it| it.name(ctx.db))
                                 .any(|it| it.to_smol_str() == dependency)
@@ -66,8 +60,6 @@ pub(crate) fn complete_derive(acc: &mut Completions, ctx: &CompletionContext) {
         }
         item.add_to(acc);
     }
-
-    flyimport_derive(acc, ctx);
 }
 
 fn get_derives_in_scope(ctx: &CompletionContext) -> Vec<(hir::Name, Macro)> {
@@ -82,51 +74,6 @@ fn get_derives_in_scope(ctx: &CompletionContext) -> Vec<(hir::Name, Macro)> {
     result
 }
 
-fn flyimport_derive(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> {
-    if ctx.token.kind() != SyntaxKind::IDENT {
-        return None;
-    };
-    let potential_import_name = ctx.token.to_string();
-    let module = ctx.module?;
-    let parent = ctx.token.parent()?;
-    let user_input_lowercased = potential_import_name.to_lowercase();
-    let import_assets = ImportAssets::for_fuzzy_path(
-        module,
-        None,
-        potential_import_name,
-        &ctx.sema,
-        parent.clone(),
-    )?;
-    let import_scope = ImportScope::find_insert_use_container(&parent, &ctx.sema)?;
-    acc.add_all(
-        import_assets
-            .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind)
-            .into_iter()
-            .filter_map(|import| match import.original_item {
-                hir::ItemInNs::Macros(mac) => Some((import, mac)),
-                _ => None,
-            })
-            .filter(|&(_, mac)| mac.kind(ctx.db) == MacroKind::Derive)
-            .filter(|&(_, mac)| !ctx.is_item_hidden(&hir::ItemInNs::Macros(mac)))
-            .sorted_by_key(|(import, _)| {
-                compute_fuzzy_completion_order_key(&import.import_path, &user_input_lowercased)
-            })
-            .filter_map(|(import, mac)| {
-                let mut item = CompletionItem::new(
-                    SymbolKind::Derive,
-                    ctx.source_range(),
-                    mac.name(ctx.db).to_smol_str(),
-                );
-                item.add_import(ImportEdit { import, scope: import_scope.clone() });
-                if let Some(docs) = mac.docs(ctx.db) {
-                    item.documentation(docs);
-                }
-                Some(item.build())
-            }),
-    );
-    Some(())
-}
-
 struct DeriveDependencies {
     label: &'static str,
     dependencies: &'static [&'static str],
diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs
@@ -142,7 +142,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
     )?;
 
     let ns_filter = |import: &LocatedImport| {
-        let kind = match ctx.path_kind() {
+        let path_kind = match ctx.path_kind() {
             Some(kind) => kind,
             None => {
                 return match import.original_item {
@@ -151,9 +151,9 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
                 }
             }
         };
-        match (kind, import.original_item) {
+        match (path_kind, import.original_item) {
             // Aren't handled in flyimport
-            (PathKind::Vis { .. } | PathKind::Use | PathKind::Derive, _) => false,
+            (PathKind::Vis { .. } | PathKind::Use, _) => false,
             // modules are always fair game
             (_, ItemInNs::Types(hir::ModuleDef::Module(_))) => true,
             // and so are macros(except for attributes)
@@ -173,6 +173,11 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
 
             (PathKind::Attr { .. }, ItemInNs::Macros(mac)) => mac.is_attr(ctx.db),
             (PathKind::Attr { .. }, _) => false,
+
+            (PathKind::Derive, ItemInNs::Macros(mac)) => {
+                mac.is_derive(ctx.db) && !ctx.existing_derives.contains(&mac)
+            }
+            (PathKind::Derive, _) => false,
         }
     };
 
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
@@ -12,6 +12,7 @@ use ide_db::{
     famous_defs::FamousDefs,
     RootDatabase,
 };
+use rustc_hash::FxHashSet;
 use syntax::{
     algo::{find_node_at_offset, non_trivia_sibling},
     ast::{self, AttrKind, HasName, NameOrNameRef},
@@ -127,7 +128,6 @@ pub(crate) struct CompletionContext<'a> {
 
     /// The parent function of the cursor position if it exists.
     pub(super) function_def: Option<ast::Fn>,
-    pub(super) attr: Option<ast::Attr>,
     /// The parent impl of the cursor position if it exists.
     pub(super) impl_def: Option<ast::Impl>,
     /// The NameLike under the cursor in the original file if it exists.
@@ -143,6 +143,8 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) pattern_ctx: Option<PatternContext>,
     pub(super) path_context: Option<PathCompletionCtx>,
 
+    pub(super) existing_derives: FxHashSet<hir::Macro>,
+
     pub(super) locals: Vec<(Name, Local)>,
 
     no_completion_required: bool,
@@ -440,7 +442,6 @@ impl<'a> CompletionContext<'a> {
             expected_name: None,
             expected_type: None,
             function_def: None,
-            attr: None,
             impl_def: None,
             name_syntax: None,
             lifetime_ctx: None,
@@ -453,6 +454,7 @@ impl<'a> CompletionContext<'a> {
             locals,
             incomplete_let: false,
             no_completion_required: false,
+            existing_derives: Default::default(),
         };
         ctx.expand_and_fill(
             original_file.syntax().clone(),
@@ -746,11 +748,6 @@ impl<'a> CompletionContext<'a> {
             (fn_is_prev && !inside_impl_trait_block) || for_is_prev2
         };
 
-        self.attr = self
-            .sema
-            .token_ancestors_with_macros(self.token.clone())
-            .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
-            .find_map(ast::Attr::cast);
         self.fake_attribute_under_caret = syntax_element.ancestors().find_map(ast::Attr::cast);
 
         self.incomplete_let =
@@ -764,9 +761,21 @@ impl<'a> CompletionContext<'a> {
 
         // Overwrite the path kind for derives
         if let Some((original_file, file_with_fake_ident, offset)) = derive_ctx {
+            let attr = self
+                .sema
+                .token_ancestors_with_macros(self.token.clone())
+                .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
+                .find_map(ast::Attr::cast);
+            if let Some(attr) = &attr {
+                self.existing_derives =
+                    self.sema.resolve_derive_macro(attr).into_iter().flatten().flatten().collect();
+            }
+
             if let Some(ast::NameLike::NameRef(name_ref)) =
                 find_node_at_offset(&file_with_fake_ident, offset)
             {
+                self.name_syntax =
+                    find_node_at_offset(&original_file, name_ref.syntax().text_range().start());
                 if let Some((path_ctx, _)) =
                     Self::classify_name_ref(&self.sema, &original_file, name_ref)
                 {
diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs
@@ -109,6 +109,7 @@ fn detail(sema: &Semantics<RootDatabase>, macro_: hir::Macro) -> Option<String>
     let _ = sema.parse_or_expand(file_id);
     let detail = match value {
         Either::Left(node) => macro_label(&node),
+        // FIXME: this should render with the derive name, not the function name
         Either::Right(node) => fn_as_proc_macro_label(&node),
     };
     Some(detail)
diff --git a/crates/ide_completion/src/tests/attribute.rs b/crates/ide_completion/src/tests/attribute.rs
@@ -764,7 +764,7 @@ mod derive {
 #[derive(der$0)] struct Test;
 "#,
             expect![[r#"
-                de DeriveIdentity (use proc_macros::DeriveIdentity)
+                de DeriveIdentity (use proc_macros::DeriveIdentity) pub macro derive_identity
             "#]],
         );
         check_derive(
@@ -805,21 +805,15 @@ use proc_macros::DeriveIdentity;
 //- minicore: derive, copy, clone
 #[derive(proc_macros::$0)] struct Test;
 "#,
-            expect![[r#"
-                de Clone, Copy
-                de Clone
-            "#]],
+            expect![[r#""#]],
         );
         check_derive(
             r#"
 //- proc_macros: derive_identity
 //- minicore: derive, copy, clone
 #[derive(proc_macros::C$0)] struct Test;
 "#,
-            expect![[r#"
-                de Clone, Copy
-                de Clone
-            "#]],
+            expect![[r#""#]],
         );
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1811,6 +1811,10 @@ impl Macro {`
`1811`	`1811`	`pub fn is_attr(&self, db: &dyn HirDatabase) -> bool {`
`1812`	`1812`	`matches!(self.kind(db), MacroKind::Attr)`
`1813`	`1813`	`}`
	`1814`	`+`
	`1815`	`+ pub fn is_derive(&self, db: &dyn HirDatabase) -> bool {`
	`1816`	`+ matches!(self.kind(db), MacroKind::Derive)`
	`1817`	`+ }`
`1814`	`1818`	`}`
`1815`	`1819`
`1816`	`1820`	`impl HasVisibility for Macro {`