Skip to content

Commit

Permalink
Merge branch 'main' into dependabot/github_actions/github/codeql-acti…
Browse files Browse the repository at this point in the history
…on-2.3.5
  • Loading branch information
remkop committed Jun 2, 2023
2 parents f31f370 + 5984589 commit eb9e744
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 4 deletions.
2 changes: 1 addition & 1 deletion dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ext {
jansiVersion = "2.4.0"
jline2Version = "2.14.6"
jline3Version = "3.23.0"
junitDepVersion = "4.11.20120805.1225"
junitDepVersion = "4.11"
junitVersion = "4.13.2"
log4j2Version = "2.20.0"
springBootVersion = "2.7.10" // Spring Boot 3.0 requires Java 17 as a minimum version
Expand Down
45 changes: 42 additions & 3 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -9526,6 +9526,7 @@ abstract static class Builder<T extends Builder<T>> {
setter = original.setter;
scope = original.scope;
scopeType = original.scopeType;
mapFallbackValue = original.mapFallbackValue;
originalDefaultValue = original.originalDefaultValue;
originalMapFallbackValue = original.originalMapFallbackValue;
}
Expand Down Expand Up @@ -12994,7 +12995,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 +13025,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 Expand Up @@ -15015,7 +15046,15 @@ static Charset getStderrEncoding() {
static Charset charsetForName(String encoding) {
if (encoding != null) {
if ("cp65001".equalsIgnoreCase(encoding)) { encoding = "UTF-8"; } // #1474 MS Windows uses code page 65001 for UTF8
return Charset.forName(encoding);
try {
return Charset.forName(encoding);
} catch (Exception e) {
// fallback to default charset if the requested encoding is not available
final Charset defaultCharset = Charset.defaultCharset();
CommandLine.tracer().info("The %s encoding in not available, falling back to %s", encoding,
defaultCharset.name());
return defaultCharset;
}
}
return Charset.defaultCharset();
}
Expand Down Expand Up @@ -18675,7 +18714,7 @@ private List<String> prefixCommandName(List<String> suggestions)
if(commandName == null || commandName.trim().isEmpty()) { return suggestions; }
List<String> prefixedSuggestions = new ArrayList<String>();
for (String s : suggestions) {
prefixedSuggestions.add(commandName + " " + s);
prefixedSuggestions.add(commandName + " " + s);
}
return prefixedSuggestions;
}
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;
}
}

}
55 changes: 55 additions & 0 deletions src/test/java/picocli/Issue2029.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package picocli;

import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.contrib.java.lang.system.SystemOutRule;
import org.junit.rules.TestRule;

import static org.junit.Assert.assertEquals;

public class Issue2029 {
@Rule
public final TestRule restoreSystemProperties = new RestoreSystemProperties();

@Rule
public final SystemOutRule systemOutRule = new SystemOutRule().enableLog();

@Rule
public final SystemErrRule systemErrRule = new SystemErrRule().enableLog();

@CommandLine.Command(name = "test")
static class TestCommand implements Runnable {

@CommandLine.Parameters
String text;

@CommandLine.Spec
CommandLine.Model.CommandSpec spec;

//@Override
public void run() {
spec.commandLine().getOut().print(text);
spec.commandLine().getOut().flush();
spec.commandLine().getErr().print(text);
spec.commandLine().getErr().flush();
}
}

@Test
public void invalidEncodingFallsbackToDefaultEncoding() {
resetLogs();
System.setProperty("sun.stdout.encoding", "cp0");
System.setProperty("sun.stdout.encoding", "cp0");

assertEquals(CommandLine.ExitCode.OK, new CommandLine(new Issue1320.TestCommand()).execute("test"));
assertEquals("test", systemOutRule.getLog());
assertEquals("test", systemErrRule.getLog());
}

private void resetLogs() {
systemOutRule.clearLog();
systemErrRule.clearLog();
}
}
53 changes: 53 additions & 0 deletions src/test/java/picocli/MapOptionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
import org.junit.contrib.java.lang.system.ProvideSystemProperty;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.rules.TestRule;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
import picocli.CommandLine.ParameterException;
import picocli.CommandLine.Parameters;

import java.util.Map;

import static org.junit.Assert.*;
import static picocli.CommandLine.ScopeType.INHERIT;

/**
* Verifies https://github.com/remkop/picocli/issues/1214
Expand Down Expand Up @@ -75,6 +77,40 @@ class App {
assertEquals(null, app.map.get("key"));
}

@Test
public void testInheritedOptionMapFallbackValueEmptyString() {
class App {
@Option(names = "-D", mapFallbackValue = "", scope = INHERIT) Map<String, String> map;
}
@Command(name = "sub")
class Sub {
}
App app = new App();
Sub sub = new Sub();
new CommandLine(app)
.addSubcommand(sub)
.parseArgs("sub", "-Dkey");
assertEquals(1, app.map.size());
assertEquals("", app.map.get("key"));
}

@Test
public void testInheritedOptionMapFallbackValueNull() {
class App {
@Option(names = "-D", mapFallbackValue = Option.NULL_VALUE, scope = INHERIT) Map<String, String> map;
}
@Command(name = "sub")
class Sub {
}
App app = new App();
Sub sub = new Sub();
new CommandLine(app)
.addSubcommand(sub)
.parseArgs("sub", "-Dkey");
assertEquals(1, app.map.size());
assertEquals(null, app.map.get("key"));
}

@Test
public void testPositionalMapFallbackValueNull() {
class App {
Expand All @@ -85,6 +121,23 @@ class App {
assertEquals(null, app.map.get("key"));
}

@Test
public void testInheritedPositionalMapFallbackValueNull() {
class App {
@Parameters(mapFallbackValue = Option.NULL_VALUE, scope = INHERIT) Map<String, String> map;
}
@Command(name = "sub")
class Sub {
}
App app = new App();
Sub sub = new Sub();
new CommandLine(app)
.addSubcommand(sub)
.parseArgs("sub", "key");
assertEquals(1, app.map.size());
assertEquals(null, app.map.get("key"));
}

@Test
public void testMapFallbackValueEmptyStringMultiple() {
class App {
Expand Down

0 comments on commit eb9e744

Please sign in to comment.