clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
14.25k stars 1.04k forks source link

`arg!(--long-arg "foo")` doesn't behave in expected way #3586

Open petrochenkov opened 2 years ago

petrochenkov commented 2 years ago

Please complete the following tasks

Rust Version

rustc 1.54.0

Clap Version

3.1.6

Minimal reproducible code

fn main() {
    let matches = Command::new("my_command").args(&[arg!(--long-option "Description")]).get_matches();
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Some("long")`,
 right: `None`: Short flags should precede long flags

Expected Behaviour

The code either builds an argument with long name --long-option, or errors at compile time.

Additional Context

The issue was found when migrating code from clap 2.

This is a footgun affecting naively ported code that cannot be detected until runtime.

Debug Output

No response

epage commented 2 years ago

This seems to be a limitation of macro-rules that I couldn't find a way around. You need to quote the long option

fn main() {
    let matches = Command::new("my_command").args(&[arg!(--"long-option" "Description")]).get_matches();
}
petrochenkov commented 2 years ago

I found the --"long-option" form later, but the fact that the --long-option form is silently accepted while being incorrect is still a problem.

epage commented 2 years ago

but the fact that the --long-option form is silently accepted while being incorrect is still a problem.

which is why this issue is open but I'm not aware of any good solution.

Also, we have resigned ourselves in general to not being able to turn every error into a compile error but backstop with debug asserts. We encourage people to add a test that calls Command::debug_asserrt() to help catch issues earlier in the development process.

evgeniy-terekhin commented 2 years ago

@epage I guess one solution could be to allow defining short and long flags only in one exact order (-f --flag) But I guess this might and probably will break someone's code

epage commented 2 years ago

@evgeniy-terekhin I think I'm missing something with your suggestion. We do only allow one order the problem is I didn't see a way to enforce that in macro rules without blowing up the number of cases we define and I didn't see a way for that to allow --foo-bar without quotes.

evgeniy-terekhin commented 2 years ago

@epage well I duess I made this suggestion without giving it a proper thought. Maybe converting this macro into a proc macro will enable better compile time errors. I'll have to experiment on this a bit.

epage commented 2 years ago

Maybe converting this macro into a proc macro will enable better compile time errors. I'll have to experiment on this a bit.

Yes, my guess is that will be the only way forward for improving this macro. While that might seem counter to one of the reasons to use the builder API over the derive (macros hurt build times),. this macro could possibly have no dependencies like xflags