resolve: Centralize some error reporting for unexpected macro resolutions

2 files changed, 82 insertions, 57 deletions

diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
index c4dcdd28817..d4a83b80990 100644
--- a/compiler/rustc_expand/src/expand.rs
+++ b/compiler/rustc_expand/src/expand.rs
@@ -209,6 +209,28 @@ impl AstFragmentKind {
         self.make_from(DummyResult::any(span)).expect("couldn't create a dummy AST fragment")
     }
 
+    /// Fragment supports macro expansion and not just inert attributes, `cfg` and `cfg_attr`.
+    pub fn supports_macro_expansion(self) -> bool {
+        match self {
+            AstFragmentKind::OptExpr
+            | AstFragmentKind::Expr
+            | AstFragmentKind::Pat
+            | AstFragmentKind::Ty
+            | AstFragmentKind::Stmts
+            | AstFragmentKind::Items
+            | AstFragmentKind::TraitItems
+            | AstFragmentKind::ImplItems
+            | AstFragmentKind::ForeignItems => true,
+            AstFragmentKind::Arms
+            | AstFragmentKind::Fields
+            | AstFragmentKind::FieldPats
+            | AstFragmentKind::GenericParams
+            | AstFragmentKind::Params
+            | AstFragmentKind::StructFields
+            | AstFragmentKind::Variants => false,
+        }
+    }
+
     fn expect_from_annotatables<I: IntoIterator<Item = Annotatable>>(
         self,
         items: I,
@@ -1014,7 +1036,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         attrs: &mut Vec<ast::Attribute>,
         after_derive: &mut bool,
     ) -> Option<ast::Attribute> {
-        let attr = attrs
+        attrs
             .iter()
             .position(|a| {
                 if a.has_name(sym::derive) {
@@ -1022,22 +1044,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 }
                 !self.cx.sess.is_attr_known(a) && !is_builtin_attr(a)
             })
-            .map(|i| attrs.remove(i));
-        if let Some(attr) = &attr {
-            if !self.cx.ecfg.custom_inner_attributes()
-                && attr.style == ast::AttrStyle::Inner
-                && !attr.has_name(sym::test)
-            {
-                feature_err(
-                    &self.cx.sess.parse_sess,
-                    sym::custom_inner_attributes,
-                    attr.span,
-                    "non-builtin inner attributes are unstable",
-                )
-                .emit();
-            }
-        }
-        attr
+            .map(|i| attrs.remove(i))
     }
 
     /// If `item` is an attr invocation, remove and return the macro attribute and derive traits.
diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
index 7b46b5fda04..b1ea2d73786 100644
--- a/compiler/rustc_resolve/src/macros.rs
+++ b/compiler/rustc_resolve/src/macros.rs
@@ -12,24 +12,24 @@ use rustc_ast_pretty::pprust;
 use rustc_attr::StabilityLevel;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::ptr_key::PtrKey;
+use rustc_data_structures::sync::Lrc;
 use rustc_errors::struct_span_err;
 use rustc_expand::base::{Indeterminate, InvocationRes, ResolverExpand, SyntaxExtension};
 use rustc_expand::compile_declarative_macro;
-use rustc_expand::expand::{AstFragment, AstFragmentKind, Invocation, InvocationKind};
+use rustc_expand::expand::{AstFragment, Invocation, InvocationKind};
 use rustc_feature::is_builtin_attr_name;
 use rustc_hir::def::{self, DefKind, NonMacroAttrKind};
 use rustc_hir::def_id;
 use rustc_middle::middle::stability;
 use rustc_middle::ty;
 use rustc_session::lint::builtin::UNUSED_MACROS;
+use rustc_session::parse::feature_err;
 use rustc_session::Session;
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::{self, ExpnData, ExpnId, ExpnKind};
+use rustc_span::hygiene::{AstPass, MacroKind};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{Span, DUMMY_SP};
-
-use rustc_data_structures::sync::Lrc;
-use rustc_span::hygiene::{AstPass, MacroKind};
 use std::cell::Cell;
 use std::{mem, ptr};
 
@@ -241,15 +241,20 @@ impl<'a> ResolverExpand for Resolver<'a> {
             }
         };
 
