Merge #8088

8088: Store an `AstId` for procedural macros r=jonas-schievink a=jonas-schievink

Point `HasSource` to the `ast::Fn`, and go to it in `TryToNav`.

bors r+

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

11 files changed, 47 insertions, 26 deletions

diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs
index 5b22ab58b..d57fad9ed 100644
--- a/crates/hir/src/has_source.rs
+++ b/crates/hir/src/has_source.rs
@@ -6,7 +6,7 @@ use hir_def::{
     src::{HasChildSource, HasSource as _},
     Lookup, VariantId,
 };
-use hir_expand::InFile;
+use hir_expand::{InFile, MacroDefKind};
 use syntax::ast;
 
 use crate::{
@@ -111,10 +111,17 @@ impl HasSource for TypeAlias {
     }
 }
 impl HasSource for MacroDef {
-    type Ast = ast::Macro;
+    type Ast = Either<ast::Macro, ast::Fn>;
     fn source(self, db: &dyn HirDatabase) -> Option<InFile<Self::Ast>> {
-        let ast_id = self.id.ast_id()?;
-        Some(InFile { file_id: ast_id.file_id, value: ast_id.to_node(db.upcast()) })
+        Some(match &self.id.kind {
+            MacroDefKind::Declarative(id)
+            | MacroDefKind::BuiltIn(_, id)
+            | MacroDefKind::BuiltInDerive(_, id)
+            | MacroDefKind::BuiltInEager(_, id) => {
+                id.with_value(Either::Left(id.to_node(db.upcast())))
+            }
+            MacroDefKind::ProcMacro(_, id) => id.map(|_| Either::Right(id.to_node(db.upcast()))),
+        })
     }
 }
 impl HasSource for Impl {
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index b860cbf3c..95cfde61c 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -1144,12 +1144,15 @@ impl MacroDef {
 
     /// XXX: this parses the file
     pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
-        self.source(db)?.value.name().map(|it| it.as_name())
+        match self.source(db)?.value {
+            Either::Left(it) => it.name().map(|it| it.as_name()),
+            Either::Right(it) => it.name().map(|it| it.as_name()),
+        }
     }
 
     /// Indicate it is a proc-macro
     pub fn is_proc_macro(&self) -> bool {
-        matches!(self.id.kind, MacroDefKind::ProcMacro(_))
+        matches!(self.id.kind, MacroDefKind::ProcMacro(..))
     }
 
     /// Indicate it is a derive macro
diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs
index aafd73b60..f3ebe7c72 100644
--- a/crates/hir_def/src/item_scope.rs
+++ b/crates/hir_def/src/item_scope.rs
@@ -252,7 +252,7 @@ impl ItemScope {
             .for_each(|vis| *vis = Visibility::Module(this_module));
 
         for (mac, vis) in self.macros.values_mut() {
-            if let MacroDefKind::ProcMacro(_) = mac.kind {
+            if let MacroDefKind::ProcMacro(..) = mac.kind {
                 // FIXME: Technically this is insufficient since reexports of proc macros are also
                 // forbidden. Practically nobody does that.
                 continue;
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 45a79e896..696370ada 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -353,17 +353,17 @@ impl DefCollector<'_> {
     /// use a dummy expander that always errors. This comes with the drawback of macros potentially
     /// going out of sync with what the build system sees (since we resolve using VFS state, but
     /// Cargo builds only on-disk files). We could and probably should add diagnostics for that.
-    fn resolve_proc_macro(&mut self, name: &Name) {
+    fn resolve_proc_macro(&mut self, name: &Name, ast_id: AstId<ast::Fn>) {
         self.exports_proc_macros = true;
         let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) {
             Some((_, expander)) => MacroDefId {
                 krate: self.def_map.krate,
-                kind: MacroDefKind::ProcMacro(*expander),
+                kind: MacroDefKind::ProcMacro(*expander, ast_id),
                 local_inner: false,
             },
             None => MacroDefId {
                 krate: self.def_map.krate,
-                kind: MacroDefKind::ProcMacro(ProcMacroExpander::dummy(self.def_map.krate)),
+                kind: MacroDefKind::ProcMacro(ProcMacroExpander::dummy(self.def_map.krate), ast_id),
                 local_inner: false,
             },
         };
@@ -1116,7 +1116,8 @@ impl ModCollector<'_, '_> {
                 ModItem::Function(id) => {
                     let func = &self.item_tree[id];
 
-                    self.collect_proc_macro_def(&func.name, &attrs);
+                    let ast_id = InFile::new(self.file_id, func.ast_id);
+                    self.collect_proc_macro_def(&func.name, ast_id, &attrs);
 
                     def = Some(DefData {
                         id: FunctionLoc {
@@ -1383,7 +1384,7 @@ impl ModCollector<'_, '_> {
     }
 
     /// If `attrs` registers a procedural macro, collects its definition.
-    fn collect_proc_macro_def(&mut self, func_name: &Name, attrs: &Attrs) {
+    fn collect_proc_macro_def(&mut self, func_name: &Name, ast_id: AstId<ast::Fn>, attrs: &Attrs) {
         // FIXME: this should only be done in the root module of `proc-macro` crates, not everywhere
         // FIXME: distinguish the type of macro
         let macro_name = if attrs.by_key("proc_macro").exists()
@@ -1404,7 +1405,7 @@ impl ModCollector<'_, '_> {
             }
         };
 
-        self.def_collector.resolve_proc_macro(&macro_name);
+        self.def_collector.resolve_proc_macro(&macro_name, ast_id);
     }
 
     fn collect_macro_rules(&mut self, id: FileItemTreeId<MacroRules>) {
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 4107d7781..2748e25cf 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -157,7 +157,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option<Arc<(TokenExpander,
             Some(Arc::new((TokenExpander::BuiltinDerive(expander), mbe::TokenMap::default())))
         }
         MacroDefKind::BuiltInEager(..) => None,
-        MacroDefKind::ProcMacro(expander) => {
+        MacroDefKind::ProcMacro(expander, ..) => {
             Some(Arc::new((TokenExpander::ProcMacro(expander), mbe::TokenMap::default())))
         }
     }
@@ -269,7 +269,7 @@ fn expand_proc_macro(
     };
 
     let expander = match loc.def.kind {
-        MacroDefKind::ProcMacro(expander) => expander,
+        MacroDefKind::ProcMacro(expander, ..) => expander,
         _ => unreachable!(),
     };
 
diff --git a/crates/hir_expand/src/eager.rs b/crates/hir_expand/src/eager.rs
index ddadaffd3..04f374a29 100644
--- a/crates/hir_expand/src/eager.rs
+++ b/crates/hir_expand/src/eager.rs
@@ -209,7 +209,7 @@ fn eager_macro_recur(
             MacroDefKind::Declarative(_)
             | MacroDefKind::BuiltIn(..)
             | MacroDefKind::BuiltInDerive(..)
-            | MacroDefKind::ProcMacro(_) => {
+            | MacroDefKind::ProcMacro(..) => {
                 let res = lazy_expand(db, &def, curr.with_value(child.clone()), krate);
                 let val = diagnostic_sink.expand_result_option(res)?;
 
diff --git a/crates/hir_expand/src/hygiene.rs b/crates/hir_expand/src/hygiene.rs
index e758b3c0a..20cda1683 100644
--- a/crates/hir_expand/src/hygiene.rs
+++ b/crates/hir_expand/src/hygiene.rs
@@ -182,7 +182,7 @@ impl HygieneFrame {
                         MacroDefKind::BuiltIn(..) => (info, Some(loc.def.krate), false),
                         MacroDefKind::BuiltInDerive(..) => (info, None, false),
                         MacroDefKind::BuiltInEager(..) => (info, None, false),
-                        MacroDefKind::ProcMacro(_) => (info, None, false),
+                        MacroDefKind::ProcMacro(..) => (info, None, false),
                     }
                 }
             },
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index 83e11019f..0a379651f 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -245,7 +245,7 @@ impl MacroDefId {
             MacroDefKind::BuiltIn(_, id) => id,
             MacroDefKind::BuiltInDerive(_, id) => id,
             MacroDefKind::BuiltInEager(_, id) => id,
-            MacroDefKind::ProcMacro(_) => return None,
+            MacroDefKind::ProcMacro(..) => return None,
         };
         Some(*id)
     }
@@ -258,7 +258,7 @@ pub enum MacroDefKind {
     // FIXME: maybe just Builtin and rename BuiltinFnLikeExpander to BuiltinExpander
     BuiltInDerive(BuiltinDeriveExpander, AstId<ast::Macro>),
     BuiltInEager(EagerExpander, AstId<ast::Macro>),
-    ProcMacro(ProcMacroExpander),
+    ProcMacro(ProcMacroExpander, AstId<ast::Fn>),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs
index c086de163..364be260c 100644
--- a/crates/ide/src/display/navigation_target.rs
+++ b/crates/ide/src/display/navigation_target.rs
@@ -339,10 +339,14 @@ impl TryToNav for hir::Field {
 impl TryToNav for hir::MacroDef {
     fn try_to_nav(&self, db: &RootDatabase) -> Option<NavigationTarget> {
         let src = self.source(db)?;
-        log::debug!("nav target {:#?}", src.value.syntax());
+        let name_owner: &dyn ast::NameOwner = match &src.value {
+            Either::Left(it) => it,
+            Either::Right(it) => it,
+        };
+        log::debug!("nav target {:#?}", name_owner.syntax());
         let mut res = NavigationTarget::from_named(
             db,
-            src.as_ref().map(|it| it as &dyn ast::NameOwner),
+            src.as_ref().with_value(name_owner),
             SymbolKind::Macro,
         );
         res.docs = self.docs(db);
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 15d309d7d..a3fb17c0a 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -331,10 +331,16 @@ fn hover_for_definition(
 ) -> Option<Markup> {
     let mod_path = definition_mod_path(db, &def);
     return match def {
-        Definition::Macro(it) => {
-            let label = macro_label(&it.source(db)?.value);
-            from_def_source_labeled(db, it, Some(label), mod_path)
-        }
+        Definition::Macro(it) => match &it.source(db)?.value {
+            Either::Left(mac) => {
+                let label = macro_label(&mac);
+                from_def_source_labeled(db, it, Some(label), mod_path)
+            }
+            Either::Right(_) => {
+                // FIXME
+                None
+            }
+        },
         Definition::Field(def) => from_hir_fmt(db, def, mod_path),
         Definition::ModuleDef(it) => match it {
             ModuleDef::Module(it) => from_hir_fmt(db, it, mod_path),
diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs
index 3fa21ba7c..7578ad50b 100644
--- a/crates/ide_completion/src/render/macro_.rs
+++ b/crates/ide_completion/src/render/macro_.rs
@@ -91,7 +91,7 @@ impl<'a> MacroRender<'a> {
     }
 
     fn detail(&self) -> Option<String> {
-        let ast_node = self.macro_.source(self.ctx.db())?.value;
+        let ast_node = self.macro_.source(self.ctx.db())?.value.left()?;
         Some(macro_label(&ast_node))
     }
 }