Default value should not be validated for required parameter
gwenn opened this issue · 4 comments
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
jcommander/src/main/java/com/beust/jcommander/ParameterDescription.java
Lines 152 to 159 in 748ddba
@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.
@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.
@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. :-)