Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 1848 #2030

Merged
merged 2 commits into from
May 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -12994,7 +12994,7 @@ void validate(CommandLine commandLine) {

validationResult = matches.isEmpty() ? GroupValidationResult.SUCCESS_ABSENT : GroupValidationResult.SUCCESS_PRESENT;
for (ArgGroupSpec missing : unmatchedSubgroups) {
if (missing.validate() && missing.multiplicity().min > 0) {
if (missing.validate() && missing.multiplicity().min > 0 && containsRequiredOptionsOrSubgroups(missing)) {
int presentCount = 0;
boolean haveMissing = true;
boolean someButNotAllSpecified = false;
Expand Down Expand Up @@ -13024,6 +13024,36 @@ void validate(CommandLine commandLine) {
}
}

private boolean containsRequiredOptionsOrSubgroups(ArgGroupSpec argGroupSpec) {
return containsRequiredOptionsOrParameters(argGroupSpec) || containsRequiredSubgroups(argGroupSpec);
}

private boolean containsRequiredOptionsOrParameters(ArgGroupSpec argGroupSpec) {
for ( OptionSpec option : argGroupSpec.options() ) {
if ( option.required() ) { return true; }
}
for ( PositionalParamSpec param : argGroupSpec.positionalParameters() ){
if( param.required() ){return true;}
}
return false;
}

private boolean containsRequiredSubgroups(ArgGroupSpec argGroupSpec) {
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);
}
return result && containsRequiredOptionsOrParameters(subgroup);
} else {
return containsRequiredOptionsOrSubgroups(subgroup);
}
}
return false;
}

private void failGroupMultiplicityExceeded(List<ParseResult.GroupMatch> groupMatches, CommandLine commandLine) {
Map<ArgGroupSpec, List<List<ParseResult.GroupMatch>>> matchesPerGroup = new LinkedHashMap<ArgGroupSpec, List<List<GroupMatch>>>();
String msg = "";
Expand Down
47 changes: 47 additions & 0 deletions src/test/java/picocli/DefaultProviderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.junit.rules.TestRule;
import picocli.CommandLine.Command;
import picocli.CommandLine.IDefaultValueProvider;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Model.ArgSpec;
import picocli.CommandLine.Option;
import picocli.CommandLine.Parameters;
Expand Down Expand Up @@ -436,4 +437,50 @@ class App {
assertEquals(123, app1.a);
}

/**
* Tests issue 1848 https://github.com/remkop/picocli/issues/1848
* Test to ensure that ArgGroups with a multiplicity of 1, with a required option, and with a default value
* provider, will properly show that required option as required, rather than optional.
* */
@Test
public void testIssue1848ArgGroupWithRequiredOptionWithDefaultValueProvider() {

@Command(name = "issue1848Command", defaultValueProvider = Issue1848CommandDefaultProvider.class)
class App {
@ArgGroup(exclusive = false, multiplicity = "1", order = 1)
public Issue1848CommandConfigOptions issue1848CommandConfigOptions;

@Option(names = {"--opt1"})
private String opt1;
}
String helpOutput = new CommandLine(new App()).getHelp().fullSynopsis();
assertTrue(helpOutput.contains("--opt2=<opt2>")); // Check that "--opt2=<opt2>" exists.
assertFalse(helpOutput.contains("[--opt2=<opt2>]")); // But make sure it's not surrounded by square brackets.
}

class Issue1848CommandConfigOptions {
@Option(names = {"--opt2"}, required = true, order = 1)
private String opt2;

@Option(names = {"--opt3"}, required = false, order = 2)
private String opt3;
}

static class Issue1848CommandDefaultProvider implements CommandLine.IDefaultValueProvider {
@Override
public String defaultValue(CommandLine.Model.ArgSpec argSpec) throws Exception {
// Commenting out for now as I'm unsure if it's expected behavior for default values supplied to a required
// option, should result in that option's help/usage information indicating that the option is not required.
/*
if (argSpec.isOption()) {
CommandLine.Model.OptionSpec option = (CommandLine.Model.OptionSpec) argSpec;
if ("--url".equals(option.longestName())) {
return "https://localhost:8080";
}
}
*/
return null;
}
}

}