I can't think of a way to unit test this.
Essentially the problem is that two different parts of the code need to be type-checked against each other, the .value_parser() bits and the .get_one() bits. But that type checking happens at runtime.
The only way I think we can test that is in the integration tests, by making sure every argument of every subcommand is tested, and checking that it doesn't panic.

It seems like Clap is very type-unsafe in this regard. I made a small effort to find other similar problems by eyeball, but there's just so many subcommands to cross-reference... Probably we need to have more rigorous testing of all CLI arguments that have a .value_parser().

@majaha Do you think migrating to clap_derive will help with that?

I imagine having something like the following will be better for cross-referencing:

#[arg(..., value_parser = ...)]
toolchain: Toolchain,

Type safety-wise I don't know much either, but maybe clap.rs already provides some sort of mechanism to avoid this kind of error? (cc @epage)

PS: The ValueParser selection mechanism looks like a potential candidate to me.

One way to unit test is to have clap::Command created by a factory function and wrapper functions around ArgMatches so you can have tests that parse arbitrary arguments and ensure the lookup doesn't panic independent of the rest of the behavior. Cargo does the first half of this but instead focuses on integration tests.

As for avoiding this completely, its either don't use value parsers or use the derive with inferred value parsers. Value parsers were primarily designed / optimized for the derive API.

@epage Thanks a lot for the help!

In that case, I suggest merging this one first and, eventually, addressing #3549.