Auto merge of rust-lang#12024 - XFFXFF:derive_completion, r=Veykril

bors · bors · commit ebf4658ae9ab · 2022-04-18T13:40:18.000Z
derive completions take existing derives into count fixes rust-lang#12019 The immediate reason is that when we are doing derive completion, [`ctx.existing_derives`](https://github.com/rust-lang/rust-analyzer/blob/d1f6b4e2a0ab1a1343ab4a381c89b186a76fd001/crates/ide_completion/src/completions/attribute/derive.rs#L82) is empty, this is because we expand the macro when looking for the ancestors of the token to be completed. Take the following code as an example, we find the first `SyntaxNode` with kind `Attr` based on the ancestors of the token, but the parent of `Attr` is not a `Struct` as we [expect](https://github.com/rust-lang/rust-analyzer/blob/d1f6b4e2a0ab1a1343ab4a381c89b186a76fd001/crates/hir/src/semantics.rs#L518). ```rust #[derive(PartialEq, Eq, Or$0)] struct S; ``` The ancestors of the token to be completed above. ``` NAME_REF@24..26 IDENT@24..26 "Or" , PATH_SEGMENT@24..26 NAME_REF@24..26 IDENT@24..26 "Or" , PATH@24..26 PATH_SEGMENT@24..26 NAME_REF@24..26 IDENT@24..26 "Or" , META@24..26 PATH@24..26 PATH_SEGMENT@24..26 NAME_REF@24..26 IDENT@24..26 "Or" , ATTR@21..28 POUND@21..22 "#" WHITESPACE@22..23 " " L_BRACK@23..24 "[" META@24..26 PATH@24..26 PATH_SEGMENT@24..26 NAME_REF@24..26 IDENT@24..26 "Or" R_BRACK@26..27 "]" WHITESPACE@27..28 " " , TUPLE_EXPR@0..32 ATTR@0..14 POUND@0..1 "#" WHITESPACE@1..2 " " L_BRACK@2..3 "[" META@3..12 PATH@3..12 PATH_SEGMENT@3..12 NAME_REF@3..12 IDENT@3..12 "PartialEq" R_BRACK@12..13 "]" WHITESPACE@13..14 " " ATTR@14..21 POUND@14..15 "#" WHITESPACE@15..16 " " L_BRACK@16..17 "[" META@17..19 PATH@17..19 PATH_SEGMENT@17..19 NAME_REF@17..19 IDENT@17..19 "Eq" R_BRACK@19..20 "]" WHITESPACE@20..21 " " ATTR@21..28 POUND@21..22 "#" WHITESPACE@22..23 " " L_BRACK@23..24 "[" META@24..26 PATH@24..26 PATH_SEGMENT@24..26 NAME_REF@24..26 IDENT@24..26 "Or" R_BRACK@26..27 "]" WHITESPACE@27..28 " " L_PAREN@28..29 "(" WHITESPACE@29..30 " " R_PAREN@30..31 ")" WHITESPACE@31..32 " " ... ``` I make a small change to not do macro expansion when looking up the ancestors of the token. What I don't understand is that `self.sema.token_ancestors_with_macros(self.token.clone())` doesn't seem to expand the macro if the derive completion triggered without any prefix, like `#[derive(PartialEq, Eq, $0)]`. The ancestors of the token with `#[derive(PartialEq, Eq, $0)]`. ``` TOKEN_TREE@8..25 L_PAREN@8..9 "(" IDENT@9..18 "PartialEq" COMMA@18..19 "," WHITESPACE@19..20 " " IDENT@20..22 "Eq" COMMA@22..23 "," WHITESPACE@23..24 " " R_PAREN@24..25 ")" , META@2..25 PATH@2..8 PATH_SEGMENT@2..8 NAME_REF@2..8 IDENT@2..8 "derive" TOKEN_TREE@8..25 L_PAREN@8..9 "(" IDENT@9..18 "PartialEq" COMMA@18..19 "," WHITESPACE@19..20 " " IDENT@20..22 "Eq" COMMA@22..23 "," WHITESPACE@23..24 " " R_PAREN@24..25 ")" , ATTR@0..26 POUND@0..1 "#" L_BRACK@1..2 "[" META@2..25 PATH@2..8 PATH_SEGMENT@2..8 NAME_REF@2..8 IDENT@2..8 "derive" TOKEN_TREE@8..25 L_PAREN@8..9 "(" IDENT@9..18 "PartialEq" COMMA@18..19 "," WHITESPACE@19..20 " " IDENT@20..22 "Eq" COMMA@22..23 "," WHITESPACE@23..24 " " R_PAREN@24..25 ")" R_BRACK@25..26 "]" , STRUCT@0..39 ATTR@0..26 POUND@0..1 "#" L_BRACK@1..2 "[" META@2..25 PATH@2..8 PATH_SEGMENT@2..8 NAME_REF@2..8 IDENT@2..8 "derive" TOKEN_TREE@8..25 L_PAREN@8..9 "(" IDENT@9..18 "PartialEq" COMMA@18..19 "," WHITESPACE@19..20 " " IDENT@20..22 "Eq" COMMA@22..23 "," WHITESPACE@23..24 " " R_PAREN@24..25 ")" R_BRACK@25..26 "]" WHITESPACE@26..27 " " STRUCT_KW@27..33 "struct" WHITESPACE@33..34 " " NAME@34..38 IDENT@34..38 "Test" SEMICOLON@38..39 ";" ... ```
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
@@ -523,7 +523,8 @@ impl<'a> CompletionContext<'a> {
                     // successful expansions
                     (Some(actual_expansion), Some((fake_expansion, fake_mapped_token))) => {
                         let new_offset = fake_mapped_token.text_range().start();
-                        derive_ctx = Some((actual_expansion, fake_expansion, new_offset));
+                        derive_ctx =
+                            Some((actual_expansion, fake_expansion, new_offset, orig_attr));
                         break 'expansion;
                     }
                     // exactly one expansion failed, inconsistent state so stop expanding completely
@@ -718,7 +719,7 @@ impl<'a> CompletionContext<'a> {
         original_file: &SyntaxNode,
         file_with_fake_ident: SyntaxNode,
         offset: TextSize,
-        derive_ctx: Option<(SyntaxNode, SyntaxNode, TextSize)>,
+        derive_ctx: Option<(SyntaxNode, SyntaxNode, TextSize, ast::Attr)>,
     ) {
         let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
         let syntax_element = NodeOrToken::Token(fake_ident_token);
@@ -742,16 +743,14 @@ impl<'a> CompletionContext<'a> {
         (self.expected_type, self.expected_name) = self.expected_type_and_name();
 
         // Overwrite the path kind for derives
-        if let Some((original_file, file_with_fake_ident, offset)) = derive_ctx {
-            let attr = self
+        if let Some((original_file, file_with_fake_ident, offset, origin_attr)) = derive_ctx {
+            self.existing_derives = 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();
-            }
+                .resolve_derive_macro(&origin_attr)
+                .into_iter()
+                .flatten()
+                .flatten()
+                .collect();
 
             if let Some(ast::NameLike::NameRef(name_ref)) =
                 find_node_at_offset(&file_with_fake_ident, offset)
diff --git a/crates/ide_completion/src/tests/attribute.rs b/crates/ide_completion/src/tests/attribute.rs
@@ -747,6 +747,27 @@ mod derive {
         );
     }
 
+    #[test]
+    fn derive_with_existing_derives() {
+        check_derive(
+            r#"
+//- minicore: derive, copy, clone, ord, eq, default, fmt
+#[derive(PartialEq, Eq, Or$0)] struct Test;
+"#,
+            expect![[r#"
+                md core
+                de Default         macro Default
+                de Clone, Copy
+                de PartialOrd, Ord
+                de Clone           macro Clone
+                de PartialOrd
+                kw self::
+                kw super::
+                kw crate::
+            "#]],
+        );
+    }
+
     #[test]
     fn derive_flyimport() {
         check_derive(
