Clean up AttributeLintKind and refactor diagnostic attribute linting

compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs

@@ -40,6 +40,45 @@ pub(crate) enum Mode {
     DiagnosticOnUnknown,
 }
 
+impl Mode {
+    fn as_str(&self) -> &'static str {
+        match self {
+            Self::RustcOnUnimplemented => "rustc_on_unimplemented",
+            Self::DiagnosticOnUnimplemented => "diagnostic::on_unimplemented",
+            Self::DiagnosticOnConst => "diagnostic::on_const",
+            Self::DiagnosticOnMove => "diagnostic::on_move",
+            Self::DiagnosticOnUnknown => "diagnostic::on_unknown",
+        }
+    }
+
+    fn expected_options(&self) -> &'static str {
+        const DEFAULT: &str =
+            "at least one of the `message`, `note` and `label` options are expected";
+        match self {
+            Self::RustcOnUnimplemented => {
+                "see <https://rustc-dev-guide.rust-lang.org/diagnostics.html#rustc_on_unimplemented>"
+            }
+            Self::DiagnosticOnUnimplemented => DEFAULT,
+            Self::DiagnosticOnConst => DEFAULT,
+            Self::DiagnosticOnMove => DEFAULT,
+            Self::DiagnosticOnUnknown => DEFAULT,
+        }
+    }
+
+    fn allowed_options(&self) -> &'static str {
+        const DEFAULT: &str = "only `message`, `note` and `label` are allowed as options";
+        match self {
+            Self::RustcOnUnimplemented => {
+                "see <https://rustc-dev-guide.rust-lang.org/diagnostics.html#rustc_on_unimplemented>"
+            }
+            Self::DiagnosticOnUnimplemented => DEFAULT,
+            Self::DiagnosticOnConst => DEFAULT,
+            Self::DiagnosticOnMove => DEFAULT,
+            Self::DiagnosticOnUnknown => DEFAULT,
+        }
+    }
+}
+
 fn merge_directives<S: Stage>(
     cx: &mut AcceptContext<'_, '_, S>,
     first: &mut Option<(Span, Directive)>,
@@ -83,6 +122,49 @@ fn merge<T, S: Stage>(
     }
 }
 
+fn parse_list<'p, S: Stage>(
+    cx: &mut AcceptContext<'_, '_, S>,
+    args: &'p ArgParser,
+    mode: Mode,
+) -> Option<&'p MetaItemListParser> {
+    let span = cx.attr_span;
+    match args {
+        ArgParser::List(items) if items.len() != 0 => return Some(items),
+        ArgParser::List(list) => {
+            // We're dealing with `#[diagnostic::attr()]`.
+            // This can be because that is what the user typed, but that's also what we'd see
+            // if the user used non-metaitem syntax. See `ArgParser::from_attr_args`.
+            cx.emit_lint(
+                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttributeLintKind::NonMetaItemDiagnosticAttribute,
+                list.span,
+            );
+        }
+        ArgParser::NoArgs => {
+            cx.emit_lint(
+                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttributeLintKind::MissingOptionsForDiagnosticAttribute {
+                    attribute: mode.as_str(),
+                    options: mode.expected_options(),
+                },
+                span,
+            );
+        }
+        ArgParser::NameValue(_) => {
+            cx.emit_lint(
+                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttributeLintKind::MalFormedDiagnosticAttribute {
+                    attribute: mode.as_str(),
+                    options: mode.allowed_options(),
+                    span,
+                },
+                span,
+            );
+        }
+    }
+    None
+}
+
 fn parse_directive_items<'p, S: Stage>(
     cx: &mut AcceptContext<'_, '_, S>,
     mode: Mode,
