Auto merge of #44055 - zackmdavis:condensed_non-ADT_derive_error, r=jseyfried

only set non-ADT derive error once per attribute, not per trait

I found the expansion code very hard to follow, leaving me unsure as to whether this might somehow be done better, but this patch does give us the behavior requested in #43927 (up to exact choice of span; here, it's the entire attribute, not just the `derive` token).

(Note to GitHub robots: _resolves #43927_.)

r? @jseyfried

2 files changed, 30 insertions, 0 deletions

diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index c139cfeaebf..0e05cce35e2 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -783,6 +783,10 @@ impl<'a> ExtCtxt<'a> {
     pub fn span_err(&self, sp: Span, msg: &str) {
         self.parse_sess.span_diagnostic.span_err(sp, msg);
     }
+    pub fn mut_span_err(&self, sp: Span, msg: &str)
+                        -> DiagnosticBuilder<'a> {
+        self.parse_sess.span_diagnostic.mut_span_err(sp, msg)
+    }
     pub fn span_warn(&self, sp: Span, msg: &str) {
         self.parse_sess.span_diagnostic.span_warn(sp, msg);
     }
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index de9c085cc78..5deb4c3cc00 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -282,6 +282,32 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     let expansion = self.expand_invoc(invoc, ext);
                     self.collect_invocations(expansion, &[])
                 } else if let InvocationKind::Attr { attr: None, traits, item } = invoc.kind {
+                    let derive_allowed = match item {
+                        Annotatable::Item(ref item) => match item.node {
+                            ast::ItemKind::Struct(..) |
+                            ast::ItemKind::Enum(..) |
+                            ast::ItemKind::Union(..) => true,
+                            _ => false,
+                        },
+                        _ => false,
+                    };
+                    if !derive_allowed {
+                        let attr = item.attrs().iter()
+                            .find(|attr| attr.check_name("derive"))
+                            .expect("`derive` attribute should exist");
+                        let span = attr.span;
+                        let mut err = self.cx.mut_span_err(span,
+                                                           "`derive` may only be applied to \
+                                                            structs, enums and unions");
+                        if let ast::AttrStyle::Inner = attr.style {
+                            let trait_list = traits.iter()
+                                .map(|t| format!("{}", t)).collect::<Vec<_>>();
+                            let suggestion = format!("#[derive({})]", trait_list.join(", "));
+                            err.span_suggestion(span, "try an outer attribute", suggestion);
+                        }
+                        err.emit();
+                    }
+
                     let item = item
                         .map_attrs(|mut attrs| { attrs.retain(|a| a.path != "derive"); attrs });
                     let item_with_markers =