feat: improve support for ignored proc macros

crates/base-db/src/input.rs

@@ -243,6 +243,7 @@ impl CrateDisplayName {
         CrateDisplayName { crate_name, canonical_name }
     }
 }
+
 pub type TargetLayoutLoadResult = Result<Arc<str>, Arc<str>>;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]

crates/hir-def/src/data.rs

@@ -634,7 +634,6 @@ impl<'a> AssocItemCollector<'a> {
                     attr,
                 ) {
                     Ok(ResolvedAttr::Macro(call_id)) => {
-                        self.attr_calls.push((ast_id, call_id));
                         // If proc attribute macro expansion is disabled, skip expanding it here
                         if !self.db.expand_proc_attr_macros() {
                             continue 'attrs;
@@ -647,10 +646,21 @@ impl<'a> AssocItemCollector<'a> {
                             // disabled. This is analogous to the handling in
                             // `DefCollector::collect_macros`.
                             if exp.is_dummy() {
+                                self.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
+                                    self.module_id.local_id,
+                                    loc.kind,
+                                    loc.def.krate,
+                                ));
+
+                                continue 'attrs;
+                            }
+                            if exp.is_disabled() {
                                 continue 'attrs;
                             }
                         }
 
+                        self.attr_calls.push((ast_id, call_id));
+
                         let res =
                             self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id);
                         self.collect_macro_items(res, &|| loc.kind.clone());

crates/hir-def/src/macro_expansion_tests/mod.rs

@@ -58,6 +58,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
             name: "identity_when_valid".into(),
             kind: ProcMacroKind::Attr,
             expander: sync::Arc::new(IdentityWhenValidProcMacroExpander),
+            disabled: false,
         },
     )];
     let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros);

crates/hir-def/src/nameres/collector.rs

@@ -11,7 +11,7 @@ use either::Either;
 use hir_expand::{
     ast_id_map::FileAstId,
     attrs::{Attr, AttrId},
-    builtin_attr_macro::find_builtin_attr,
+    builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander},
     builtin_derive_macro::find_builtin_derive,
     builtin_fn_macro::find_builtin_macro,
     name::{name, AsName, Name},
@@ -98,9 +98,13 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
                         };
                         (
                             name.as_name(),
-                            CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
-                                idx as u32,
-                            )),
+                            if it.disabled {
+                                CustomProcMacroExpander::disabled()
+                            } else {
+                                CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
+                                    idx as u32,
+                                ))
+                            },
                         )
                     })
                     .collect())
