#285 Vararg positional parameters should not consume options

(cherry picked from commit e17afee) Closes #285
remkop · Feb 11, 2018 · b287f78 · b287f78
1 parent cdee277
commit b287f78
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 25 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -1,5 +1,44 @@
 # picocli Release Notes
 
+# <a name="2.3.0"></a> Picocli 2.3.0
+The picocli community is pleased to announce picocli 2.3.0.
+
+This is a bugfix release.
+
+
+This is the twentieth public release.
+Picocli follows [semantic versioning](http://semver.org/).
+
+## <a name="2.3.0-toc"></a> Table of Contents
+
+* [New and noteworthy](#2.3.0-new)
+* [Promoted features](#2.3.0-promoted)
+* [Fixed issues](#2.3.0-fixes)
+* [Deprecations](#2.3.0-deprecated)
+* [Potential breaking changes](#2.3.0-breaking-changes)
+
+## <a name="2.3.0-new"></a> New and noteworthy
+
+This is a bugfix release and does not include any new features.
+
+## <a name="2.3.0-promoted"></a> Promoted features
+Promoted features are features that were incubating in previous versions of picocli but are now supported and subject to backwards compatibility. 
+
+No features have been promoted in this picocli release.
+
+## <a name="2.3.0-fixes"></a> Fixed issues
+
+- [#285] Bugfix: Vararg positional parameters should not consume options.
+
+## <a name="2.3.0-deprecated"></a> Deprecations
+
+This release has no additional deprecations.
+
+## <a name="2.3.0-breaking-changes"></a> Potential breaking changes
+
+This release has no breaking changes.
+
+
 # <a name="2.2.2"></a> Picocli 2.2.2
 The picocli community is pleased to announce picocli 2.2.2.
 

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -2028,6 +2028,7 @@ private class Interpreter {
         private boolean isHelpRequested;
         private String separator = Help.DEFAULT_SEPARATOR;
         private int position;
+        private boolean endOfOptions;
 
         Interpreter(Object command, IFactory factory) {
             this.command                 = Assert.notNull(command, "command");
@@ -2237,6 +2238,8 @@ private void expandValidArgumentFile(String fileName, File file, List<String> ar
 
         private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack, String[] originalArgs) {
             // first reset any state in case this CommandLine instance is being reused
+            position = 0;
+            endOfOptions = false;
             isHelpRequested = false;
             CommandLine.this.versionHelpRequested = false;
             CommandLine.this.usageHelpRequested = false;
@@ -2293,6 +2296,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
                 }
@@ -2588,10 +2592,8 @@ private void consumeMapArguments(Field field,
             }
             // now process the varargs if any
             for (int i = arity.min; i < arity.max && !args.isEmpty(); i++) {
-                if (!field.isAnnotationPresent(Parameters.class)) {
-                    if (commands.containsKey(args.peek()) || isOption(args.peek())) {
-                        return;
-                    }
+                if (!varargCanConsumeNextValue(field, args.peek())) {
+                    break;
                 }
                 consumeOneMapArgument(field, arity, args, classes, keyConverter, valueConverter, result, i, argDescription);
             }
@@ -2703,10 +2705,8 @@ private List<Object> consumeArguments(Field field,
             }
             // now process the varargs if any
             for (int i = arity.min; i < arity.max && !args.isEmpty(); i++) {
-                if (annotation != Parameters.class) { // for vararg Options, we stop if we encounter '--', a command, or another option
-                    if (commands.containsKey(args.peek()) || isOption(args.peek())) {
-                        break;
-                    }
+                if (!varargCanConsumeNextValue(field, args.peek())) {
+                    break;
                 }
                 consumeOneArgument(field, arity, args, type, result, i, originalSize, argDescription);
             }
@@ -2748,7 +2748,16 @@ private String splitRegex(Field field) {
         }
         private String[] split(String value, Field field) {
             String regex = splitRegex(field);
-            return regex.length() == 0 ? new String[] {value} : value.split(regex);
+            return regex.length() == 0 ? new String[]{value} : value.split(regex);
+        }
+        /** 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(Field field, String nextValue) {
+            if (endOfOptions && field.isAnnotationPresent(Parameters.class)) { return true; }
+            boolean isCommand = commands.containsKey(nextValue);
+            return !isCommand && !isOption(nextValue);
         }
 
         /**

diff --git a/src/test/java/picocli/CommandLineArityTest.java b/src/test/java/picocli/CommandLineArityTest.java
@@ -15,23 +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.MissingParameterException;
-import picocli.CommandLine.Option;
-import picocli.CommandLine.Parameters;
-import picocli.CommandLine.Range;
-import picocli.CommandLine.UnmatchedArgumentException;
-
 import static org.junit.Assert.*;
 
 public class CommandLineArityTest {
@@ -842,18 +836,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");
@@ -911,4 +905,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);
+    }
 }