Skip to content

Commit

Permalink
[#1055] bugfix: options should not be accepted as option-parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
remkop committed Jun 18, 2020
1 parent 6308c63 commit 27d040d
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 13 deletions.
8 changes: 7 additions & 1 deletion src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -12829,13 +12829,19 @@ private int applyValueToSingleValuedField(ArgSpec argSpec,
Set<ArgSpec> initialized,
String argDescription) throws Exception {
boolean noMoreValues = args.isEmpty();
String value = args.isEmpty() ? null : args.pop();
String value = noMoreValues ? null : args.pop();
String quotedValue = value;
if (commandSpec.parser().trimQuotes() && !alreadyUnquoted) {value = unquote(value);}
Range arity = argSpec.arity().isUnspecified ? derivedArity : argSpec.arity(); // #509
if (arity.max == 0 && !arity.isUnspecified && lookBehind == LookBehind.ATTACHED_WITH_SEPARATOR) { // #509
throw new MaxValuesExceededException(CommandLine.this, optionDescription("", argSpec, 0) +
" should be specified without '" + value + "' parameter");
}
if (arity.min > 0) {
args.push(quotedValue);
assertNoMissingMandatoryParameter(argSpec, args, 0, arity);
args.pop();
}
int consumed = arity.min; // the number or args we need to consume

String actualValue = value;
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/picocli/ArityTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ class App {
CommandLine.populateCommand(new App(), "-Long", "-boolean");
fail("should fail");
} catch (CommandLine.ParameterException ex) {
assertEquals("Invalid value for option '-Long': '-boolean' is not a long", ex.getMessage());
assertEquals("Expected parameter for option '-Long' but found '-boolean'", ex.getMessage());
}
}
/** see <a href="https://github.com/remkop/picocli/issues/279">issue #279</a> */
Expand Down
16 changes: 12 additions & 4 deletions src/test/java/picocli/CommandLineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -919,16 +919,24 @@ public void testOptionsMixedWithParameters() {
}
@Test
public void testShortOptionsWithSeparatorButNoValueAssignsEmptyStringEvenIfNotLast() {
CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-ro= -v".split(" "));
verifyCompact(compact, false, true, "-v", null);
try {
CommandLine.populateCommand(new CompactFields(), "-ro= -v".split(" "));
fail("Expected exception");
} catch (MissingParameterException ex) {
assertEquals("Expected parameter for option '-o' but found '-v'", ex.getMessage());
}
}
@Test
public void testShortOptionsWithColonSeparatorButNoValueAssignsEmptyStringEvenIfNotLast() {
CompactFields compact = new CompactFields();
CommandLine cmd = new CommandLine(compact);
cmd.setSeparator(":");
cmd.parseArgs("-ro: -v".split(" "));
verifyCompact(compact, false, true, "-v", null);
try {
cmd.parseArgs("-ro: -v".split(" "));
fail("Expected exception");
} catch (MissingParameterException ex) {
assertEquals("Expected parameter for option '-o' but found '-v'", ex.getMessage());
}
}
@Test
public void testShortOptionsWithSeparatorButNoValueFailsIfValueRequired() {
Expand Down
17 changes: 14 additions & 3 deletions src/test/java/picocli/FallbackTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.junit.rules.TestRule;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
import picocli.CommandLine.ParameterException;
import picocli.CommandLine.Parameters;

import static org.junit.Assert.*;
Expand Down Expand Up @@ -74,14 +75,24 @@ public void testQuotedSubcommandAssignedToOptionWithFallback() {

@Test
public void testSubcommandAssignedToOption() {
class MyBadParams implements CommandLine.IParameterExceptionHandler {
ParameterException ex;
public int handleParseException(ParameterException ex, String[] args) throws Exception {
this.ex = ex;
return 2;
}
}
MyBadParams handler = new MyBadParams();
MyCommand cmd = new MyCommand();
int exitCode = new CommandLine(cmd)
.setUnmatchedOptionsArePositionalParams(true)
.setParameterExceptionHandler(handler)
.execute("-x run app a".split(" "));

assertEquals("run", cmd.arity1);
assertEquals(0, exitCode);
assertEquals(2, exitCode);
assertEquals(null, cmd.arity1);
assertNull(cmd.subApp);
assertArrayEquals(new String[]{"app", "a"}, cmd.params);
assertNotNull(handler.ex);
assertEquals("Expected parameter for option '-x' but found 'run'", handler.ex.getMessage());
}
}
2 changes: 1 addition & 1 deletion src/test/java/picocli/LenientParsingTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ class App {
cmd.getCommandSpec().parser().collectErrors(true);
cmd.parseArgs("-Long", "-boolean");
assertEquals(1, cmd.getParseResult().errors().size());
assertEquals("Invalid value for option '-Long': '-boolean' is not a long", cmd.getParseResult().errors().get(0).getMessage());
assertEquals("Expected parameter for option '-Long' but found '-boolean'", cmd.getParseResult().errors().get(0).getMessage());
}
@Test
public void testBooleanOptionsArity0_nFailsIfAttachedParamNotABoolean() { // ignores varargs
Expand Down
4 changes: 1 addition & 3 deletions src/test/java/picocli/UnmatchedOptionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,14 @@ class App {
assertArrayEquals(new String[]{"4", "5"}, app.y);
}

@Ignore
@Test
public void testSingleValueOptionDoesNotConsumeActualOption() {
class App {
@Option(names = "-x") int x;
@Option(names = "-y") String y;
}

//FIXME
expect(new App(), "Missing required parameter for option '-x' (<x>)", MissingParameterException.class, "-y", "-x");
expect(new App(), "Expected parameter for option '-y' but found '-x'", MissingParameterException.class, "-y", "-x");
}

@Test
Expand Down

0 comments on commit 27d040d

Please sign in to comment.