Skip to content

Clean up AttributeLintKind and refactor diagnostic attribute linting #155215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 8 commits into from
Apr 16, 2026
Merged

Clean up AttributeLintKind and refactor diagnostic attribute linting #155215

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e9d0e32
Merge malformed diagnostic attribute lint
mejrs Apr 12, 2026
57fd974
Merge "missing options" lints for diagnostic attributes
mejrs Apr 12, 2026
217c073
move list parsing into its own function
mejrs Apr 12, 2026
0dd591a
Delete unused `AttributeLintKind` variant
mejrs Apr 12, 2026
f468cef
Format the duplicate macro
mejrs Apr 12, 2026
75273e2
Warn instead of error for some ructs_on_unimplemented errors
mejrs Apr 14, 2026
72db94f
Pass allowed options as parameter to diagnostic lints
mejrs Apr 14, 2026
d6bccac
Fix error code example
mejrs Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 72 additions & 46 deletions compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ 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 merge_directives<S: Stage>(
cx: &mut AcceptContext<'_, '_, S>,
first: &mut Option<(Span, Directive)>,
Expand Down Expand Up @@ -83,6 +95,44 @@ 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(),
},
span,
);
}
ArgParser::NameValue(_) => {
cx.emit_lint(
MALFORMED_DIAGNOSTIC_ATTRIBUTES,
AttributeLintKind::MalFormedDiagnosticAttribute { attribute: mode.as_str(), span },
span,
);
}
}
None
}

fn parse_directive_items<'p, S: Stage>(
cx: &mut AcceptContext<'_, '_, S>,
mode: Mode,
Expand All @@ -100,38 +150,17 @@ 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 },
if matches!(mode, Mode::RustcOnUnimplemented) {
cx.emit_err(NoValueInOnUnimplemented { span: item.span() });
} else {
cx.emit_lint(
MALFORMED_DIAGNOSTIC_ATTRIBUTES,
AttributeLintKind::MalFormedDiagnosticAttribute {
attribute: mode.as_str(),
span,
);
}
Mode::DiagnosticOnUnknown => {
cx.emit_lint(
MALFORMED_DIAGNOSTIC_ATTRIBUTES,
AttributeLintKind::MalformedOnUnknownAttr { span },
span,
);
}
},
span,
);
}
continue;
}}
Expand All @@ -146,21 +175,18 @@ 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,
);
}
if matches!(mode, Mode::RustcOnUnimplemented) {
Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like matches! here because if we ever add another rustc diagnostic attribute, we could forget to check it. How about a method mode.should_error(), that has an exhaustive match in it?

View changes since the review

Copy link
Copy Markdown
Contributor Author

@mejrs mejrs Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't actually matter, assuming we don't ship a std that compiles with warnings?

So far my decisions on whether rustc_on_unimplemented errors or issues a diagnostic namespace lint have been largely focused on keeping the implementation simple:

  • erroring is easier than adding AttributeLintKind variants
  • so hard error in rustc_on_unimplemented-only paths to simplify there
  • issue lints in shared paths so you don't have to care about which attr it is

In fact, maybe this should just be a warning regardless?

Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That mentality also sounds reasonable to me, lets make this a warning to keep the implementation simple

Copy link
Copy Markdown
Contributor Author

@mejrs mejrs Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I remember why it is this way, I applied the suggestion and then I got annoyed because

LL | #[rustc_on_unimplemented(lorem = "")]
   |                          ^^^^^^^^^^ invalid option found here
   |
   = help: only `message`, `note` and `label` are allowed as options

is not quite right.

The last commit fixes that by letting you customize the "allowed options" list per attribute. Note that I'm inclined to have an attribute with only "note" allowed, (#155200 (comment)) so we will need some kind of customization (or more lint variants) anyway.

Let me know what you think about the last few commits, idm backing it out if you prefer.

Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, especially since we need this in the future :)

