Skip to content

Commit

Permalink
#285 Vararg positional parameters should not consume options
Browse files Browse the repository at this point in the history
  • Loading branch information
@remkop
remkop committed Feb 11, 2018
1 parent 498d36d commit e17afee
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 20 deletions.
25 changes: 17 additions & 8 deletions src/main/java/picocli/CommandLine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3309,6 +3309,7 @@ private class Interpreter {
private final Map<Class<?>, ITypeConverter<?>> converterRegistry = new HashMap<Class<?>, ITypeConverter<?>>();
private boolean isHelpRequested;
private int position;
private boolean endOfOptions;

Interpreter() { registerBuiltInConverters(); }

Expand Down Expand Up @@ -3440,6 +3441,7 @@ private void expandValidArgumentFile(String fileName, File file, List<String> ar

private void clear() {
position = 0;
endOfOptions = false;
isHelpRequested = false;
CommandLine.this.versionHelpRequested = false;
CommandLine.this.usageHelpRequested = false;
Expand Down Expand Up @@ -3503,6 +3505,7 @@ private void processArguments(List<CommandLine> parsedCommands,
// If found, then interpret the remaining args as positional parameters.
if ("--".equals(arg)) {
tracer.info("Found end-of-options delimiter '--'. Treating remainder as positional parameters.%n");
endOfOptions = true;
processRemainderAsPositionalParameters(required, initialized, args);
return; // we are done
}
Expand Down Expand Up @@ -3803,10 +3806,8 @@ private void consumeMapArguments(ArgSpec<?> argSpec,
}
// now process the varargs if any
for (int i = arity.min; i < arity.max && !args.isEmpty(); i++) {
if (argSpec.isOption()) {
if (commandSpec.subcommands().containsKey(args.peek()) || isOption(args.peek())) {
return;
}
if (!varargCanConsumeNextValue(argSpec, args.peek())) {
break;
}
consumeOneMapArgument(argSpec, args, classes, keyConverter, valueConverter, result, i, argDescription);
}
Expand Down Expand Up @@ -3915,10 +3916,8 @@ private List<Object> consumeArguments(ArgSpec<?> argSpec,
}
// now process the varargs if any
for (int i = arity.min; i < arity.max && !args.isEmpty(); i++) {
if (argSpec.isOption()) { // for vararg Options, we stop if we encounter '--', a command, or another option
if (commandSpec.subcommands().containsKey(args.peek()) || isOption(args.peek())) {
break;
}
if (!varargCanConsumeNextValue(argSpec, args.peek())) {
break;
}
consumeOneArgument(argSpec, arity, args, type, result, i, argDescription);
}
Expand Down Expand Up @@ -3949,6 +3948,16 @@ private int consumeOneArgument(ArgSpec<?> argSpec,
return ++index;
}

/** Returns whether the next argument can be assigned to a vararg option/positional parameter.
* <p>
* Usually, we stop if we encounter '--', a command, or another option.
* However, if end-of-options has been reached, positional parameters may consume all remaining arguments. </p>*/
private boolean varargCanConsumeNextValue(ArgSpec<?> argSpec, String nextValue) {
if (endOfOptions && argSpec.isPositional()) { return true; }
boolean isCommand = commandSpec.subcommands().containsKey(nextValue);
return !isCommand && !isOption(nextValue);
}

/**
* Called when parsing varargs parameters for a multi-value option.
* When an option is encountered, the remainder should not be interpreted as vararg elements.
Expand Down
66 changes: 54 additions & 12 deletions src/test/java/picocli/CommandLineArityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,17 @@
*/
package picocli;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import picocli.CommandLine.*;

import java.io.File;
import java.net.InetAddress;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.TimeUnit;

import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

import picocli.CommandLine.*;

import static org.junit.Assert.*;
import static picocli.HelpTestUtil.setTraceLevel;

Expand Down Expand Up @@ -852,18 +850,18 @@ class NonVarArgArrayParamsArity2 {
}
}
@Test
public void testMixPositionalParamsWithOptions_ParamsUnboundedArity_isGreedy() {
public void testMixPositionalParamsWithOptions_ParamsUnboundedArity() {
class Arg {
@Parameters(arity = "1..*") List<String> parameters;
@Option(names = "-o") List<String> options;
}
Arg result = CommandLine.populateCommand(new Arg(), "-o", "v1", "p1", "p2", "-o", "v2", "p3", "p4");
assertEquals(Arrays.asList("p1", "p2", "-o", "v2", "p3", "p4"), result.parameters);
assertEquals(Arrays.asList("v1"), result.options);
assertEquals(Arrays.asList("p1", "p2", "p3", "p4"), result.parameters);
assertEquals(Arrays.asList("v1", "v2"), result.options);

Arg result2 = CommandLine.populateCommand(new Arg(), "-o", "v1", "p1", "-o", "v2", "p3");
assertEquals(Arrays.asList("p1", "-o", "v2", "p3"), result2.parameters);
assertEquals(Arrays.asList("v1"), result2.options);
assertEquals(Arrays.asList("p1", "p3"), result2.parameters);
assertEquals(Arrays.asList("v1", "v2"), result2.options);

try {
CommandLine.populateCommand(new Arg(), "-o", "v1", "-o", "v2");
Expand Down Expand Up @@ -921,4 +919,48 @@ class Arg {
assertEquals("positional parameter at index 0..* (<parameters>) requires at least 2 values, but only 1 were specified: [p3]", ex.getMessage());
}
}

@Test
public void test284VarargPositionalShouldNotConsumeOptions() {
class Cmd {
@Option(names = "--alpha") String alpha;
@Parameters(index = "0", arity = "1") String foo;
@Parameters(index = "1..*", arity = "*") List<String> params;
}
Cmd cmd = CommandLine.populateCommand(new Cmd(), "foo", "xx", "--alpha", "--beta");
assertEquals("foo", cmd.foo);
assertEquals("--beta", cmd.alpha);
assertEquals(Arrays.asList("xx"), cmd.params);
}

@Test
public void test284VarargPositionalShouldConsumeOptionsAfterDoubleDash() {
class Cmd {
@Option(names = "--alpha") String alpha;
@Parameters(index = "0", arity = "1") String foo;
@Parameters(index = "1..*", arity = "*") List<String> params;
}
Cmd cmd = CommandLine.populateCommand(new Cmd(), "foo", "--", "xx", "--alpha", "--beta");
assertEquals("foo", cmd.foo);
assertEquals(null, cmd.alpha);
assertEquals(Arrays.asList("xx", "--alpha", "--beta"), cmd.params);
}

@Test
public void testPositionalShouldCaptureDoubleDashAfterDoubleDash() {
class Cmd {
@Parameters List<String> params;
}
Cmd cmd = CommandLine.populateCommand(new Cmd(), "foo", "--", "--", "--");
assertEquals(Arrays.asList("foo", "--", "--"), cmd.params);
}

@Test
public void testVarargPositionalShouldCaptureDoubleDashAfterDoubleDash() {
class Cmd {
@Parameters(index = "0..*", arity = "*") List<String> params;
}
Cmd cmd = CommandLine.populateCommand(new Cmd(), "foo", "--", "--", "--");
assertEquals(Arrays.asList("foo", "--", "--"), cmd.params);
}
}

0 comments on commit e17afee

Please sign in to comment.