Add on_missing_args

[chenyukang]

Fixes #152494
r? @estebank

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

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

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

Thanks, great idea. I love this.

A few points

• you need more tests, for invalid args, on invalid items, cross crate use, format parameters etc.
• format parameters; do these make sense? Currently they are not handled at all, just silently ignored. In check_attr.rs you can call directive_visit_params and issue diagnostics for them. Would it be useful to have indexed parameters like {1}, {2} to reference arguments? Something to think about, perhaps.
• 2c on the naming; I don't have a problem with the name perse, I just intensely dislike names that are really long. Also you don't have to use the exact name estebank suggested :) Something shorter would be lovely. Also consider whether this is something we could extend to functions etc?
• Can you please add some docs about this in the unstable book: https://doc.rust-lang.org/nightly/unstable-book/

View changes since this review

[chenyukang]

for naming, maybe on_missing_args maybe better, incase we can extend it in future?

[rustbot]

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 addressing my earlier comments (I cannot click resolve myself 😮‍💨).

Finally, I just remembered that declarative macro derives and attributes exist - can you add tests (and perhaps support for them) as well? See #145208 and #144579.

View changes since this review

[mejrs]

This is not quite what I understood from #152494.

I think @estebank's idea is that it should trigger when no macro arm matches, not just when it is incomplete. For example the current implementation does not trigger with the following invocations:

#[diagnostic::on_missing_args(
    note = "this macro expects a type and a value, like `pair!(u8, 0)`",
    note = "make sure to pass both arguments",
)]
macro_rules! pair {
    //~^ NOTE when calling this macro
    ($ty:ty, $value:expr) => {};
    //~^ NOTE while trying to match `,`
}

pair!(u8, <);
pair!(u8, 0, 42);

I think triggering on no match makes more sense. Maybe then the name should be on_no_match? on_failed_match? on_fallthrough? (please do wait for consensus before renaming everything 😅 ).

[chenyukang]

yes, seems 'on_unmatch_args` is a proper name, if we extend this diagnostic to more scenarios, such as like:

#![feature(diagnostic_on_missing_args)]

#[diagnostic::on_missing_args(
    message = "invalid route method",
    note = "this macro expects a action, like `{This}!(get \"/hello\")`"
)]
macro_rules! route {
    (get $path:literal) => {};
}

fn main() {
    route!(post "/");
}

we also can report:

error: invalid route method
  --> tests/ui/diagnostic_namespace/on_missing_args/other_match_macro_error.rs:12:12
   |
 7 | macro_rules! route {
   | ------------------ when calling this macro
...
12 |     route!(post "/");
   |            ^^^^ no rules expected this token in macro call
   |
note: while trying to match `get`
  --> tests/ui/diagnostic_namespace/on_missing_args/other_match_macro_error.rs:8:6
   |
 8 |     (get $path:literal) => {};
   |      ^^^
   = note: this macro expects a action, like `route!(get "/hello")`

error: aborting due to 1 previous error