Duplicate short option (and other misconfigurations) gives runtime error, wouldn't it be cool, if it was compile time error

[epage]

Issue by typetetris
Tuesday Oct 05, 2021 at 13:32 GMT
Originally opened as TeXitoi/structopt#500

---

Something like

use structopt::StructOpt;

#[derive(Debug,StructOpt)]
struct Opt {
      #[structopt(short, long)]
      fnord: u16,

      #[structopt(short, long)]
      file: String
}

gives an error at runtime of the program.

It would be way cooler, if it was a compile time error.

If you agree, I could look into trying to implement this.

[epage]

Comment by TeXitoi
Tuesday Oct 05, 2021 at 15:15 GMT

---

I doubt this particular example will panic. Edit: ho, OK, short argument, yep, it'll panic.

There is no easy way of checking you use the same name several times. Basic case is double, bit when you add flatten and friends, it can become a nightmare. I don't want complicated code to do compile time check that would be catch in all case with a basic test. Now, if the code is really trivial, why not, but I doubt.

[pksunkara]

This is panic during development runtime now. But looks the issue is talking about compile time.

[epage]

Yes. As TeXitoi says, we can do this in the trivial case though it won't scale to complex caes (flatten).

For the trivial case, the main complication is how we currently have the code structured. We take the raw attributes and convert them straight to Methods.

Our options for implementing this:

• Select specific Methods that we check for duplicates across fields. This seems more straightforward but is more brittle.
• Preserve all of the ClapAttrs we parsed and compare those. Requires some restructuring of the code but less brittle.

[epage]

Note: we have #2740 which is about supporting all of the debug assets at compile time.

[martinvonz]

A workaround seems to be to use clap_complete. That seems to catch the cases I've cared about. I don't know what the complication caused by flatten is, but it seems feasible to at least catch the things that generation of command-line-completion scripts will catch, right?

[epage]

Could you expand on how clap_complete is catching these cases? It seems like it only be able to report them at runtime as well, and not compile time.

We do recommend creating tests out of the runtime checks which will catch it during development and will check every subcommand possible, rather than what was run by the user. See https://github.com/clap-rs/clap/blob/v3.1.6/examples/tutorial_builder/README.md#tips

[martinvonz]

Ah, sorry, I was unclear. What I meant is that I used clap_complete at runtime to catch all (AFAICT) errors instead of having to test-run each command and subcommand to see if its definition is valid. So it seems debug_assert() is a better way of doing that then (thanks!). I guess what I (and @typetetris?) were expecting was for that to run at compile-time instead.

[epage]

I guess what I (and @typetetris?) were expecting was for that to run at compile-time instead.

Yes, I understand that but that increases our maintenance burden because we have to support and test the checks at both compile time and runtime. We also would need to detect and limit what checks we run when the user #[clap(flatten)]s one struct into another (example) because we only have knowledge of the struct we are currently processing. I'm also concerned that having different behavior for with and without flatten will confuse users.

[martinvonz]

Makes sense. I understand the issue with flatten now. It's also much less of a concern to me now that I have added that debug_assert() test case you suggested.