Default value should not be validated for required parameter

[gwenn]

Minimal example:

import com.beust.jcommander.IValueValidator;
import com.beust.jcommander.JCommander;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.ParameterException;

public class JC {
    @Parameter(names = "-shift", required = true, validateValueWith = StrictlyPositiveInteger.class)
    private int shift = 1; // FIXME: we must not need to specify a default value compatible with StrictlyPositiveInteger when required is true

    public static void main(String[] args) {
        JCommander.newBuilder()
            .addObject(new JC())
            .build()
            .parse(args);
    }

    public static class StrictlyPositiveInteger implements IValueValidator<Integer> {
        @Override
        public void validate(String name, Integer i) throws ParameterException {
            if (i <= 0) {
                throw new ParameterException("Parameter " + name
                    + " should be strictly positive (found " + i + ")");
            }
        }
    }
}

If we remove the default meaningless (because parameter is required) value of shift (private int shift;), JCommander throws:

Exception in thread "main" com.beust.jcommander.ParameterException: Parameter -shift should be strictly positive (found 0)
    at JC$StrictlyPositiveInteger.validate(JC.java:21)
    at JC$StrictlyPositiveInteger.validate(JC.java:17)
    at com.beust.jcommander.ParameterDescription.validateValueParameter(ParameterDescription.java:364)
    at com.beust.jcommander.ParameterDescription.validateValueParameter(ParameterDescription.java:353)
    at com.beust.jcommander.ParameterDescription.validateDefaultValues(ParameterDescription.java:164)
    at com.beust.jcommander.ParameterDescription.init(ParameterDescription.java:157)
    at com.beust.jcommander.ParameterDescription.<init>(ParameterDescription.java:65)
    at com.beust.jcommander.JCommander.addDescription(JCommander.java:631)
    at com.beust.jcommander.JCommander.createDescriptions(JCommander.java:601)
    at com.beust.jcommander.JCommander.parse(JCommander.java:361)
    at com.beust.jcommander.JCommander.parse(JCommander.java:342)
    at JC.main(JC.java:14)

See https://github.com/cbeust/jcommander/blob/748ddba20a28956332b413eaa99d7e8b5422104d/src/main/java/com/beust/jcommander/ParameterDescription.java#L152-L159

[mkarg]

@cbeust Chime in if needed, but I personally do neither see that this is a bug nor that it could get solved. The reproducer deliberately is using int instead of Integer, so the default is 0, not null. Hence defaultObject is not null, but an Integer instance holding the value 0. Hence it is correct that JCommander applies the validator.

@gwenn There is an implied default provided by the Java language itself, even if the application programmer does not explicitly provide a default value (try to find out whether there is no default or whether the default is 0 for an int variable in Java, then you will understand this explanation). There is a simple workaround: Replace int by Integer and your code will work just fine. That's why Integer exists in Java, as it allows to tell between the value 0 and "having no value" (null). JCommander cannot and will not change the way Java deals with defaults.

[gwenn]

@mkarg But for a required parameter, using a non-nullable type (int instead of Integer) seems wise, no ? And I don't want to change java defaults, I want JCommander to ignore any default value for required parameter because default value is meaningless for required parameter.

[mkarg]

@gwenn I do not see why int should be more wise than Integer, actually: You could instantly workaround your problem that way, which is very wise. ;-) Implementing the requested change just for required values is acceptable on the other hand. Feel free to post a PR, I would be happy to review it. :-)

[gwenn]

@mkarg see #567 (I am not sure that unit test should be added to the 1500 lines JCommanderTest class)