Introduce a `#[diagnostic::on_unknown]` attribute#152901

weiznich · 2026-02-20T12:07:59Z

This PR introduces a #[diagnostic::on_unknown] attribute that allows crate authors to customize the error messages emitted by unresolved imports. The main usecase for this is using this attribute as part of a proc macro that expects a certain external module structure to exist or certain dependencies to be there.

For me personally the motivating use-case are several derives in diesel, that expect to refer to a tabe module. That is done either implicitly (via the name of the type with the derive) or explicitly by the user. This attribute would allow us to improve the error message in both cases:

For the implicit case we could explicity call out our assumptions (turning the name into lower case, adding an s in the end)

point to the explicit variant as alternative

For the explicit variant we would add additional notes to tell the user why this is happening and what they should look for to fix the problem (be more explicit about certain diesel specific assumptions of the module structure)

I assume that similar use-cases exist for other proc-macros as well, therefore I decided to put in the work implementing this new attribute. I would also assume that this is likely not useful for std-lib internal usage.

related #152900 and #128674

rustbot · 2026-02-20T12:08:02Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-02-20T12:08:05Z

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 68 candidates
Random selection from 15 candidates

jdonszelmann · 2026-02-20T19:42:26Z

@weiznich maybe this should be rebased on #151558

weiznich · 2026-02-22T14:24:32Z

I can try this, although I need to read up on how the new infrastructure works and check if it's possible to use this inside of the name resolution stage. Somehow certain things (like lints) act weirdly there.

jdonszelmann · 2026-02-23T22:59:36Z

The attribute refactor is pretty much finished, which means all old style parsers at this point have been removed from the compiler. There are many examples of new-parsing-infrastructure attribute parsers that work at this stage of the compiler (and none more that work using the old system). I don't think I want to accept any new old-style attribute parsers into the compiler anymore for that reason.

r? me

weiznich · 2026-02-24T06:31:05Z

@jdonszelmann I can totally understand that you don't want to accept any attributes using the old style. Your comment as currently written is still not useful for me as person that contributes only from time to time to the compiler and doesn't keep up with all the internal changes all the time. I get that I need to change something, but it is really unclear for me:

What need to change exactly
How the new attribute parsing framework works
If the new attribute parsing framework would allow me to access the relevant attribute information during name resolution, which as far as I know is kind of a strange place as it is between compiler phases and doesn't have access to the full set of information yet

It's especially not helpful to write that "There are many examples of new-parsing-infrastructure attribute parsers" without even providing a link to one of them.

Do you have any documentation or other hints where to get these information from? Otherwise I fear it's impossible for me to satisfy these requests with the limited amount of time I'm able to spend on this change.

jdonszelmann · 2026-02-24T10:22:00Z

Fair enough, take a look at how we handle MacroUse in rustc_resolve (grep for it). It should look similar to that. For that attribute we similarly have less information available, but it works using parse_limited.

mejrs

#151558 has merged now so you can rebase on that.

What need to change exactly

I think it'll look pretty similar to https://github.com/rust-lang/rust/blob/main/compiler/rustc_attr_parsing/src/attributes/diagnostic/on_const.rs
Add a new variant to https://github.com/rust-lang/rust/blob/main/compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs#L28
I think we should not(?) support formatting parameters like #[diagnostic::on_unknown_item(message = "{A}{B}{C}"]. To do that either change the logic in https://github.com/rust-lang/rust/blob/main/compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs#L287 or fire lints like at https://github.com/rust-lang/rust/blob/main/compiler/rustc_passes/src/check_attr.rs#L630 . I think the former is easier, especially if you want to reject "{Self}"

I have some review comments as well. Thanks for continuing this work by the way :)

View changes since this review

compiler/rustc_resolve/src/macros.rs

tests/ui/diagnostic_namespace/on_unknown_item/unknown_import.rs

tests/ui/diagnostic_namespace/on_unknown/multiple_errors.rs

jdonszelmann · 2026-02-26T09:08:09Z

@rustbot author

rustbot · 2026-02-26T09:08:14Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-03-06T10:15:13Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

weiznich · 2026-03-06T10:20:15Z

@rustbot ready

The new attribute infrastructure is really nice to work with as soon as you get your head around it. Thanks for all the effort that want into it.

jdonszelmann · 2026-04-05T10:08:22Z

@rustbot author

mejrs

