Merge #11938

11938: feat: improve assoc. item completion in trait impls r=jonas-schievink a=jonas-schievink

Account for macro-generated items, increase the score of these completions since they're very relevant, and allow them to trigger when the cursor is directly in the assoc. item list without requiring further input.

![screenshot-2022-04-08-18:12:06](https://user-images.githubusercontent.com/1786438/162481277-2a0d2f21-dc20-4452-804d-6370766216b6.png)

Part of #11860

bors r+

Co-authored-by: Jonas Schievink <[email protected]>

Author: bors[bot]

crates/ide_completion/src/completions/trait_impl.rs

@@ -36,11 +36,13 @@ use ide_db::{path_transform::PathTransform, traits::get_missing_assoc_items, Sym
 use syntax::{
     ast::{self, edit_in_place::AttrsOwnerEdit},
     display::function_declaration,
-    AstNode, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, T,
+    AstNode, SyntaxElement, SyntaxKind, SyntaxNode, TextRange, T,
 };
 use text_edit::TextEdit;
 
-use crate::{CompletionContext, CompletionItem, CompletionItemKind, Completions};
+use crate::{
+    CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance, Completions,
+};
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 enum ImplCompletionKind {
@@ -51,30 +53,40 @@ enum ImplCompletionKind {
 }
 
 pub(crate) fn complete_trait_impl(acc: &mut Completions, ctx: &CompletionContext) {
-    if let Some((kind, trigger, impl_def)) = completion_match(ctx.token.clone()) {
+    if let Some((kind, replacement_range, impl_def)) = completion_match(ctx) {
         if let Some(hir_impl) = ctx.sema.to_def(&impl_def) {
             get_missing_assoc_items(&ctx.sema, &impl_def).into_iter().for_each(|item| {
                 match (item, kind) {
                     (
                         hir::AssocItem::Function(fn_item),
                         ImplCompletionKind::All | ImplCompletionKind::Fn,
-                    ) => add_function_impl(acc, ctx, &trigger, fn_item, hir_impl),
+                    ) => add_function_impl(acc, ctx, replacement_range, fn_item, hir_impl),
                     (
                         hir::AssocItem::TypeAlias(type_item),
                         ImplCompletionKind::All | ImplCompletionKind::TypeAlias,
-                    ) => add_type_alias_impl(acc, ctx, &trigger, type_item),
+                    ) => add_type_alias_impl(acc, ctx, replacement_range, type_item),
                     (
                         hir::AssocItem::Const(const_item),
                         ImplCompletionKind::All | ImplCompletionKind::Const,
-                    ) => add_const_impl(acc, ctx, &trigger, const_item, hir_impl),
+                    ) => add_const_impl(acc, ctx, replacement_range, const_item, hir_impl),
                     _ => {}
                 }
             });
         }
     }
 }
 
-fn completion_match(mut token: SyntaxToken) -> Option<(ImplCompletionKind, SyntaxNode, ast::Impl)> {
+fn completion_match(ctx: &CompletionContext) -> Option<(ImplCompletionKind, TextRange, ast::Impl)> {
+    let mut token = ctx.token.clone();
+
+    let parent_kind = token.parent().map_or(SyntaxKind::EOF, |it| it.kind());
+    if parent_kind == SyntaxKind::ASSOC_ITEM_LIST {
+        let impl_def = ast::Impl::cast(token.parent()?.parent()?)?;
+        let kind = ImplCompletionKind::All;
+        let replacement_range = TextRange::empty(ctx.position.offset);
+        return Some((kind, replacement_range, impl_def));
+    }
+
     // For keyword without name like `impl .. { fn $0 }`, the current position is inside
     // the whitespace token, which is outside `FN` syntax node.
     // We need to follow the previous token in this case.
@@ -122,13 +134,16 @@ fn completion_match(mut token: SyntaxToken) -> Option<(ImplCompletionKind, Synta
         SyntaxKind::MACRO_CALL => ImplCompletionKind::All,
         _ => return None,
     };
-    Some((kind, impl_item, impl_def))
+
+    let replacement_range = replacement_range(ctx, &impl_item);
+
+    Some((kind, replacement_range, impl_def))
 }
 
 fn add_function_impl(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    fn_def_node: &SyntaxNode,
+    replacement_range: TextRange,
     func: hir::Function,
     impl_def: hir::Impl,
 ) {
@@ -146,9 +161,10 @@ fn add_function_impl(
         CompletionItemKind::SymbolKind(SymbolKind::Function)
     };
 
-    let range = replacement_range(ctx, fn_def_node);
-    let mut item = CompletionItem::new(completion_kind, range, label);
-    item.lookup_by(fn_name).set_documentation(func.docs(ctx.db));
+    let mut item = CompletionItem::new(completion_kind, replacement_range, label);
+    item.lookup_by(fn_name)
+        .set_documentation(func.docs(ctx.db))
+        .set_relevance(CompletionRelevance { is_item_from_trait: true, ..Default::default() });
 
     if let Some(source) = ctx.sema.source(func) {
         let assoc_item = ast::AssocItem::Fn(source.value);
@@ -162,11 +178,11 @@ fn add_function_impl(
             match ctx.config.snippet_cap {
                 Some(cap) => {
                     let snippet = format!("{} {{\n    $0\n}}", function_decl);
-                    item.snippet_edit(cap, TextEdit::replace(range, snippet));
+                    item.snippet_edit(cap, TextEdit::replace(replacement_range, snippet));
                 }
                 None => {
                     let header = format!("{} {{", function_decl);
-                    item.text_edit(TextEdit::replace(range, header));
+                    item.text_edit(TextEdit::replace(replacement_range, header));
                 }
             };
             item.add_to(acc);
@@ -201,25 +217,25 @@ fn get_transformed_assoc_item(
 fn add_type_alias_impl(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    type_def_node: &SyntaxNode,
+    replacement_range: TextRange,
     type_alias: hir::TypeAlias,
 ) {
     let alias_name = type_alias.name(ctx.db).to_smol_str();
 
     let snippet = format!("type {} = ", alias_name);
 
-    let range = replacement_range(ctx, type_def_node);
-    let mut item = CompletionItem::new(SymbolKind::TypeAlias, range, &snippet);
-    item.text_edit(TextEdit::replace(range, snippet))
+    let mut item = CompletionItem::new(SymbolKind::TypeAlias, replacement_range, &snippet);
+    item.text_edit(TextEdit::replace(replacement_range, snippet))
         .lookup_by(alias_name)
-        .set_documentation(type_alias.docs(ctx.db));
+        .set_documentation(type_alias.docs(ctx.db))
+        .set_relevance(CompletionRelevance { is_item_from_trait: true, ..Default::default() });
     item.add_to(acc);
 }
 
 fn add_const_impl(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    const_def_node: &SyntaxNode,
+    replacement_range: TextRange,
     const_: hir::Const,
     impl_def: hir::Impl,
 ) {
@@ -236,11 +252,14 @@ fn add_const_impl(
 
                 let snippet = make_const_compl_syntax(&transformed_const);
 
-                let range = replacement_range(ctx, const_def_node);
-                let mut item = CompletionItem::new(SymbolKind::Const, range, &snippet);
-                item.text_edit(TextEdit::replace(range, snippet))
+                let mut item = CompletionItem::new(SymbolKind::Const, replacement_range, &snippet);
+                item.text_edit(TextEdit::replace(replacement_range, snippet))
                     .lookup_by(const_name)
-                    .set_documentation(const_.docs(ctx.db));
+                    .set_documentation(const_.docs(ctx.db))
+                    .set_relevance(CompletionRelevance {
+                        is_item_from_trait: true,
+                        ..Default::default()
+                    });
                 item.add_to(acc);
             }
         }
@@ -987,4 +1006,24 @@ where Self: SomeTrait<u32> {
 "#,
         )
     }
+
+    #[test]
+    fn works_directly_in_impl() {
+        check(
+            r#"
+trait Tr {
+    fn provided() {}
+    fn required();
+}
+
+impl Tr for () {
+    fn provided() {}
+    $0
+}
+"#,
+            expect![[r#"
+            fn fn required()
+        "#]],
+        );
+    }
 }

crates/ide_completion/src/item.rs

@@ -132,6 +132,8 @@ pub struct CompletionRelevance {
     /// }
     /// ```
     pub is_local: bool,
+    /// This is set when trait items are completed in an impl of that trait.
+    pub is_item_from_trait: bool,
     /// Set for method completions of the `core::ops` and `core::cmp` family.
     pub is_op_method: bool,
     /// Set for item completions that are private but in the workspace.
@@ -197,6 +199,7 @@ impl CompletionRelevance {
             exact_name_match,
             type_match,
             is_local,
+            is_item_from_trait,
             is_op_method,
             is_private_editable,
             postfix_match,
@@ -228,6 +231,9 @@ impl CompletionRelevance {
         if is_local {
             score += 1;
         }
+        if is_item_from_trait {
+            score += 1;
+        }
         if is_definite {
             score += 10;
         }

crates/ide_completion/src/render.rs

@@ -624,6 +624,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            is_item_from_trait: false,
                             is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
@@ -646,6 +647,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            is_item_from_trait: false,
                             is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
@@ -734,6 +736,7 @@ fn foo() { A { the$0 } }
                                 CouldUnify,
                             ),
                             is_local: false,
+                            is_item_from_trait: false,
                             is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,

crates/ide_db/src/traits.rs

@@ -3,10 +3,7 @@
 use crate::RootDatabase;
 use hir::Semantics;
 use rustc_hash::FxHashSet;
-use syntax::{
-    ast::{self, HasName},
-    AstNode,
-};
+use syntax::{ast, AstNode};
 
 /// Given the `impl` block, attempts to find the trait this `impl` corresponds to.
 pub fn resolve_target_trait(
@@ -28,32 +25,28 @@ pub fn get_missing_assoc_items(
     sema: &Semantics<RootDatabase>,
     impl_def: &ast::Impl,
 ) -> Vec<hir::AssocItem> {
+    let imp = match sema.to_def(impl_def) {
+        Some(it) => it,
+        None => return vec![],
+    };
+
     // Names must be unique between constants and functions. However, type aliases
     // may share the same name as a function or constant.
     let mut impl_fns_consts = FxHashSet::default();
     let mut impl_type = FxHashSet::default();
 
-    if let Some(item_list) = impl_def.assoc_item_list() {
-        for item in item_list.assoc_items() {
-            match item {
-                ast::AssocItem::Fn(f) => {
-                    if let Some(n) = f.name() {
-                        impl_fns_consts.insert(n.syntax().to_string());
-                    }
-                }
-
-                ast::AssocItem::TypeAlias(t) => {
-                    if let Some(n) = t.name() {
-                        impl_type.insert(n.syntax().to_string());
-                    }
-                }
-
-                ast::AssocItem::Const(c) => {
-                    if let Some(n) = c.name() {
-                        impl_fns_consts.insert(n.syntax().to_string());
-                    }
+    for item in imp.items(sema.db) {
+        match item {
+            hir::AssocItem::Function(it) => {
+                impl_fns_consts.insert(it.name(sema.db).to_string());
+            }
+            hir::AssocItem::Const(it) => {
+                if let Some(name) = it.name(sema.db) {
+                    impl_fns_consts.insert(name.to_string());
                 }
-                ast::AssocItem::MacroCall(_) => (),
+            }
+            hir::AssocItem::TypeAlias(it) => {
+                impl_type.insert(it.name(sema.db).to_string());
             }
         }
     }
@@ -219,5 +212,22 @@ impl Foo {
 }"#,
             expect![[r#""#]],
         );
+
+        check_missing_assoc(
+            r#"
+trait Tr {
+    fn required();
+}
+macro_rules! m {
+    () => { fn required() {} };
+}
+impl Tr for () {
+    m!();
+    $0
+}
+
+            "#,
+            expect![[r#""#]],
+        );
     }
 }