-        let (path, kind, derives, after_derive) = match invoc.kind {
+        let (path, kind, inner_attr, derives, after_derive) = match invoc.kind {
             InvocationKind::Attr { ref attr, ref derives, after_derive, .. } => (
                 &attr.get_normal_item().path,
                 MacroKind::Attr,
+                attr.style == ast::AttrStyle::Inner,
                 self.arenas.alloc_ast_paths(derives),
                 after_derive,
             ),
-            InvocationKind::Bang { ref mac, .. } => (&mac.path, MacroKind::Bang, &[][..], false),
-            InvocationKind::Derive { ref path, .. } => (path, MacroKind::Derive, &[][..], false),
+            InvocationKind::Bang { ref mac, .. } => {
+                (&mac.path, MacroKind::Bang, false, &[][..], false)
+            }
+            InvocationKind::Derive { ref path, .. } => {
+                (path, MacroKind::Derive, false, &[][..], false)
+            }
             InvocationKind::DeriveContainer { ref derives, .. } => {
                 // Block expansion of the container until we resolve all derives in it.
                 // This is required for two reasons:
@@ -299,8 +304,17 @@ impl<'a> ResolverExpand for Resolver<'a> {
 
         // Derives are not included when `invocations` are collected, so we have to add them here.
         let parent_scope = &ParentScope { derives, ..parent_scope };
+        let require_inert = !invoc.fragment_kind.supports_macro_expansion();
         let node_id = self.lint_node_id(eager_expansion_root);
-        let (ext, res) = self.smart_resolve_macro_path(path, kind, parent_scope, node_id, force)?;
+        let (ext, res) = self.smart_resolve_macro_path(
+            path,
+            kind,
+            require_inert,
+            inner_attr,
+            parent_scope,
+            node_id,
+            force,
+        )?;
 
         let span = invoc.span();
         invoc_id.set_expn_data(ext.expn_data(
@@ -318,29 +332,6 @@ impl<'a> ResolverExpand for Resolver<'a> {
             self.definitions.add_parent_module_of_macro_def(invoc_id, normal_module_def_id);
         }
 
-        match invoc.fragment_kind {
-            AstFragmentKind::Arms
-            | AstFragmentKind::Fields
-            | AstFragmentKind::FieldPats
-            | AstFragmentKind::GenericParams
-            | AstFragmentKind::Params
-            | AstFragmentKind::StructFields
-            | AstFragmentKind::Variants => {
-                if let Res::Def(..) = res {
-                    self.session.span_err(
-                        span,
-                        &format!(
-                            "expected an inert attribute, found {} {}",
-                            res.article(),
-                            res.descr()
-                        ),
-                    );
-                    return Ok(InvocationRes::Single(self.dummy_ext(kind)));
-                }
-            }
-            _ => {}
-        }
-
         Ok(InvocationRes::Single(ext))
     }
 
@@ -403,10 +394,14 @@ impl<'a> ResolverExpand for Resolver<'a> {
 
 impl<'a> Resolver<'a> {
     /// Resolve macro path with error reporting and recovery.
+    /// Uses dummy syntax extensions for unresolved macros or macros with unexpected resolutions
+    /// for better error recovery.
     fn smart_resolve_macro_path(
         &mut self,
         path: &ast::Path,
         kind: MacroKind,
+        require_inert: bool,
+        inner_attr: bool,
         parent_scope: &ParentScope<'a>,
         node_id: NodeId,
         force: bool,
@@ -414,7 +409,6 @@ impl<'a> Resolver<'a> {
         let (ext, res) = match self.resolve_macro_path(path, Some(kind), parent_scope, true, force)
         {
             Ok((Some(ext), res)) => (ext, res),
-            // Use dummy syntax extensions for unresolved macros for better recovery.
             Ok((None, res)) => (self.dummy_ext(kind), res),
             Err(Determinacy::Determined) => (self.dummy_ext(kind), Res::Err),
             Err(Determinacy::Undetermined) => return Err(Indeterminate),
@@ -451,19 +445,43 @@ impl<'a> Resolver<'a> {
 
         self.check_stability_and_deprecation(&ext, path, node_id);
 
-        Ok(if ext.macro_kind() != kind {
-            let expected = kind.descr_expected();
+        let unexpected_res = if ext.macro_kind() != kind {
+            Some((kind.article(), kind.descr_expected()))
+        } else if require_inert && matches!(res, Res::Def(..)) {
+            Some(("a", "non-macro attribute"))
+        } else {
+            None
+        };
+        if let Some((article, expected)) = unexpected_res {
             let path_str = pprust::path_to_string(path);
             let msg = format!("expected {}, found {} `{}`", expected, res.descr(), path_str);
             self.session
                 .struct_span_err(path.span, &msg)
-                .span_label(path.span, format!("not {} {}", kind.article(), expected))
+                .span_label(path.span, format!("not {} {}", article, expected))
                 .emit();
-            // Use dummy syntax extensions for unexpected macro kinds for better recovery.
-            (self.dummy_ext(kind), Res::Err)
-        } else {
-            (ext, res)
-        })
+            return Ok((self.dummy_ext(kind), Res::Err));
+        }
+
+        // We are trying to avoid reporting this error if other related errors were reported.
+        if inner_attr
+            && !self.session.features_untracked().custom_inner_attributes
+            && path != &sym::test
+            && res != Res::Err
+        {
+            feature_err(
+                &self.session.parse_sess,
+                sym::custom_inner_attributes,
+                path.span,
+                match res {
+                    Res::Def(..) => "inner macro attributes are unstable",
+                    Res::NonMacroAttr(..) => "custom inner attributes are unstable",
+                    _ => unreachable!(),
+                },
+            )
+            .emit();
+        }
+
+        Ok((ext, res))
     }
 
     pub fn resolve_macro_path(