Skip to content

Introduce a #[diagnostic::on_unknown_item] attribute#152901

Open
weiznich wants to merge 1 commit intorust-lang:mainfrom
weiznich:feature/on_unknown_item
Open

Introduce a #[diagnostic::on_unknown_item] attribute#152901
weiznich wants to merge 1 commit intorust-lang:mainfrom
weiznich:feature/on_unknown_item

Conversation

@weiznich
Copy link
Contributor

@weiznich weiznich commented Feb 20, 2026

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.

related #152900 and #128674

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2026

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

cc @jdonszelmann, @JonathanBrouwer

@rustbot 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
Copy link
Collaborator

rustbot commented Feb 20, 2026

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

@rust-log-analyzer

This comment has been minimized.

@weiznich weiznich force-pushed the feature/on_unknown_item branch from 5c63f6f to a9dd689 Compare February 20, 2026 13:33
@rust-log-analyzer

This comment has been minimized.

@weiznich weiznich force-pushed the feature/on_unknown_item branch from a9dd689 to c87fc9e Compare February 20, 2026 14:37
@jdonszelmann
Copy link
Contributor

@weiznich maybe this should be rebased on #151558

@weiznich
Copy link
Contributor Author

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
Copy link
Contributor

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

@rustbot rustbot assigned jdonszelmann and unassigned nnethercote Feb 23, 2026
@weiznich
Copy link
Contributor Author

@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
Copy link
Contributor

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.

@rust-bors

This comment has been minimized.

Copy link
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

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

What need to change exactly

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

View changes since this review

Comment on lines +703 to +711
const DIAG_ATTRS: &[Symbol] =
&[sym::on_unimplemented, sym::do_not_recommend, sym::on_const];
&[sym::on_unimplemented, sym::do_not_recommend, sym::on_const, sym::on_unknown_item];

if res == Res::NonMacroAttr(NonMacroAttrKind::Tool)
&& let [namespace, attribute, ..] = &*path.segments
&& namespace.ident.name == sym::diagnostic
&& !DIAG_ATTRS.contains(&attribute.ident.name)
&& (!DIAG_ATTRS.contains(&attribute.ident.name)
|| (attribute.ident.name == sym::on_unknown_item
&& !self.tcx.features().diagnostic_on_unknown_item()))
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to handle this in a more general way since on_const and (soon) on_move are also unstable. Such an expression is going to be unwieldy if we do this for those as well.

something like let diag_attrs = /* vec of all accessible diagnostic attrs */

Comment on lines +1 to +17
#![feature(diagnostic_on_unknown_item)]
pub mod foo {
pub struct Bar;
}

#[diagnostic::on_unknown_item(
message = "first message",
label = "first label",
note = "custom note",
note = "custom note 2"
)]
use foo::Foo;
//~^ERROR first message

use foo::Bar;

fn main() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you intend to support extern crate declarations? If so please add a test like

#[diagnostic::on_unknown_item(
    message = "first message",
    label = "first label",
    note = "custom note",
    note = "custom note 2"
)]
extern crate foo;

Or if you don't, add it to incorrect-locations.rs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's currently not supported

//~^ ERROR: custom message
};
}
fn main() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test for use inside the use declaration, like

mod test5 {
    use std::{
        string::String,
        #[diagnostic::on_unknown_item(
           message = "custom message",
           label = "custom label",
            note = "custom note"
        )]
        vec::{NonExisting, Vec},
        //~^ ERROR: custom message
    };
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if that's meaningful given that attributes are not allowed in this location:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=cc07eba36fd33ac462672ccbc2a7e5c7

