Skip to content

Commit

Permalink
[#2059] ArgGroup fix regression and refine solution introduced in [#1848
Browse files Browse the repository at this point in the history
  • Loading branch information
remkop committed Aug 26, 2023
1 parent 9f6ef2a commit a63975d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 13 deletions.
32 changes: 19 additions & 13 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -8789,6 +8789,7 @@ public abstract static class ArgSpec {
private Object initialValue;
private final boolean hasInitialValue;
private InitialValueState initialValueState;
protected boolean valueIsDefaultValue;
protected final IAnnotatedElement annotatedElement;
private final IGetter getter;
private final ISetter setter;
Expand Down Expand Up @@ -12998,7 +12999,8 @@ void validate(CommandLine commandLine) {

validationResult = matches.isEmpty() ? GroupValidationResult.SUCCESS_ABSENT : GroupValidationResult.SUCCESS_PRESENT;
for (ArgGroupSpec missing : unmatchedSubgroups) {
if (missing.validate() && missing.multiplicity().min > 0 && containsRequiredOptionsOrSubgroups(missing)) {
// error is 1+ occurrence is required, unless all required options have a default
if (missing.validate() && !isGroupEffectivelyOptional(missing)) {
int presentCount = 0;
boolean haveMissing = true;
boolean someButNotAllSpecified = false;
Expand Down Expand Up @@ -13051,27 +13053,28 @@ private boolean isGroupEffectivelyOptional(ArgGroupSpec argGroupSpec) {
return requiredOptionsExistAndAllHaveDefaultValues(argGroupSpec) && !containsRequiredSubgroups(argGroupSpec);
}

private boolean containsRequiredOptionsOrParameters(ArgGroupSpec argGroupSpec) {
for ( OptionSpec option : argGroupSpec.options() ) {
if ( option.required() ) { return true; }
private boolean requiredOptionsExistAndAllHaveDefaultValues(ArgGroupSpec argGroupSpec) {
if (argGroupSpec.requiredArgs().size() == 0) { return false; }
for (OptionSpec option : argGroupSpec.options()) {
if (option.required() && !option.valueIsDefaultValue) { return false; }
}
for ( PositionalParamSpec param : argGroupSpec.positionalParameters() ){
if( param.required() ){return true;}
for (PositionalParamSpec param : argGroupSpec.positionalParameters()) {
if (param.required() && !param.valueIsDefaultValue) { return false; }
}
return false;
return true;
}

private boolean containsRequiredSubgroups(ArgGroupSpec argGroupSpec) {
for ( ArgGroupSpec subgroup : argGroupSpec.subgroups() ) {
if ( subgroup.exclusive() ) {
for (ArgGroupSpec subgroup : argGroupSpec.subgroups()) {
if (subgroup.exclusive()) {
// Only return true if all of the subgroups contain required options or subgroups
boolean result = true;
for ( ArgGroupSpec subsubgroup : subgroup.subgroups() ) {
result &= containsRequiredOptionsOrSubgroups(subsubgroup);
for (ArgGroupSpec subsubgroup : subgroup.subgroups()) {
result &= !isGroupEffectivelyOptional(subsubgroup);
}
return result && containsRequiredOptionsOrParameters(subgroup);
return result && !requiredOptionsExistAndAllHaveDefaultValues(subgroup);
} else {
return containsRequiredOptionsOrSubgroups(subgroup);
return !isGroupEffectivelyOptional(subgroup);
}
}
return false;
Expand Down Expand Up @@ -13710,11 +13713,13 @@ private boolean applyDefault(IDefaultValueProvider defaultValueProvider, ArgSpec
tracer.debug("Applying defaultValue (%s)%s to %s on %s", defaultValue, provider, arg, arg.scopeString());}
Range arity = arg.arity().min(Math.max(1, arg.arity().min));
applyOption(arg, false, LookBehind.SEPARATE, false, arity, stack(defaultValue), new HashSet<ArgSpec>(), arg.toString);
arg.valueIsDefaultValue = true;
} else {
if (arg.typeInfo().isOptional()) {
if (tracer.isDebug()) {
tracer.debug("Applying Optional.empty() to %s on %s", arg, arg.scopeString());}
arg.setValue(getOptionalEmpty());
arg.valueIsDefaultValue = true;
} else if (ArgSpec.UNSPECIFIED.equals(arg.originalDefaultValue)) {
tracer.debug("defaultValue not defined for %s", arg);
return false;
Expand All @@ -13724,6 +13729,7 @@ private boolean applyDefault(IDefaultValueProvider defaultValueProvider, ArgSpec
if (tracer.isDebug()) {
tracer.debug("Applying defaultValue (%s)%s to %s on %s", defaultValue, provider, arg, arg.scopeString());}
arg.setValue(defaultValue);
arg.valueIsDefaultValue = true;
return true;
}
tracer.debug("defaultValue not defined for %s", arg);
Expand Down
62 changes: 62 additions & 0 deletions src/test/java/picocli/ArgGroupTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4377,4 +4377,66 @@ private static class Group {
public void run() {
}
}

@Command(name = "multiplicity")
static class Issue2059Multiplicity {

@ArgGroup(exclusive = false, multiplicity = "1")
RequiredGroup requiredGroup;

static class RequiredGroup {
@Option(names = "--option_one")
private String optionOne;

@Option(names = "--option_two")
private String optionTwo;
}
}

@Test
public void testIssue2059() {
Issue2059Multiplicity obj = new Issue2059Multiplicity();
try {
new CommandLine(obj).parseArgs();
fail("Expected exception");
} catch (ParameterException ok) {
assertEquals("Error: Missing required argument(s): ([--option_one=<optionOne>] [--option_two=<optionTwo>])", ok.getMessage());
}
}

static class Issue947RequireOneOrBothNotNoneGroup {
@Option(names = {"-f", "--feature"}) String feature;
@Option(names = {"-p", "--project"}) String project;
}

@Command
static class Issue947Cmd {
@ArgGroup(multiplicity = "1", exclusive = false)
Issue947RequireOneOrBothNotNoneGroup group;
}

@Test
public void testIssue947RequireOneOrBothNotNone() {
Issue947Cmd ok1 = CommandLine.populateCommand(new Issue947Cmd(), "--project", "Foo");
assertNotNull(ok1.group);
assertEquals("Foo", ok1.group.project);
assertNull(ok1.group.feature);

Issue947Cmd ok2 = CommandLine.populateCommand(new Issue947Cmd(), "--feature", "Bar");
assertNotNull(ok2.group);
assertNull(ok2.group.project);
assertEquals("Bar", ok2.group.feature);

Issue947Cmd ok3 = CommandLine.populateCommand(new Issue947Cmd(), "--project", "Foo", "--feature", "Bar");
assertNotNull(ok3.group);
assertEquals("Foo", ok3.group.project);
assertEquals("Bar", ok3.group.feature);

try {
CommandLine.populateCommand(new Issue947Cmd());
fail("Expected exception");
} catch (ParameterException ok) {
assertEquals("Error: Missing required argument(s): ([-f=<feature>] [-p=<project>])", ok.getMessage());
}
}
}

0 comments on commit a63975d

Please sign in to comment.