Auto merge of #14610 - lowr:fix/hygiene-for-meta-item, r=Veykril

fix: Resolve `$crate` in derive paths

Paths in derive meta item list may contain any kind of paths, including those that start with `$crate` generated by macros. We need to take hygiene into account when we lower paths in the list.

This issue was identified while investigating #14607, though this patch doesn't fix the broken trait resolution.

3 files changed, 76 insertions, 17 deletions

diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 756a8f50490..ba93b2c70ef 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -14,6 +14,7 @@ use hir_expand::{
     builtin_attr_macro::find_builtin_attr,
     builtin_derive_macro::find_builtin_derive,
     builtin_fn_macro::find_builtin_macro,
+    hygiene::Hygiene,
     name::{name, AsName, Name},
     proc_macro::ProcMacroExpander,
     ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc,
@@ -122,6 +123,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T
         from_glob_import: Default::default(),
         skip_attrs: Default::default(),
         is_proc_macro,
+        hygienes: FxHashMap::default(),
     };
     if tree_id.is_block() {
         collector.seed_with_inner(tree_id);
@@ -269,6 +271,12 @@ struct DefCollector<'a> {
     /// This also stores the attributes to skip when we resolve derive helpers and non-macro
     /// non-builtin attributes in general.
     skip_attrs: FxHashMap<InFile<ModItem>, AttrId>,
+    /// `Hygiene` cache, because `Hygiene` construction is expensive.
+    ///
+    /// Almost all paths should have been lowered to `ModPath` during `ItemTree` construction.
+    /// However, `DefCollector` still needs to lower paths in attributes, in particular those in
+    /// derive meta item list.
+    hygienes: FxHashMap<HirFileId, Hygiene>,
 }
 
 impl DefCollector<'_> {
@@ -312,13 +320,15 @@ impl DefCollector<'_> {
                 }
 
                 if *attr_name == hir_expand::name![feature] {
-                    let features =
-                        attr.parse_path_comma_token_tree().into_iter().flatten().filter_map(
-                            |feat| match feat.segments() {
-                                [name] => Some(name.to_smol_str()),
-                                _ => None,
-                            },
-                        );
+                    let hygiene = &Hygiene::new_unhygienic();
+                    let features = attr
+                        .parse_path_comma_token_tree(self.db.upcast(), hygiene)
+                        .into_iter()
+                        .flatten()
+                        .filter_map(|feat| match feat.segments() {
+                            [name] => Some(name.to_smol_str()),
+                            _ => None,
+                        });
                     self.def_map.unstable_features.extend(features);
                 }
 
@@ -1224,7 +1234,19 @@ impl DefCollector<'_> {
                         };
                         let ast_id = ast_id.with_value(ast_adt_id);
 
-                        match attr.parse_path_comma_token_tree() {
+                        let extend_unhygenic;
+                        let hygiene = if file_id.is_macro() {
+                            self.hygienes
+                                .entry(file_id)
+                                .or_insert_with(|| Hygiene::new(self.db.upcast(), file_id))
+                        } else {
+                            // Avoid heap allocation (`Hygiene` embraces `Arc`) and hash map entry
+                            // when we're in an oridinary (non-macro) file.
+                            extend_unhygenic = Hygiene::new_unhygienic();
+                            &extend_unhygenic
+                        };
+
+                        match attr.parse_path_comma_token_tree(self.db.upcast(), hygiene) {
                             Some(derive_macros) => {
                                 let mut len = 0;
                                 for (idx, path) in derive_macros.enumerate() {
@@ -2212,6 +2234,7 @@ mod tests {
             from_glob_import: Default::default(),
             skip_attrs: Default::default(),
             is_proc_macro: false,
+            hygienes: FxHashMap::default(),
         };
         collector.seed_with_top_level();
         collector.collect();
diff --git a/crates/hir-def/src/nameres/tests/macros.rs b/crates/hir-def/src/nameres/tests/macros.rs
index a4ccd14cbb4..6ee56c9368e 100644
--- a/crates/hir-def/src/nameres/tests/macros.rs
+++ b/crates/hir-def/src/nameres/tests/macros.rs
@@ -665,6 +665,29 @@ pub struct bar;
 }
 
 #[test]
+fn macro_dollar_crate_is_correct_in_derive_meta() {
+    let map = compute_crate_def_map(
+        r#"
+//- minicore: derive, clone
+//- /main.rs crate:main deps:lib
+lib::foo!();
+
+//- /lib.rs crate:lib
+#[macro_export]
+macro_rules! foo {
+    () => {
+        #[derive($crate::Clone)]
+        struct S;
+    }
+}
+
+pub use core::clone::Clone;
+"#,
+    );
+    assert_eq!(map.modules[map.root].scope.impls().len(), 1);
+}
+
+#[test]
 fn expand_derive() {
     let map = compute_crate_def_map(
         r#"
diff --git a/crates/hir-expand/src/attrs.rs b/crates/hir-expand/src/attrs.rs
index 7a61ca4f4d3..17360090db1 100644
--- a/crates/hir-expand/src/attrs.rs
+++ b/crates/hir-expand/src/attrs.rs
@@ -12,8 +12,7 @@ use syntax::{ast, match_ast, AstNode, SmolStr, SyntaxNode};
 use crate::{
     db::ExpandDatabase,
     hygiene::Hygiene,
-    mod_path::{ModPath, PathKind},
-    name::AsName,
+    mod_path::ModPath,
     tt::{self, Subtree},
     InFile,
 };
@@ -267,7 +266,11 @@ impl Attr {
     }
 
     /// Parses this attribute as a token tree consisting of comma separated paths.
-    pub fn parse_path_comma_token_tree(&self) -> Option<impl Iterator<Item = ModPath> + '_> {
+    pub fn parse_path_comma_token_tree<'a>(
+        &'a self,
+        db: &'a dyn ExpandDatabase,
+        hygiene: &'a Hygiene,
+    ) -> Option<impl Iterator<Item = ModPath> + 'a> {
         let args = self.token_tree_value()?;
 
         if args.delimiter.kind != DelimiterKind::Parenthesis {
@@ -276,15 +279,25 @@ impl Attr {
         let paths = args
             .token_trees
             .split(|tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. }))))
-            .filter_map(|tts| {
+            .filter_map(move |tts| {
                 if tts.is_empty() {
                     return None;
                 }
-                let segments = tts.iter().filter_map(|tt| match tt {
-                    tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
-                    _ => None,
-                });
-                Some(ModPath::from_segments(PathKind::Plain, segments))
+                // FIXME: This is necessarily a hack. It'd be nice if we could avoid allocation here.
+                let subtree = tt::Subtree {
+                    delimiter: tt::Delimiter::unspecified(),
+                    token_trees: tts.into_iter().cloned().collect(),
+                };
+                let (parse, _) =
+                    mbe::token_tree_to_syntax_node(&subtree, mbe::TopEntryPoint::MetaItem);
+                let meta = ast::Meta::cast(parse.syntax_node())?;
+                // Only simple paths are allowed.
+                if meta.eq_token().is_some() || meta.expr().is_some() || meta.token_tree().is_some()
+                {
+                    return None;
+                }
+                let path = meta.path()?;
+                ModPath::from_src(db, path, hygiene)
             });
 
         Some(paths)