@@ -604,9 +608,6 @@ impl DefCollector<'_> {
         id: ItemTreeId<item_tree::Function>,
         fn_id: FunctionId,
     ) {
-        if self.def_map.block.is_some() {
-            return;
-        }
         let kind = def.kind.to_basedb_kind();
         let (expander, kind) =
             match self.proc_macros.as_ref().map(|it| it.iter().find(|(n, _)| n == &def.name)) {
@@ -1120,9 +1121,16 @@ impl DefCollector<'_> {
         let mut push_resolved = |directive: &MacroDirective, call_id| {
             resolved.push((directive.module_id, directive.depth, directive.container, call_id));
         };
+
+        #[derive(PartialEq, Eq)]
+        enum Resolved {
+            Yes,
+            No,
+        }
+
         let mut res = ReachedFixedPoint::Yes;
         // Retain unresolved macros after this round of resolution.
-        macros.retain(|directive| {
+        let mut retain = |directive: &MacroDirective| {
             let subns = match &directive.kind {
                 MacroDirectiveKind::FnLike { .. } => MacroSubNs::Bang,
                 MacroDirectiveKind::Attr { .. } | MacroDirectiveKind::Derive { .. } => {
@@ -1156,10 +1164,11 @@ impl DefCollector<'_> {
                         self.def_map.modules[directive.module_id]
                             .scope
                             .add_macro_invoc(ast_id.ast_id, call_id);
+
                         push_resolved(directive, call_id);
 
                         res = ReachedFixedPoint::No;
-                        return false;
+                        return Resolved::Yes;
                     }
                 }
                 MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, call_site } => {
@@ -1198,7 +1207,7 @@ impl DefCollector<'_> {
 
                         push_resolved(directive, call_id);
                         res = ReachedFixedPoint::No;
-                        return false;
+                        return Resolved::Yes;
                     }
                 }
                 MacroDirectiveKind::Attr { ast_id: file_ast_id, mod_item, attr, tree } => {
@@ -1221,7 +1230,7 @@ impl DefCollector<'_> {
                         }
                         .collect(&[*mod_item], directive.container);
                         res = ReachedFixedPoint::No;
-                        false
+                        Resolved::Yes
                     };
 
                     if let Some(ident) = path.as_ident() {
@@ -1237,13 +1246,18 @@ impl DefCollector<'_> {
 
                     let def = match resolver_def_id(path.clone()) {
                         Some(def) if def.is_attribute() => def,
-                        _ => return true,
+                        _ => return Resolved::No,
                     };
-                    if matches!(
-                        def,
-                        MacroDefId { kind: MacroDefKind::BuiltInAttr(expander, _),.. }
-                        if expander.is_derive()
-                    ) {
+
+                    if let MacroDefId {
+                        kind:
+                            MacroDefKind::BuiltInAttr(
+                                BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst,
+                                _,
+                            ),
+                        ..
+                    } = def
+                    {
                         // Resolved to `#[derive]`, we don't actually expand this attribute like
                         // normal (as that would just be an identity expansion with extra output)
                         // Instead we treat derive attributes special and apply them separately.
@@ -1316,16 +1330,6 @@ impl DefCollector<'_> {
                     let call_id =
                         attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
 
-                    // If proc attribute macro expansion is disabled, skip expanding it here
-                    if !self.db.expand_proc_attr_macros() {
-                        self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
-                            directive.module_id,
-                            self.db.lookup_intern_macro_call(call_id).kind,
-                            def.krate,
-                        ));
-                        return recollect_without(self);
-                    }
-
                     // Skip #[test]/#[bench] expansion, which would merely result in more memory usage
                     // due to duplicating functions into macro expansions
                     if matches!(
@@ -1337,17 +1341,29 @@ impl DefCollector<'_> {
                     }
 
                     if let MacroDefKind::ProcMacro(exp, ..) = def.kind {
-                        if exp.is_dummy() {
-                            // If there's no expander for the proc macro (e.g.
-                            // because proc macros are disabled, or building the
-                            // proc macro crate failed), report this and skip
-                            // expansion like we would if it was disabled
+                        // If proc attribute macro expansion is disabled, skip expanding it here
+                        if !self.db.expand_proc_attr_macros() {
                             self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
                                 directive.module_id,
                                 self.db.lookup_intern_macro_call(call_id).kind,
                                 def.krate,
                             ));
+                            return recollect_without(self);
+                        }
 
+                        // If there's no expander for the proc macro (e.g.
+                        // because proc macros are disabled, or building the
+                        // proc macro crate failed), report this and skip
+                        // expansion like we would if it was disabled
+                        if exp.is_dummy() {
+                            self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
+                                directive.module_id,
+                                self.db.lookup_intern_macro_call(call_id).kind,
+                                def.krate,
+                            ));
+                            return recollect_without(self);
+                        }
+                        if exp.is_disabled() {
                             return recollect_without(self);
                         }
                     }
@@ -1358,12 +1374,13 @@ impl DefCollector<'_> {
 
                     push_resolved(directive, call_id);
                     res = ReachedFixedPoint::No;
-                    return false;
+                    return Resolved::Yes;
                 }
             }
 
-            true
-        });
+            Resolved::No
+        };
+        macros.retain(|it| retain(it) == Resolved::No);
         // Attribute resolution can add unresolved macro invocations, so concatenate the lists.
         macros.extend(mem::take(&mut self.unresolved_macros));
         self.unresolved_macros = macros;
@@ -1673,7 +1690,11 @@ impl ModCollector<'_, '_> {
                         FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db);
 
                     let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