@@ -100,39 +182,15 @@ fn parse_directive_items<'p, S: Stage>(
         let span = item.span();
 
         macro malformed() {{
-            match mode {
-                Mode::RustcOnUnimplemented => {
-                    cx.emit_err(NoValueInOnUnimplemented { span: item.span() });
-                }
-                Mode::DiagnosticOnUnimplemented => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MalformedOnUnimplementedAttr { span },
-                        span,
-                    );
-                }
-                Mode::DiagnosticOnConst => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MalformedOnConstAttr { span },
-                        span,
-                    );
-                }
-                Mode::DiagnosticOnMove => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MalformedOnMoveAttr { span },
-                        span,
-                    );
-                }
-                Mode::DiagnosticOnUnknown => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MalformedOnUnknownAttr { span },
-                        span,
-                    );
-                }
-            }
+            cx.emit_lint(
+                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttributeLintKind::MalFormedDiagnosticAttribute {
+                    attribute: mode.as_str(),
+                    options: mode.allowed_options(),
+                    span,
+                },
+                span,
+            );
             continue;
         }}
 
@@ -146,22 +204,15 @@ fn parse_directive_items<'p, S: Stage>(
         }}
 
         macro duplicate($name: ident, $($first_span:tt)*) {{
-            match mode {
-                Mode::RustcOnUnimplemented => {
-                    cx.emit_err(NoValueInOnUnimplemented { span: item.span() });
-                }
-                Mode::DiagnosticOnUnimplemented |Mode::DiagnosticOnConst | Mode::DiagnosticOnMove | Mode::DiagnosticOnUnknown => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::IgnoredDiagnosticOption {
-                            first_span: $($first_span)*,
-                            later_span: span,
-                            option_name: $name,
-                        },
-                        span,
-                    );
-                }
-            }
+            cx.emit_lint(
+                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttributeLintKind::IgnoredDiagnosticOption {
+                    first_span: $($first_span)*,
+                    later_span: span,
+                    option_name: $name,
+                },
+                span,
+            );
         }}
 
         let item: &MetaItemParser = or_malformed!(item.meta_item()?);
@@ -540,15 +591,6 @@ pub(crate) enum InvalidOnClause {
     },
 }
 