(I mean I can add it to incorrect-locations.rs, but I'm not sure if it is worth that)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I hadn't realized that's not syntactically valid. Nevermind then 😆

@jdonszelmann
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2026

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

@weiznich weiznich force-pushed the feature/on_unknown_item branch from c87fc9e to fad08dd Compare March 6, 2026 10:15
@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2026

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

@rustbot

This comment has been minimized.

@weiznich weiznich force-pushed the feature/on_unknown_item branch from fad08dd to 6319918 Compare March 6, 2026 10:19
@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2026

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.

@weiznich
Copy link
Contributor Author

weiznich commented Mar 6, 2026

@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.

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 6, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2026
@rust-log-analyzer

This comment has been minimized.

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.
@weiznich weiznich force-pushed the feature/on_unknown_item branch from 6319918 to 7e81115 Compare March 6, 2026 11:22
Copy link
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

👍 on the implementation, just some nits.

I have some general questions:

  • this PR (and the tracking issue) are pretty light on motivation. I think I roughly understand the use case and what you want to use it for (I remember something to do with the fallout from cargo's resolver being weird?). Can you give an example?
  • the intent seems to be that macro authors inject this attribute into their users code. Are there use cases that don't involve macros? This also means that the feature has to be activated by the users, not by you, which seems unfortunate.
  • regarding the name, "unknown item" can reasonably be understood to mean more than unresolved use statements. Perhaps change the name to something like on_unresolved_import?

View changes since this review

Comment on lines +858 to +870
let mut diag = if self.tcx.features().diagnostic_on_unknown_item()
&& let Some((_, message)) =
errors[0].1.on_unknown_item_attr.as_mut().and_then(|a| a.directive.message.take())
{
let message = message.format(None);
let mut diag = struct_span_code_err!(self.dcx(), span, E0432, "{message}");
diag.note(default_message);
diag
} else {
struct_span_code_err!(self.dcx(), span, E0432, "{default_message}")
};

if let Some((_, UnresolvedImportError { note: Some(note), .. })) = errors.iter().last() {
if self.tcx.features().diagnostic_on_unknown_item()
Copy link
Contributor

Choose a reason for hiding this comment

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

When I ported on_const I put the feature check during parsing

if !cx.features().diagnostic_on_const() {
return;
}

rather than at the use site.

I don't have strong feelings either way but if it is checked during parsing (before the this.span = Some(span); statement) then the compiler ignores it completely which simplifies the code at the use site.

Also I do intend on making a diagnostic for when unstable diagnostic attrs are used without the feature, which would move this check there anyway.

this.parse(cx, args, Mode::DiagnosticOnUnknownItem);
},
)];
//FIXME attribute is not parsed for non-traits but diagnostics are issued in `check_attr.rs`
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment looks like a copy paste error

@jdonszelmann jdonszelmann added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2026
@jdonszelmann
Copy link
Contributor

(ping me for final review when mejrs' comments are processed)

@weiznich
Copy link
Contributor Author

weiznich commented Mar 9, 2026

this PR (and the tracking issue) are pretty light on motivation. I think I roughly understand the use case and what you want to use it for (I remember something to do with the fallout from cargo's resolver being weird?). Can you give an example?

It's unrelated to cargo

My main motivation for this goes as follows: Diesel has several derives that need to know the "table struct" from the schema.rs file.

Users write code like this:

#[derive(HasQuery)]
struct User { // if no `use crate::schema::users` (or similar) was in scope we would like to inform the user about that requirement
 // fields, which don't matter for this example
}

or this

#[derive(HasQuery)]
#[diesel(table_name = users)] // if that was missing we would like to have a message that says: If your table is not named `user_with_some_special_fieldss` please use this attribute
struct UserWithSomeSpecialFields {
 // fields, which don't matter for this example
}

which for both cases assumes that an import like crate::schema::users is in scope. Especially for the case where the name is implicitly generated (just struct name + s in lowercase) new users are often confused about the relevant error. The idea is to essentially extend this specific error case to:

  • Explain what's going on by typing out the rules there as well
  • Point to the relevant attribute (mostly #[diesel(table_name = …)])

Hopefully that provides enough context and a motivating use case.

the intent seems to be that macro authors inject this attribute into their users code. Are there use cases that don't involve macros? This also means that the feature has to be activated by the users, not by you, which seems unfortunate.

Yes this is likely to be injected in macro generated code and yes having it behind a feature flag is not great. I just wanted to not try to argue having it as ungated nightly only option in the diagnostic namespace for now (or even just let it ride the train to stable) as that might require bigger discussions.

regarding the name, "unknown item" can reasonably be understood to mean more than unresolved use statements. Perhaps change the name to something like on_unresolved_import?

I'm happy to change it to whatever the consensus is about the naming here. I just went with the first thing that came up in my mind, but naming is hard as you all know.

I will try to address the other comments in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants