Run debug_asserts during compile time for derives

[glittershark]

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Clap Version

3.0.0-beta.4

Describe your use case

Consider the following app:

use clap::Clap;

#[derive(Clap)]
struct Opts {
    #[clap(long, requires("foo"))]
    bar: i32
}

That app has a bug - the foo argument doesn't exist - but you don't have any way of finding out about it unless you actually run the application! In the real world, outside of the realm of contrived example code, this happened at $work where one parallel branch renamed an argument, while another added a new argument that required the old name - and since we didn't (previously, we do now) have a test covering option parsing, this went uncaught until we actually tried to run the binary.

Describe the solution you'd like

I was thinking it'd be awesome if #[derive(Clap)] could detect usages of requires (and other things that reference the name of another argument) and emit a compilation error if they reference arguments that don't exist. This might get a little hairy in the presence of #[clap(flatten)] et al, though, and I'm not super familiar with the internals of derive macros so the information may well just not be available at this point

Alternatives, if applicable

Maybe if it's not feasible to do this inside the macro itself (or not desired, because of the weird interaction with #[clap(flatten)] we could instead have some sort of custom clippy lint that detects this?

Additional Context

No response

[epage]

Yes, this would not be possible with flatten

I was thinking maybe we could if flatten isn't present but that wouldn't work if we were deriving a mix-in.

It looks like we do have a debug_assert for this case, unless there is a corner we are missing.

[glittershark]

I wonder how frequently people use requires on a field that isn't actually defined in the struct they're defining - I'd bet it's a minority of cases. I wonder if we could get away with something like

use clap::Clap;

#[derive(Clap)]
struct Opts {
    #[clap(long, requires("foo", allow_missing))]
    bar: i32
}

to opt-out of the compile-time check for the field existing.

[glittershark]

It looks like we do have a debug_assert for this case, unless there is a corner we are missing.

I'm not familiar with the structure of clap - is assert_app run as part of the derive macro (so at compile-time) or at runtime?

[epage]

I'm not familiar with the structure of clap - is assert_app run as part of the derive macro (so at compile-time) or at runtime?

It happens at runtime with debug builds. You could have your CI run --help to ensure all debug asserts pass which gives you at least some level of enforcement.

[glittershark]

You could have your CI run --help to ensure all debug asserts pass which gives you at least some level of enforcement.

Yeah, I just wrote a test that calls Opts::parse_from(...), but it'd be nice to have it as a compile-time check without having to remember to write a test.

[epage]

Thats understandable

[epage]

Note: #3133 is a subset of this issue.

[epage]

Due to the challenge of flatten, I'm inclined to close this.

Our options

• Only do the checks when flatten isn't present
  • This gives people a false sense of security as their app scales
  • This can cause challenges with the struct you flatten into your top-level. You could condition the assert on deriving Parser instead of Args but we let people derive Parser on those
• Require cross-struct references to opt-out
  • This is adding API and implementation complexity

On top of this, this requires us to duplicate our debug asserts.

We have added a App::debug_assert() for use in unit tests which is more extensive than the Opt::parse_from mentioned before (helps with subcommands).

If there are any concerns with us closing this, let us know!