-#[derive(Diagnostic)]
-#[diag("this attribute must have a value", code = E0232)]
-#[note("e.g. `#[rustc_on_unimplemented(message=\"foo\")]`")]
-pub(crate) struct NoValueInOnUnimplemented {
-    #[primary_span]
-    #[label("expected value here")]
-    pub span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag(
     "using multiple `rustc_on_unimplemented` (or mixing it with `diagnostic::on_unimplemented`) is not supported"

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_const.rs

@@ -1,6 +1,4 @@
 use rustc_hir::attrs::diagnostic::Directive;
-use rustc_hir::lints::AttributeLintKind;
-use rustc_session::lint::builtin::MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
@@ -22,30 +20,11 @@ impl<S: Stage> AttributeParser<S> for OnConstParser {
 
             let span = cx.attr_span;
             this.span = Some(span);
+            let mode = Mode::DiagnosticOnConst;
 
-            let items = match args {
-                ArgParser::List(items) if items.len() != 0 => items,
-                ArgParser::NoArgs | ArgParser::List(_) => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MissingOptionsForOnConst,
-                        span,
-                    );
-                    return;
-                }
-                ArgParser::NameValue(_) => {
-                    cx.emit_lint(
-                        MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        AttributeLintKind::MalformedOnConstAttr { span },
-                        span,
-                    );
-                    return;
-                }
-            };
+            let Some(items) = parse_list(cx, args, mode) else { return };
 
-            let Some(directive) =
-                parse_directive_items(cx, Mode::DiagnosticOnConst, items.mixed(), true)
-            else {
+            let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) else {
                 return;
             };
             merge_directives(cx, &mut this.directive, (span, directive));

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_move.rs

@@ -1,7 +1,5 @@
 use rustc_feature::template;
 use rustc_hir::attrs::AttributeKind;
-use rustc_hir::lints::AttributeLintKind;
-use rustc_session::lint::builtin::MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 use rustc_span::sym;
 
 use crate::attributes::diagnostic::*;
@@ -29,25 +27,10 @@ impl OnMoveParser {
 
         let span = cx.attr_span;
         self.span = Some(span);
-        let Some(list) = args.list() else {
-            cx.emit_lint(
-                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                AttributeLintKind::MissingOptionsForOnMove,
-                span,
-            );
-            return;
-        };
 
-        if list.is_empty() {
-            cx.emit_lint(
-                MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                AttributeLintKind::OnMoveMalformedAttrExpectedLiteralOrDelimiter,
-                list.span,
-            );
-            return;
-        }
+        let Some(items) = parse_list(cx, args, mode) else { return };
 
-        if let Some(directive) = parse_directive_items(cx, mode, list.mixed(), true) {
+        if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) {
             merge_directives(cx, &mut self.directive, (span, directive));
         }
     }

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unimplemented.rs

@@ -1,6 +1,4 @@
 use rustc_hir::attrs::diagnostic::Directive;
-use rustc_hir::lints::AttributeLintKind;
-use rustc_session::lint::builtin::MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
@@ -29,25 +27,7 @@ impl OnUnimplementedParser {
             return;
         }
 
-        let items = match args {
-            ArgParser::List(items) if items.len() != 0 => items,
-            ArgParser::NoArgs | ArgParser::List(_) => {
-                cx.emit_lint(
-                    MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                    AttributeLintKind::MissingOptionsForOnUnimplemented,
-                    span,
-                );
-                return;
-            }
-            ArgParser::NameValue(_) => {
-                cx.emit_lint(
-                    MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                    AttributeLintKind::MalformedOnUnimplementedAttr { span },
-                    span,
-                );
-                return;
-            }
-        };
+        let Some(items) = parse_list(cx, args, mode) else { return };
 
         if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) {
             merge_directives(cx, &mut self.directive, (span, directive));

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unknown.rs

@@ -1,5 +1,4 @@
 use rustc_hir::attrs::diagnostic::Directive;
-use rustc_session::lint::builtin::MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
@@ -23,25 +22,7 @@ impl OnUnknownParser {
         let span = cx.attr_span;
         self.span = Some(span);
 
-        let items = match args {
-            ArgParser::List(items) if !items.is_empty() => items,
-            ArgParser::NoArgs | ArgParser::List(_) => {
-                cx.emit_lint(
-                    MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                    AttributeLintKind::MissingOptionsForOnUnknown,
-                    span,
-                );
-                return;
-            }
-            ArgParser::NameValue(_) => {
-                cx.emit_lint(
-                    MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                    AttributeLintKind::MalformedOnUnknownAttr { span },
-                    span,
-                );
-                return;
-            }
-        };
+        let Some(items) = parse_list(cx, args, mode) else { return };
 
         if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) {
             merge_directives(cx, &mut self.directive, (span, directive));

compiler/rustc_error_codes/src/error_codes/E0232.md

@@ -1,19 +1,14 @@
 The `#[rustc_on_unimplemented]` attribute lets you specify a custom error
 message for when a particular trait isn't implemented on a type placed in a
-position that needs that trait. For example, when the following code is
-compiled:
+position that needs that trait. The attribute will let you filter on
+various types, with `on`:
 
 ```compile_fail,E0232
 #![feature(rustc_attrs)]
 #![allow(internal_features)]
 
-#[rustc_on_unimplemented(lorem="")] // error!
+#[rustc_on_unimplemented(on(blah, message = "foo"))] // error!
 trait BadAnnotation {}
 ```
-
-there will be an error about `bool` not implementing `Index<u8>`, followed by a
-note saying "the type `bool` cannot be indexed by `u8`".
-
-For this to work, some note must be specified. An empty attribute will not do
-anything, please remove the attribute or add some helpful note for users of the
-trait.
+For this to work a cfg-like predicate must be supplied. A malformed filter
+will not do anything.