I made some changes to the Directive api (see #154858). When you rebase please drop the FormatString::format and args -> Some(args) changes...

View changes since this review

mejrs · 2026-04-06T19:50:58Z

compiler/rustc_resolve/src/imports.rs

-        let msg = format!("unresolved import{} {}", pluralize!(paths.len()), paths.join(", "),);
-
-        let mut diag = struct_span_code_err!(self.dcx(), span, E0432, "{msg}");
+        let default_message =


...and here you'll need to do something like this:

let args = FormatArgs { this: paths.join(", ").collect(), // Unused this_sugared: String::new(), // Unused item_context: "", // Unused generic_args: Vec::new(), }; let CustomDiagnostic { message, label, notes, parent_label: _ } = directive.eval(None, &args);

rustbot · 2026-04-08T06:37:42Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

mejrs

Thanks for the rename. Can you update the PR title and description as well?

View changes since this review

mejrs · 2026-04-08T10:12:06Z

compiler/rustc_lint_defs/src/lib.rs

 pub enum FormatWarning {
    PositionalArgument { span: Span, help: String },
    InvalidSpecifier { name: String, span: Span },
+    DisallowedPlaceholder { span: Span },


This enum variant is never constructed.

One of your previous commits had

+ if matches!(mode, Mode::DiagnosticOnUnknownItem) { + warnings.push(FormatWarning::DisallowedPlaceholder { span }); + }

but I cannot find it anymore; you may have accidentally dropped it.

Please put this in the match in fn parse_arg and make it return FormatArg::AsIs. And add a test for it (idk if you had one, maybe it was also dropped?).

weiznich · 2026-04-08T10:33:28Z

I do not have much capacity or motivation left to do yet another set of changes on top of this.
Honestly it have been quite a frustrating experience to try getting this accepted over the past month. I can understand that you have other priorities but this PR got now rewritten three times to satisfy yet another round of feedback on topics that didn't change from the initial version. Additionally it was stuck on feedback from the reviewer for quite a lot of time, while in the meanwhile the same persons pushed changes that required several rather fundamental changes to the PR. This is then communicated with another tense comment that essentially says: Now change this again. Again I can understand that you might have other priorities and also I see that's your changes make things easier. It still is super frustrating for me to work in that environment.

mejrs · 2026-04-08T10:50:29Z

I'm sorry to hear that. I definitely understand the frustration as this part of the codebase is also very merge conflict prone. I still really want to see this happen, would you mind if I continue your work in the future?

weiznich · 2026-04-08T11:33:31Z

To clarify that again: The main point of frustration is not resolving the merge conflicts. The main point of frustration is that it feels you you as reviewer continuously shift the goal post on this by bringing up yet another thing that was there from the beginning and now suddenly needs to be changed. That in combination with out of my perspective delayed reviews and resolutions to open questions + merge conflicts from changes by exactly these reviews that delay the review in favor of the their changes make it very painful to continue working on this. It just feels disrespectful and especially not welcome to me. And to be extra clear: The question about the naming was open for more than 4 weeks with noone on the reviewers side even seem to care about it more than "that name doesn't sound perfect". You certainly can do such a thing, but please respect that at some point people just walk away and stop trying to contribute at all.

I'm still open to consider finishing that at some point, but at this point that would require a honest, final list of what still needs to be done that doesn't contain yet another rewrite. Also at least acknowledging that you (not as person but as reviewers) could have done better might help with the motivation.

This PR introduces a `#[diagnostic::on_unknown_item]` attribute that allows crate authors to customize the error messages emitted by unresolved imports. The main usecase for this is using this attribute as part of a proc macro that expects a certain external module structure to exist or certain dependencies to be there. For me personally the motivating use-case are several derives in diesel, that expect to refer to a `tabe` module. That is done either implicitly (via the name of the type with the derive) or explicitly by the user. This attribute would allow us to improve the error message in both cases: * For the implicit case we could explicity call out our assumptions (turning the name into lower case, adding an `s` in the end) + point to the explicit variant as alternative * For the explicit variant we would add additional notes to tell the user why this is happening and what they should look for to fix the problem (be more explicit about certain diesel specific assumptions of the module structure) I assume that similar use-cases exist for other proc-macros as well, therefore I decided to put in the work implementing this new attribute. I would also assume that this is likely not useful for std-lib internal usage.

jdonszelmann · 2026-04-09T15:16:32Z

This is quite a large change, which means review takes a bit and on re-review, new things can be spotted. In addition, both of us were reviewing, so not our opinions may align. I think that can be good actually: we can discuss how we want things to work. I think this is simply how review works in a large project with a lot of people involved.

Anyway, looks good to me as is.

@bors r=jdonszelmann,mejrs

Thanks for all the review work @mejrs and thanks for the implementation @weiznich

rust-bors · 2026-04-09T15:16:36Z

📌 Commit 93f9aec has been approved by jdonszelmann,mejrs

It is now in the queue for this repository.

⚠️ The following reviewer(s) could not be found: mejrs

weiznich · 2026-04-09T15:53:37Z

@jdonszelmann I can understand why it looks like that from your point of view, but let me assure that this hasn't been the case in the past. This kind of delay and bringing up of new remarks again and again is something that did not happen in the past. I write that as a person that contributes various larger PR's over the last years. I'm currently honestly considering if I want to continue that in the future, given that experience here.

…r=jdonszelmann,mejrs Introduce a `#[diagnostic::on_unknown]` attribute This PR introduces a `#[diagnostic::on_unknown]` attribute that allows crate authors to customize the error messages emitted by unresolved imports. The main usecase for this is using this attribute as part of a proc macro that expects a certain external module structure to exist or certain dependencies to be there. For me personally the motivating use-case are several derives in diesel, that expect to refer to a `tabe` module. That is done either implicitly (via the name of the type with the derive) or explicitly by the user. This attribute would allow us to improve the error message in both cases: * For the implicit case we could explicity call out our assumptions (turning the name into lower case, adding an `s` in the end) + point to the explicit variant as alternative * For the explicit variant we would add additional notes to tell the user why this is happening and what they should look for to fix the problem (be more explicit about certain diesel specific assumptions of the module structure) I assume that similar use-cases exist for other proc-macros as well, therefore I decided to put in the work implementing this new attribute. I would also assume that this is likely not useful for std-lib internal usage. related rust-lang#152900 and rust-lang#128674

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2026

rustbot assigned nnethercote Feb 20, 2026