Merge #10030

10030: fix: Fix multiple derives in one attribute not expanding all in expand_macro r=Veykril a=Veykril

It's probably better to only expand the exact derive the cursor is on(if possible) instead of all derives in the attribute the cursor is one.
follow up to #10029
bors r+

Co-authored-by: Lukas Wirth <[email protected]>

Author: bors[bot]

crates/hir/src/semantics.rs

@@ -148,7 +148,7 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
         self.imp.expand_attr_macro(item)
     }
 
-    pub fn expand_derive_macro(&self, derive: &ast::Attr) -> Option<SyntaxNode> {
+    pub fn expand_derive_macro(&self, derive: &ast::Attr) -> Option<Vec<SyntaxNode>> {
         self.imp.expand_derive_macro(derive)
     }
 
@@ -389,16 +389,29 @@ impl<'db> SemanticsImpl<'db> {
         Some(node)
     }
 
-    fn expand_derive_macro(&self, attr: &ast::Attr) -> Option<SyntaxNode> {
+    fn expand_derive_macro(&self, attr: &ast::Attr) -> Option<Vec<SyntaxNode>> {
         let item = attr.syntax().parent().and_then(ast::Item::cast)?;
         let sa = self.analyze(item.syntax());
         let item = InFile::new(sa.file_id, &item);
         let src = InFile::new(sa.file_id, attr.clone());
-        let macro_call_id = self.with_ctx(|ctx| ctx.attr_to_derive_macro_call(item, src))?;
-        let file_id = macro_call_id.as_file();
-        let node = self.db.parse_or_expand(file_id)?;
-        self.cache(node.clone(), file_id);
-        Some(node)
+        self.with_ctx(|ctx| {
+            let macro_call_ids = ctx.attr_to_derive_macro_call(item, src)?;
+
+            let expansions: Vec<_> = macro_call_ids
+                .iter()
+                .map(|call| call.as_file())
+                .flat_map(|file_id| {
+                    let node = self.db.parse_or_expand(file_id)?;
+                    self.cache(node.clone(), file_id);
+                    Some(node)
+                })
+                .collect();
+            if expansions.is_empty() {
+                None
+            } else {
+                Some(expansions)
+            }
+        })
     }
 
     fn is_attr_macro_call(&self, item: &ast::Item) -> bool {

crates/hir/src/semantics/source_to_def.rs

@@ -246,9 +246,9 @@ impl SourceToDefCtx<'_, '_> {
         &mut self,
         item: InFile<&ast::Item>,
         src: InFile<ast::Attr>,
-    ) -> Option<MacroCallId> {
+    ) -> Option<&[MacroCallId]> {
         let map = self.dyn_map(item)?;
-        map[keys::DERIVE_MACRO].get(&src).copied()
+        map[keys::DERIVE_MACRO].get(&src).map(AsRef::as_ref)
     }
 
     fn to_def<Ast: AstNode + 'static, ID: Copy + 'static>(

crates/hir_def/src/child_by_source.rs

@@ -6,6 +6,7 @@
 
 use either::Either;
 use hir_expand::HirFileId;
+use itertools::Itertools;
 use syntax::ast::AttrsOwner;
 
 use crate::{
@@ -109,10 +110,13 @@ impl ChildBySource for ItemScope {
             let item = ast_id.with_value(ast_id.to_node(db.upcast()));
             res[keys::ATTR_MACRO].insert(item, call_id);
         });
-        self.derive_macro_invocs().for_each(|(ast_id, (attr_id, call_id))| {
+        self.derive_macro_invocs().for_each(|(ast_id, calls)| {
             let item = ast_id.to_node(db.upcast());
-            if let Some(attr) = item.attrs().nth(attr_id.ast_index as usize) {
-                res[keys::DERIVE_MACRO].insert(ast_id.with_value(attr), call_id);
+            let grouped = calls.iter().copied().into_group_map();
+            for (attr_id, calls) in grouped {
+                if let Some(attr) = item.attrs().nth(attr_id.ast_index as usize) {
+                    res[keys::DERIVE_MACRO].insert(ast_id.with_value(attr), calls.into());
+                }
             }
         });
 

crates/hir_def/src/item_scope.rs

@@ -195,8 +195,8 @@ impl ItemScope {
 
     pub(crate) fn derive_macro_invocs(
         &self,
-    ) -> impl Iterator<Item = (AstId<ast::Item>, (AttrId, MacroCallId))> + '_ {
-        self.derive_macros.iter().flat_map(|(k, v)| v.iter().map(move |v| (*k, *v)))
+    ) -> impl Iterator<Item = (AstId<ast::Item>, &[(AttrId, MacroCallId)])> + '_ {
+        self.derive_macros.iter().map(|(k, v)| (*k, v.as_ref()))
     }
 
     pub(crate) fn unnamed_trait_vis(&self, tr: TraitId) -> Option<Visibility> {

crates/hir_def/src/keys.rs

@@ -33,7 +33,7 @@ pub const CONST_PARAM: Key<ast::ConstParam, ConstParamId> = Key::new();
 
 pub const MACRO: Key<ast::MacroCall, MacroDefId> = Key::new();
 pub const ATTR_MACRO: Key<ast::Item, MacroCallId> = Key::new();
-pub const DERIVE_MACRO: Key<ast::Attr, MacroCallId> = Key::new();
+pub const DERIVE_MACRO: Key<ast::Attr, Box<[MacroCallId]>> = Key::new();
 
 /// XXX: AST Nodes and SyntaxNodes have identity equality semantics: nodes are
 /// equal if they point to exactly the same object.

crates/ide/src/expand_macro.rs

@@ -37,10 +37,11 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
             if path == "derive" {
                 let mut tt = tt.syntax().children_with_tokens().skip(1).join("");
                 tt.pop();
-                return sema
-                    .expand_derive_macro(&attr)
-                    .map(insert_whitespaces)
-                    .map(|expansion| ExpandedMacro { name: tt, expansion });
+                let expansions = sema.expand_derive_macro(&attr)?;
+                return Some(ExpandedMacro {
+                    name: tt,
+                    expansion: expansions.into_iter().map(insert_whitespaces).join(""),
+                });
             }
         }
     }
@@ -382,4 +383,26 @@ struct Foo {}
             "#]],
         );
     }
+
+    #[test]
+    fn macro_expand_derive_multi() {
+        check(
+            r#"
+#[rustc_builtin_macro]
+pub macro Clone {}
+#[rustc_builtin_macro]
+pub macro Copy {}
+
+#[derive(Cop$0y, Clone)]
+struct Foo {}
+"#,
+            expect![[r#"
+                Copy, Clone
+                impl< >crate::marker::Copy for Foo< >{}
+
+                impl< >crate::clone::Clone for Foo< >{}
+
+            "#]],
+        );
+    }
 }