rvesse / airline

Java annotation-based framework for parsing Git like command line structures with deep extensibility
https://rvesse.github.io/airline/
Apache License 2.0
128 stars 20 forks source link

Options with non-zero arity and no value supplied can consume other option names instead of producing an error #110

Closed gmseed closed 3 years ago

gmseed commented 3 years ago

I have the code:

@Option(name = { "-h", "--help"}, 
        description = "help",
        arity=0)
@Unrestricted
private String help;

@Option(name = { "-o", "--open"},
        description = "open",
        arity=1) // arity=1 ensuring we have a file path/file for -open
@Unrestricted
private String open;

@Option(name = { "-d", "--debug"},
        description = "debug",
        arity=0)
@Unrestricted
private String devdebug;

If I run the cmdline "--help --debug --open" I get the expected error:

Parser error: Required values for option 'open' not provided

However, if I run "--help --open --debug" I get NO error. Why is that?

rvesse commented 3 years ago

Most of the default option parsers are greedy and just attempt to consume the number of values required by the arity of the option without any real inspection of those values. So more specifically to your use case it doesn't try to disambiguate between the next token being a value vs it being another option.

So in your first case it sees --open and tries to parse a value for it but finds none thus you receive an error

In the second case it sees --open so grabs the next value which is --debug so your open option gets the value --debug and you don't receive an error

rvesse commented 3 years ago

But certainly this could be handled better, the default option parsers could check whether the next value is actually an option name and stop parsing further values if it is which should then generate the correct error in both cases

rvesse commented 3 years ago

So on thinking on this more I thought that the option parsers were supposed to be smart enough to check if the next token was an option and if so not treat it as a value. However it looks like one of the default parsers special cases the logic for arity = 1 to just consume the next token without that check.

So this is definitely a bug that wants fixing and I'll try to get this into the next release

rvesse commented 3 years ago

Changed needed:

rvesse commented 3 years ago

Should now be fully fixed on master, will push out a new bug-fix release shortly

rvesse commented 3 years ago

Fixed in 2.8.2 which is now released. It may take a couple of hours for artifacts to show up on Maven Central