clap icon indicating copy to clipboard operation
clap copied to clipboard

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

Open petrochenkov opened this issue 2 years ago • 7 comments

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

petrochenkov avatar Mar 28 '22 15:03 petrochenkov

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();
}

epage avatar Mar 28 '22 15:03 epage

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.

petrochenkov avatar Mar 28 '22 15:03 petrochenkov

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.

epage avatar Mar 28 '22 16:03 epage

@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

evgeniy-terekhin avatar May 07 '22 14:05 evgeniy-terekhin

@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.

epage avatar May 08 '22 01:05 epage

@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.

evgeniy-terekhin avatar May 08 '22 06:05 evgeniy-terekhin

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

epage avatar May 09 '22 14:05 epage