Long boolean options with arity 0 still allow parameters.

[zegelin]

Given a option definition:

@CommandLine.Option(names = {"--no-global-labels"}, arity = "0")
public void setNoGlobalLabels(final boolean noGlobalLabels) {}

This still allows --no-global-labels=false.

For non-boolean long options arity = "0" probably doesn't make sense. But for options like this it'd be nice to have picocli enforce the arity rather than have to check the boolean parameter and throw an exception if its false.

[remkop]

I can see this is inconsistent, thanks for raising it. I’ll take a look.

[remkop]

I assumed that people would only specify arity for boolean options to optionally allow a parameter (as in arity = "0..1"), I did not foresee your use case of strict validation to prevent end users from specifying a boolean value.

Here is what currently happens:

• During the construction stage, all options are assigned a non-null arity. If omitted, the arity is derived from the type.
• For boolean types, the derived arity is zero.
• During parsing, if the parser finds a value attached to the option (as in --no-global-labels=false), picocli concludes that the "actual arity" is arity="1".

At the moment, picocli does not distinguish between an omitted arity and an explicitly specified arity = "0" for boolean options.

Still looking at how to fix this.

[remkop]

I have a potential solution that involves keeping track of whether the arity was explicitly specified in the option definition.

This test now passes:

@Test
public void testArityZeroForBooleanOption() {
    class App {
        @Option(names = "--explicit", arity = "0") boolean explicit;
        @Option(names = "--implicit") boolean implicit;
    }
    try {
        new CommandLine(new App()).parseArgs("--implicit=false --explicit=false".split(" "));
        fail("--explicit option should not accept parameters");
    } catch (ParameterException ex) {
        assertEquals("option '--explicit' (<explicit>) should be specified without 'false' parameter", ex.getMessage());
    }
}

Still checking for regressions...

[remkop]

Fixed in master.

Is this a critical issue for you? (That is, do you need a 3.6.2 release for this, or can it wait a few weeks for the 3.7 release?)

[zegelin]

@remkop Thanks for the quick-fix. I can wait till 3.7 since I have interm a work-around (throw if value = false).