better diagnostics for arguments

jdonszelmann · jdonszelmann · commit 865eb6534b0c · 2025-09-16T16:55:22.000-07:00
diff --git a/compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs
@@ -159,7 +159,7 @@ impl<S: Stage> AttributeParser<S> for NakedParser {
     const ATTRIBUTES: AcceptMapping<Self, S> =
         &[(&[sym::naked], template!(Word), |this, cx, args| {
             if let Err(span) = args.no_args() {
-                cx.expected_no_args(span);
+                cx.expected_no_args(&cx.attr_path, span);
                 return;
             }
 
diff --git a/compiler/rustc_attr_parsing/src/attributes/crate_level.rs b/compiler/rustc_attr_parsing/src/attributes/crate_level.rs
@@ -214,7 +214,7 @@ impl<S: Stage> CombineAttributeParser<S> for FeatureParser {
                 continue;
             };
             if let Err(arg_span) = elem.args().no_args() {
-                cx.expected_no_args(arg_span);
+                cx.expected_no_args(&elem.path().get_attribute_path(), arg_span);
                 continue;
             }
 
diff --git a/compiler/rustc_attr_parsing/src/attributes/macro_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/macro_attrs.rs
@@ -88,7 +88,10 @@ impl<S: Stage> AttributeParser<S> for MacroUseParser {
                                     continue;
                                 };
                                 if let Err(err_span) = item.args().no_args() {
-                                    cx.expected_no_args(err_span);
+                                    cx.expected_no_args(
+                                        &item.path().get_attribute_path(),
+                                        err_span,
+                                    );
                                     continue;
                                 }
                                 let Some(item) = item.path().word() else {
diff --git a/compiler/rustc_attr_parsing/src/attributes/mod.rs b/compiler/rustc_attr_parsing/src/attributes/mod.rs
@@ -280,7 +280,7 @@ impl<T: NoArgsAttributeParser<S>, S: Stage> SingleAttributeParser<S> for Without
 
     fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
         if let Err(span) = args.no_args() {
-            cx.expected_no_args(span);
+            cx.expected_no_args(&cx.attr_path, span);
         }
         Some(T::CREATE(cx.attr_span))
     }
diff --git a/compiler/rustc_attr_parsing/src/attributes/proc_macro_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/proc_macro_attrs.rs
@@ -88,13 +88,19 @@ fn parse_derive_like<S: Stage>(
         return None;
     }
     if let Err(e) = trait_attr.args().no_args() {
-        cx.expected_no_args(e);
+        cx.expected_no_args(&trait_attr.path().get_attribute_path(), e);
         return None;
     };
 
+    // updated if we see `attributes(...)` to keep track of the last
+    // argument we did accept for the final diagnostic
+    let mut last = trait_ident.span;
+
     // Parse optional attributes
     let mut attributes = ThinVec::new();
     if let Some(attrs) = items.next() {
+        last = attrs.span();
+
         let Some(attr_list) = attrs.meta_item() else {
             cx.expected_list(attrs.span());
             return None;
@@ -115,7 +121,7 @@ fn parse_derive_like<S: Stage>(
                 return None;
             };
             if let Err(e) = attr.args().no_args() {
-                cx.expected_no_args(e);
+                cx.expected_no_args(&attr.path().get_attribute_path(), e);
                 return None;
             };
             let Some(ident) = attr.path().word() else {
@@ -132,7 +138,7 @@ fn parse_derive_like<S: Stage>(
 
     // If anything else is specified, we should reject it
     if let Some(next) = items.next() {
-        cx.expected_no_args(next.span());
+        cx.expected_end_of_list(last, next.span());
     }
 
     Some((Some(trait_ident.name), attributes))
diff --git a/compiler/rustc_attr_parsing/src/attributes/rustc_internal.rs b/compiler/rustc_attr_parsing/src/attributes/rustc_internal.rs
@@ -42,7 +42,7 @@ impl<S: Stage> SingleAttributeParser<S> for RustcObjectLifetimeDefaultParser {
 
     fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
         if let Err(span) = args.no_args() {
-            cx.expected_no_args(span);
+            cx.expected_no_args(&cx.attr_path, span);
             return None;
         }
 
diff --git a/compiler/rustc_attr_parsing/src/attributes/traits.rs b/compiler/rustc_attr_parsing/src/attributes/traits.rs
@@ -35,7 +35,7 @@ impl<S: Stage> SingleAttributeParser<S> for SkipDuringMethodDispatchParser {
                 continue;
             };
             if let Err(span) = arg.args().no_args() {
-                cx.expected_no_args(span);
+                cx.expected_no_args(&arg.path().get_attribute_path(), span);
             }
             let path = arg.path();
             let (key, skip): (Symbol, &mut bool) = match path.word_sym() {
diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs
@@ -448,13 +448,13 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         })
     }
 
-    pub(crate) fn expected_no_args(&self, args_span: Span) -> ErrorGuaranteed {
+    pub(crate) fn expected_no_args(&self, path: &AttrPath, args_span: Span) -> ErrorGuaranteed {
         self.emit_err(AttributeParseError {
             span: args_span,
             attr_span: self.attr_span,
             template: self.template.clone(),
             attribute: self.attr_path.clone(),
-            reason: AttributeParseErrorReason::ExpectedNoArgs,
+            reason: AttributeParseErrorReason::ExpectedNoArgs { path },
             attr_style: self.attr_style,
         })
     }
@@ -509,6 +509,21 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         })
     }
 
+    /// Expected the end of an argument list.
+    ///
+    /// Note: only useful when arguments in an attribute are ordered and we've seen the last one we expected.
+    /// Most attributes shouldn't care about their argument order.
+    pub(crate) fn expected_end_of_list(&self, last_item_span: Span, span: Span) -> ErrorGuaranteed {
+        self.emit_err(AttributeParseError {
+            span,
+            attr_span: self.attr_span,
+            template: self.template.clone(),
+            attribute: self.attr_path.clone(),
+            reason: AttributeParseErrorReason::ExpectedEnd { last: last_item_span },
+            attr_style: self.attr_style,
+        })
+    }
+
     pub(crate) fn expected_single_argument(&self, span: Span) -> ErrorGuaranteed {
         self.emit_err(AttributeParseError {
             span,
diff --git a/compiler/rustc_attr_parsing/src/session_diagnostics.rs b/compiler/rustc_attr_parsing/src/session_diagnostics.rs
@@ -559,7 +559,9 @@ pub(crate) struct LinkOrdinalOutOfRange {
 }
 
 pub(crate) enum AttributeParseErrorReason<'a> {
-    ExpectedNoArgs,
+    ExpectedNoArgs {
+        path: &'a AttrPath,
+    },
     ExpectedStringLiteral {
         byte_string: Option<Span>,
     },
@@ -577,6 +579,9 @@ pub(crate) enum AttributeParseErrorReason<'a> {
         list: bool,
     },
     ExpectedIdentifier,
+    ExpectedEnd {
+        last: Span,
+    },
 }
 
 pub(crate) struct AttributeParseError<'a> {
@@ -588,13 +593,28 @@ pub(crate) struct AttributeParseError<'a> {
     pub(crate) reason: AttributeParseErrorReason<'a>,
 }
 
+/// based on the attribute's template we add relevant suggestions to the error automatically.
+enum DefaultSuggestionStyle {
+    /// give a hint about the valid forms of the attribute.
+    /// Useful if there's already a better suggestion given than the automatic ones can provide
+    /// but we'd still like to show which syntax forms are valid.
+    Hint,
+    /// Use the template to suggest changes to the attribute
+    Suggestion,
+    /// Don't show any default suggestions
+    None,
+}
+
 impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError<'_> {
     fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, G> {
         let name = self.attribute.to_string();
 
         let mut diag = Diag::new(dcx, level, format!("malformed `{name}` attribute input"));
         diag.span(self.attr_span);
         diag.code(E0539);
+
+        let mut show_default_suggestions = DefaultSuggestionStyle::Suggestion;
+
         match self.reason {
             AttributeParseErrorReason::ExpectedStringLiteral { byte_string } => {
                 if let Some(start_point_span) = byte_string {
@@ -606,7 +626,7 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError<'_> {
                     );
                     diag.note("expected a normal string literal, not a byte string literal");
 
-                    return diag;
+                    show_default_suggestions = DefaultSuggestionStyle::None;
                 } else {
                     diag.span_label(self.span, "expected a string literal here");
                 }
@@ -632,9 +652,19 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError<'_> {
                 diag.span_label(self.span, "didn't expect a literal here");
                 diag.code(E0565);
             }
-            AttributeParseErrorReason::ExpectedNoArgs => {
+            AttributeParseErrorReason::ExpectedNoArgs { path } => {
                 diag.span_label(self.span, "didn't expect any arguments here");
                 diag.code(E0565);
+
+                if path.span != self.attribute.span {
+                    diag.span_suggestion(
+                        path.span.to(self.span),
+                        "remove this argument",
+                        path,
+                        Applicability::MachineApplicable,
+                    );
+                    show_default_suggestions = DefaultSuggestionStyle::Hint;
+                }
             }
             AttributeParseErrorReason::ExpectedNameValue(None) => {
                 // If the span is the entire attribute, the suggestion we add below this match already contains enough information
@@ -716,23 +746,36 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError<'_> {
             AttributeParseErrorReason::ExpectedIdentifier => {
                 diag.span_label(self.span, "expected a valid identifier here");
             }
+            AttributeParseErrorReason::ExpectedEnd { last } => {
+                diag.span_label(last, "expected no more arguments after this");
+                diag.span_label(self.span, "remove this argument");
+            }
         }
 
         if let Some(link) = self.template.docs {
             diag.note(format!("for more information, visit <{link}>"));
         }
+
         let suggestions = self.template.suggestions(self.attr_style, &name);
+        let text = match show_default_suggestions {
+            DefaultSuggestionStyle::Hint => {
+                if suggestions.len() == 1 {
+                    "the only valid form of the attribute is"
+                } else {
+                    "these are the valid forms of the attribute"
+                }
+            }
+            DefaultSuggestionStyle::Suggestion => {
+                if suggestions.len() == 1 {
+                    "must be of the form"
+                } else {
+                    "try changing it to one of the following valid forms of the attribute"
+                }
+            }
+            DefaultSuggestionStyle::None => return diag,
+        };
 
-        diag.span_suggestions(
-            self.attr_span,
-            if suggestions.len() == 1 {
-                "must be of the form"
-            } else {
-                "try changing it to one of the following valid forms of the attribute"
-            },
-            suggestions,
-            Applicability::HasPlaceholders,
-        );
+        diag.span_suggestions(self.attr_span, text, suggestions, Applicability::HasPlaceholders);
 
         diag
     }
diff --git a/tests/ui/attributes/invalid-macro-use.stderr b/tests/ui/attributes/invalid-macro-use.stderr
@@ -43,7 +43,12 @@ LL | #[macro_use(a = "b")]
    |               didn't expect any arguments here
    |
    = note: for more information, visit <https://doc.rust-lang.org/reference/macros-by-example.html#the-macro_use-attribute>
-help: try changing it to one of the following valid forms of the attribute
+help: remove this argument
+   |
+LL - #[macro_use(a = "b")]
+LL + #[macro_use(a)]
+   |
+help: these are the valid forms of the attribute
    |
 LL - #[macro_use(a = "b")]
 LL + #[macro_use(name1, name2, ...)]
@@ -61,7 +66,12 @@ LL | #[macro_use(a(b))]
    |              didn't expect any arguments here
    |
    = note: for more information, visit <https://doc.rust-lang.org/reference/macros-by-example.html#the-macro_use-attribute>
-help: try changing it to one of the following valid forms of the attribute
+help: remove this argument
+   |
+LL - #[macro_use(a(b))]
+LL + #[macro_use(a)]
+   |
+help: these are the valid forms of the attribute
    |
 LL - #[macro_use(a(b))]
 LL + #[macro_use(name1, name2, ...)]
diff --git a/tests/ui/attributes/rustc_skip_during_method_dispatch.stderr b/tests/ui/attributes/rustc_skip_during_method_dispatch.stderr
@@ -48,9 +48,19 @@ error[E0565]: malformed `rustc_skip_during_method_dispatch` attribute input
    |
 LL | #[rustc_skip_during_method_dispatch(array = true)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^------^^
-   | |                                         |
-   | |                                         didn't expect any arguments here
-   | help: must be of the form: `#[rustc_skip_during_method_dispatch(array, boxed_slice)]`
+   |                                           |
+   |                                           didn't expect any arguments here
+   |
+help: remove this argument
+   |
+LL - #[rustc_skip_during_method_dispatch(array = true)]
+LL + #[rustc_skip_during_method_dispatch(array)]
+   |
+help: the only valid form of the attribute is
+   |
+LL - #[rustc_skip_during_method_dispatch(array = true)]
+LL + #[rustc_skip_during_method_dispatch(array, boxed_slice)]
+   |
 
 error[E0565]: malformed `rustc_skip_during_method_dispatch` attribute input
   --> $DIR/rustc_skip_during_method_dispatch.rs:27:1
diff --git a/tests/ui/attributes/unsafe/proc-unsafe-attributes.stderr b/tests/ui/attributes/unsafe/proc-unsafe-attributes.stderr
@@ -123,7 +123,12 @@ LL | #[proc_macro_derive(unsafe(Foo))]
    |                           didn't expect any arguments here
    |
    = note: for more information, visit <https://doc.rust-lang.org/reference/procedural-macros.html#derive-macros>
-help: try changing it to one of the following valid forms of the attribute
+help: remove this argument
+   |
+LL - #[proc_macro_derive(unsafe(Foo))]
+LL + #[proc_macro_derive(r#unsafe)]
+   |
+help: these are the valid forms of the attribute
    |
 LL - #[proc_macro_derive(unsafe(Foo))]
 LL + #[proc_macro_derive(TraitName)]
diff --git a/tests/ui/conditional-compilation/cfg-attr-syntax-validation.rs b/tests/ui/conditional-compilation/cfg-attr-syntax-validation.rs
@@ -36,16 +36,17 @@ struct S7;
 //~| NOTE for more information, visit
 struct S8;
 
-#[cfg(a = b"hi")]  //~ ERROR malformed `cfg` attribute input
+#[cfg(a = b"hi")] //~ ERROR malformed `cfg` attribute input
 //~^ NOTE expected a normal string literal, not a byte string literal
+//~| NOTE: for more information, visit
 struct S9;
 
 macro_rules! generate_s10 {
     ($expr: expr) => {
         #[cfg(feature = $expr)]
         //~^ ERROR expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found `expr` metavariable
         struct S10;
-    }
+    };
 }
 
 generate_s10!(concat!("nonexistent"));
diff --git a/tests/ui/conditional-compilation/cfg-attr-syntax-validation.stderr b/tests/ui/conditional-compilation/cfg-attr-syntax-validation.stderr
@@ -80,9 +80,10 @@ LL | #[cfg(a = b"hi")]
    |           help: consider removing the prefix
    |
    = note: expected a normal string literal, not a byte string literal
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg-attribute>
 
 error: expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found `expr` metavariable
-  --> $DIR/cfg-attr-syntax-validation.rs:45:25
+  --> $DIR/cfg-attr-syntax-validation.rs:46:25
    |
 LL |         #[cfg(feature = $expr)]
    |                         ^^^^^
diff --git a/tests/ui/feature-gates/gated-bad-feature.stderr b/tests/ui/feature-gates/gated-bad-feature.stderr
diff --git a/tests/ui/macros/macro-use-bad-args-1.stderr b/tests/ui/macros/macro-use-bad-args-1.stderr
diff --git a/tests/ui/macros/macro-use-bad-args-2.stderr b/tests/ui/macros/macro-use-bad-args-2.stderr
diff --git a/tests/ui/proc-macro/attribute.rs b/tests/ui/proc-macro/attribute.rs
diff --git a/tests/ui/proc-macro/attribute.stderr b/tests/ui/proc-macro/attribute.stderr

Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ impl<S: Stage> AttributeParser<S> for NakedParser {`
`159`	`159`	`const ATTRIBUTES: AcceptMapping<Self, S> =`
`160`	`160`	`&[(&[sym::naked], template!(Word), \|this, cx, args\| {`
`161`	`161`	`if let Err(span) = args.no_args() {`
`162`		`- cx.expected_no_args(span);`
	`162`	`+ cx.expected_no_args(&cx.attr_path, span);`
`163`	`163`	`return;`
`164`	`164`	`}`
`165`	`165`
Original file line number	Diff line number	Diff line change
`@@ -214,7 +214,7 @@ impl<S: Stage> CombineAttributeParser<S> for FeatureParser {`
`214`	`214`	`continue;`
`215`	`215`	`};`
`216`	`216`	`if let Err(arg_span) = elem.args().no_args() {`
`217`		`- cx.expected_no_args(arg_span);`
	`217`	`+ cx.expected_no_args(&elem.path().get_attribute_path(), arg_span);`
`218`	`218`	`continue;`
`219`	`219`	`}`
`220`	`220`
Original file line number	Diff line number	Diff line change
`@@ -280,7 +280,7 @@ impl<T: NoArgsAttributeParser<S>, S: Stage> SingleAttributeParser<S> for Without`
`280`	`280`
`281`	`281`	`fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {`
`282`	`282`	`if let Err(span) = args.no_args() {`
`283`		`- cx.expected_no_args(span);`
	`283`	`+ cx.expected_no_args(&cx.attr_path, span);`
`284`	`284`	`}`
`285`	`285`	`Some(T::CREATE(cx.attr_span))`
`286`	`286`	`}`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ impl<S: Stage> SingleAttributeParser<S> for RustcObjectLifetimeDefaultParser {`
`42`	`42`
`43`	`43`	`fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {`
`44`	`44`	`if let Err(span) = args.no_args() {`
`45`		`- cx.expected_no_args(span);`
	`45`	`+ cx.expected_no_args(&cx.attr_path, span);`
`46`	`46`	`return None;`
`47`	`47`	`}`
`48`	`48`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ impl<S: Stage> SingleAttributeParser<S> for SkipDuringMethodDispatchParser {`
`35`	`35`	`continue;`
`36`	`36`	`};`
`37`	`37`	`if let Err(span) = arg.args().no_args() {`
`38`		`- cx.expected_no_args(span);`
	`38`	`+ cx.expected_no_args(&arg.path().get_attribute_path(), span);`
`39`	`39`	`}`
`40`	`40`	`let path = arg.path();`
`41`	`41`	`let (key, skip): (Symbol, &mut bool) = match path.word_sym() {`