Merge #6942

6942: Minor `#[derive]` resolution cleanup r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

6 files changed, 66 insertions, 33 deletions

diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs
index 1b9c64ee55a..18525406c92 100644
--- a/crates/hir_def/src/attr.rs
+++ b/crates/hir_def/src/attr.rs
@@ -5,7 +5,7 @@ use std::{ops, sync::Arc};
 use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
 use either::Either;
-use hir_expand::{hygiene::Hygiene, AstId, InFile};
+use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
 use itertools::Itertools;
 use mbe::ast_to_token_tree;
 use syntax::{
@@ -19,7 +19,7 @@ use crate::{
     db::DefDatabase,
     item_tree::{ItemTreeId, ItemTreeNode},
     nameres::ModuleSource,
-    path::ModPath,
+    path::{ModPath, PathKind},
     src::HasChildSource,
     AdtId, AttrDefId, Lookup,
 };
@@ -357,6 +357,46 @@ impl Attr {
         };
         Some(Attr { path, input })
     }
+
+    /// Parses this attribute as a `#[derive]`, returns an iterator that yields all contained paths
+    /// to derive macros.
+    ///
+    /// Returns `None` when the attribute is not a well-formed `#[derive]` attribute.
+    pub(crate) fn parse_derive(&self) -> Option<impl Iterator<Item = ModPath>> {
+        if self.path.as_ident() != Some(&hir_expand::name![derive]) {
+            return None;
+        }
+
+        match &self.input {
+            Some(AttrInput::TokenTree(args)) => {
+                let mut counter = 0;
+                let paths = args
+                    .token_trees
+                    .iter()
+                    .group_by(move |tt| {
+                        match tt {
+                            tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
+                                counter += 1;
+                            }
+                            _ => {}
+                        }
+                        counter
+                    })
+                    .into_iter()
+                    .map(|(_, tts)| {
+                        let segments = tts.filter_map(|tt| match tt {
+                            tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
+                            _ => None,
+                        });
+                        ModPath::from_segments(PathKind::Plain, segments)
+                    })
+                    .collect::<Vec<_>>();
+
+                Some(paths.into_iter())
+            }
+            _ => None,
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -384,7 +424,7 @@ impl<'a> AttrQuery<'a> {
         self.attrs().next().is_some()
     }
 
-    fn attrs(self) -> impl Iterator<Item = &'a Attr> {
+    pub(crate) fn attrs(self) -> impl Iterator<Item = &'a Attr> {
         let key = self.key;
         self.attrs
             .iter()
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 55228e480a5..a636ec77d71 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -1289,20 +1289,20 @@ impl ModCollector<'_, '_> {
     }
 
     fn collect_derives(&mut self, attrs: &Attrs, ast_id: FileAstId<ast::Item>) {
-        for derive_subtree in attrs.by_key("derive").tt_values() {
-            // for #[derive(Copy, Clone)], `derive_subtree` is the `(Copy, Clone)` subtree
-            for tt in &derive_subtree.token_trees {
-                let ident = match &tt {
-                    tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident,
-                    tt::TokenTree::Leaf(tt::Leaf::Punct(_)) => continue, // , is ok
-                    _ => continue, // anything else would be an error (which we currently ignore)
-                };
-                let path = ModPath::from_tt_ident(ident);
-
-                let ast_id = AstIdWithPath::new(self.file_id, ast_id, path);
-                self.def_collector
-                    .unexpanded_attribute_macros
-                    .push(DeriveDirective { module_id: self.module_id, ast_id });
+        for derive in attrs.by_key("derive").attrs() {
+            match derive.parse_derive() {
+                Some(derive_macros) => {
+                    for path in derive_macros {
+                        let ast_id = AstIdWithPath::new(self.file_id, ast_id, path);
+                        self.def_collector
+                            .unexpanded_attribute_macros
+                            .push(DeriveDirective { module_id: self.module_id, ast_id });
+                    }
+                }
+                None => {
+                    // FIXME: diagnose
+                    log::debug!("malformed derive: {:?}", derive);
+                }
             }
         }
     }
diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs
index a4d1fb8f31d..c459fa66d27 100644
--- a/crates/hir_def/src/nameres/tests.rs
+++ b/crates/hir_def/src/nameres/tests.rs
@@ -13,8 +13,8 @@ use test_utils::mark;
 
 use crate::{db::DefDatabase, nameres::*, test_db::TestDB};
 
-fn compute_crate_def_map(fixture: &str) -> Arc<CrateDefMap> {
-    let db = TestDB::with_files(fixture);
+fn compute_crate_def_map(ra_fixture: &str) -> Arc<CrateDefMap> {
+    let db = TestDB::with_files(ra_fixture);
     let krate = db.crate_graph().iter().next().unwrap();
     db.crate_def_map(krate)
 }
diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs
index 6fe2ee78a1f..f9bf5bc7253 100644
--- a/crates/hir_def/src/nameres/tests/macros.rs
+++ b/crates/hir_def/src/nameres/tests/macros.rs
@@ -632,11 +632,11 @@ pub struct bar;
 #[test]
 fn expand_derive() {
     let map = compute_crate_def_map(
-        "
+        r#"
         //- /main.rs crate:main deps:core
-        use core::*;
+        use core::Copy;
 
-        #[derive(Copy, Clone)]
+        #[derive(Copy, core::Clone)]
         struct Foo;
 
         //- /core.rs crate:core
@@ -645,7 +645,7 @@ fn expand_derive() {
 
         #[rustc_builtin_macro]
         pub macro Clone {}
-        ",
+        "#,
     );
     assert_eq!(map.modules[map.root].scope.impls().len(), 2);
 }
diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs
index 00a69a8a63d..e2bf85bbc3a 100644
--- a/crates/hir_def/src/path.rs
+++ b/crates/hir_def/src/path.rs
@@ -9,11 +9,8 @@ use std::{
 
 use crate::{body::LowerCtx, type_ref::LifetimeRef};
 use base_db::CrateId;
-use hir_expand::{
-    hygiene::Hygiene,
-    name::{AsName, Name},
-};
-use syntax::ast::{self};
+use hir_expand::{hygiene::Hygiene, name::Name};
+use syntax::ast;
 
 use crate::{
     type_ref::{TypeBound, TypeRef},
@@ -56,11 +53,6 @@ impl ModPath {
         ModPath { kind, segments }
     }
 
-    /// Converts an `tt::Ident` into a single-identifier `Path`.
-    pub(crate) fn from_tt_ident(ident: &tt::Ident) -> ModPath {
-        ident.as_name().into()
-    }
-
     /// Calls `cb` with all paths, represented by this use item.
     pub(crate) fn expand_use_item(
         item_src: InFile<ast::Use>,
diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs
index 77eeee3fe9f..2f44876a8a3 100644
--- a/crates/hir_expand/src/name.rs
+++ b/crates/hir_expand/src/name.rs
@@ -152,6 +152,7 @@ pub mod known {
         str,
         // Special names
         macro_rules,
+        derive,
         doc,
         cfg_attr,
         // Components of known path (value or mod name)