-                    if self.def_collector.is_proc_macro && self.module_id == DefMap::ROOT {
+
+                    if self.def_collector.def_map.block.is_none()
+                        && self.def_collector.is_proc_macro
+                        && self.module_id == DefMap::ROOT
+                    {
                         if let Some(proc_macro) = attrs.parse_proc_macro_decl(&it.name) {
                             self.def_collector.export_proc_macro(
                                 proc_macro,

crates/hir-def/src/nameres/diagnostics.rs

@@ -103,6 +103,9 @@ impl DefDiagnostic {
     }
 
     // FIXME: Whats the difference between this and unresolved_macro_call
+    // FIXME: This is used for a lot of things, unresolved proc macros, disabled proc macros, etc
+    // yet the diagnostic handler in ide-diagnostics has to figure out what happened because this
+    // struct loses all that information!
     pub(crate) fn unresolved_proc_macro(
         container: LocalModuleId,
         ast: MacroCallKind,

crates/hir-expand/src/lib.rs

@@ -129,6 +129,8 @@ pub type ExpandResult<T> = ValueResult<T, ExpandError>;
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub enum ExpandError {
     UnresolvedProcMacro(CrateId),
+    /// The macro expansion is disabled.
+    MacroDisabled,
     Mbe(mbe::ExpandError),
     RecursionOverflowPoisoned,
     Other(Box<Box<str>>),
@@ -160,6 +162,7 @@ impl fmt::Display for ExpandError {
                 f.write_str(it)
             }
             ExpandError::Other(it) => f.write_str(it),
+            ExpandError::MacroDisabled => f.write_str("macro disabled"),
         }
     }
 }

crates/hir-expand/src/proc_macro.rs

@@ -49,27 +49,43 @@ pub struct ProcMacro {
     pub name: SmolStr,
     pub kind: ProcMacroKind,
     pub expander: sync::Arc<dyn ProcMacroExpander>,
+    pub disabled: bool,
 }
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
 pub struct CustomProcMacroExpander {
     proc_macro_id: ProcMacroId,
 }
 
-const DUMMY_ID: u32 = !0;
-
 impl CustomProcMacroExpander {
+    const DUMMY_ID: u32 = !0;
+    const DISABLED_ID: u32 = !1;
+
     pub fn new(proc_macro_id: ProcMacroId) -> Self {
-        assert_ne!(proc_macro_id.0, DUMMY_ID);
+        assert_ne!(proc_macro_id.0, Self::DUMMY_ID);
+        assert_ne!(proc_macro_id.0, Self::DISABLED_ID);
         Self { proc_macro_id }
     }
 
-    pub fn dummy() -> Self {
-        Self { proc_macro_id: ProcMacroId(DUMMY_ID) }
+    /// A dummy expander that always errors. This is used for proc-macros that are missing, usually
+    /// due to them not being built yet.
+    pub const fn dummy() -> Self {
+        Self { proc_macro_id: ProcMacroId(Self::DUMMY_ID) }
     }
 
-    pub fn is_dummy(&self) -> bool {
-        self.proc_macro_id.0 == DUMMY_ID
+    /// The macro was not yet resolved.
+    pub const fn is_dummy(&self) -> bool {
+        self.proc_macro_id.0 == Self::DUMMY_ID
+    }
+
+    /// A dummy expander that always errors. This expander is used for macros that have been disabled.
+    pub const fn disabled() -> Self {
+        Self { proc_macro_id: ProcMacroId(Self::DISABLED_ID) }
+    }
+
+    /// The macro is explicitly disabled and cannot be expanded.
+    pub const fn is_disabled(&self) -> bool {
+        self.proc_macro_id.0 == Self::DISABLED_ID
     }
 
     pub fn expand(
@@ -84,10 +100,14 @@ impl CustomProcMacroExpander {
         mixed_site: Span,
     ) -> ExpandResult<tt::Subtree> {
         match self.proc_macro_id {
-            ProcMacroId(DUMMY_ID) => ExpandResult::new(
+            ProcMacroId(Self::DUMMY_ID) => ExpandResult::new(
                 tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
                 ExpandError::UnresolvedProcMacro(def_crate),
             ),
+            ProcMacroId(Self::DISABLED_ID) => ExpandResult::new(
+                tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
+                ExpandError::MacroDisabled,
+            ),
             ProcMacroId(id) => {
                 let proc_macros = db.proc_macros();
                 let proc_macros = match proc_macros.get(&def_crate) {
@@ -110,7 +130,7 @@ impl CustomProcMacroExpander {
                         );
                         return ExpandResult::new(
                             tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
-                            ExpandError::other("Internal error"),
+                            ExpandError::other("Internal error: proc-macro index out of bounds"),
                         );
                     }
                 };