cx.emit_err(NoValueInOnUnimplemented { span: item.span() });
} else {
cx.emit_lint(
MALFORMED_DIAGNOSTIC_ATTRIBUTES,
AttributeLintKind::IgnoredDiagnosticOption {
first_span: $($first_span)*,
later_span: span,
option_name: $name,
},
span,
);
}
}}

Expand Down
27 changes: 3 additions & 24 deletions compiler/rustc_attr_parsing/src/attributes/diagnostic/on_const.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::*;
Expand All @@ -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));
Expand Down
21 changes: 2 additions & 19 deletions compiler/rustc_attr_parsing/src/attributes/diagnostic/on_move.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -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));
}
}
Expand Down
22 changes: 1 addition & 21 deletions compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unimplemented.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -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));
Expand Down
21 changes: 1 addition & 20 deletions compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unknown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::*;
Expand All @@ -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));
Expand Down
34 changes: 7 additions & 27 deletions compiler/rustc_lint/src/early/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,10 @@ impl<'a> Diagnostic<'a, ()> for DecorateAttrLint<'_, '_, '_> {
&AttributeLintKind::ExpectedNoArgs => lints::ExpectedNoArgs.into_diag(dcx, level),

&AttributeLintKind::ExpectedNameValue => lints::ExpectedNameValue.into_diag(dcx, level),
&AttributeLintKind::MalformedOnUnimplementedAttr { span } => {
lints::MalformedOnUnimplementedAttrLint { span }.into_diag(dcx, level)
}
&AttributeLintKind::MalformedOnUnknownAttr { span } => {
lints::MalformedOnUnknownAttrLint { span }.into_diag(dcx, level)
}
&AttributeLintKind::MalformedOnConstAttr { span } => {
lints::MalformedOnConstAttrLint { span }.into_diag(dcx, level)
&AttributeLintKind::MalFormedDiagnosticAttribute { attribute, span } => {
lints::MalFormedDiagnosticAttributeLint { attribute, span }.into_diag(dcx, level)
}

AttributeLintKind::MalformedDiagnosticFormat { warning } => match warning {
FormatWarning::PositionalArgument { .. } => {
lints::DisallowedPositionalArgument.into_diag(dcx, level)
Expand All @@ -203,26 +198,11 @@ impl<'a> Diagnostic<'a, ()> for DecorateAttrLint<'_, '_, '_> {
lints::IgnoredDiagnosticOption { option_name, first_span, later_span }
.into_diag(dcx, level)
}
&AttributeLintKind::MissingOptionsForOnUnimplemented => {
lints::MissingOptionsForOnUnimplementedAttr.into_diag(dcx, level)
}
&AttributeLintKind::MissingOptionsForOnConst => {
lints::MissingOptionsForOnConstAttr.into_diag(dcx, level)
}
&AttributeLintKind::MalformedOnMoveAttr { span } => {
lints::MalformedOnMoveAttrLint { span }.into_diag(dcx, level)
}
&AttributeLintKind::OnMoveMalformedFormatLiterals { name } => {
lints::OnMoveMalformedFormatLiterals { name }.into_diag(dcx, level)
}
&AttributeLintKind::OnMoveMalformedAttrExpectedLiteralOrDelimiter => {
lints::OnMoveMalformedAttrExpectedLiteralOrDelimiter.into_diag(dcx, level)
}
&AttributeLintKind::MissingOptionsForOnMove => {
lints::MissingOptionsForOnMoveAttr.into_diag(dcx, level)
&AttributeLintKind::MissingOptionsForDiagnosticAttribute { attribute } => {
lints::MissingOptionsForDiagnosticAttribute { attribute }.into_diag(dcx, level)
}
&AttributeLintKind::MissingOptionsForOnUnknown => {
lints::MissingOptionsForOnUnknownAttr.into_diag(dcx, level)
&AttributeLintKind::NonMetaItemDiagnosticAttribute => {
lints::NonMetaItemDiagnosticAttribute.into_diag(dcx, level)
}
}
}
Expand Down
Loading
Loading