[#233] Bugfix: Parser bug: first argument following clustered options…

… is treated as a positional parameter. Thanks to [mgrossmann](https://github.com/mgrossmann). Closes #233
remkop · Nov 10, 2017 · d808111 · d808111
1 parent f0826b0
commit d808111
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 7 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -33,6 +33,7 @@ No features have been promoted in this picocli release.
 - [#223] New feature: Added `examples` subproject containing running examples.  Thanks to [aadrian](https://github.com/aadrian) and [RobertZenz](https://github.com/RobertZenz).
 - [#68]  Enhancement: Reject private final primitive fields annotated with @Option or @Parameters: because compile-time constants are inlined, updates by picocli to such fields would not be visible to the application.
 - [#230] Enhancement: Support embedded newlines in usage help sections like header or descriptions. Thanks to [ddimtirov](https://github.com/ddimtirov).
+- [#233] Bugfix: Parser bug: first argument following clustered options is treated as a positional parameter. Thanks to [mgrossmann](https://github.com/mgrossmann). 
 
 ## <a name="2.1.0-deprecated"></a> Deprecations
 

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -2220,14 +2220,13 @@ private void processClusteredShortOptions(Collection<Field> required,
                     // arity may be >= 1, or
                     // arity <= 0 && !cluster.startsWith(separator)
                     // e.g., boolean @Option("-v", arity=0, varargs=true); arg "-rvTRUE", remainder cluster="TRUE"
-                    args.push(cluster); // interpret remainder as option parameter (CAUTION: may be empty string!)
-                    if (!args.isEmpty() && args.peek().length() == 0 && !paramAttachedToOption) {
-                        args.pop(); // throw out empty string we get at the end of a group of clustered short options
+                    if (!empty(cluster)) {
+                        args.push(cluster); // interpret remainder as option parameter (CAUTION: may be empty string!)
                     }
                     int argCount = args.size();
                     int consumed = applyOption(field, Option.class, arity, paramAttachedToOption, args, initialized, argDescription);
                     // if cluster was consumed as a parameter or if this field was the last in the cluster we're done; otherwise continue do-while loop
-                    if (args.isEmpty() || args.size() < argCount) {
+                    if (empty(cluster) || args.isEmpty() || args.size() < argCount) {
                         return;
                     }
                     cluster = args.pop();

diff --git a/src/test/java/picocli/CommandLineTest.java b/src/test/java/picocli/CommandLineTest.java
@@ -466,6 +466,9 @@ public void testCompactFieldsAnyOrder() {
         compact = CommandLine.populateCommand(new CompactFields(), "-r -v -oout p1 p2".split(" "));
         verifyCompact(compact, true, true, "out", fileArray("p1", "p2"));
 
+        compact = CommandLine.populateCommand(new CompactFields(), "-rv -o out p1 p2".split(" ")); //#233
+        verifyCompact(compact, true, true, "out", fileArray("p1", "p2"));
+
         compact = CommandLine.populateCommand(new CompactFields(), "-oout -r -v p1 p2".split(" "));
         verifyCompact(compact, true, true, "out", fileArray("p1", "p2"));
 
@@ -523,19 +526,41 @@ public void testOptionsMixedWithParameters() {
     @Test
     public void testShortOptionsWithSeparatorButNoValueAssignsEmptyStringEvenIfNotLast() {
         CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-ro= -v".split(" "));
-        verifyCompact(compact, true, true, "", null);
+        verifyCompact(compact, false, true, "-v", null);
     }
     @Test
     public void testShortOptionsWithColonSeparatorButNoValueAssignsEmptyStringEvenIfNotLast() {
         CompactFields compact = new CompactFields();
         CommandLine cmd = new CommandLine(compact);
         cmd.setSeparator(":");
         cmd.parse("-ro: -v".split(" "));
+        verifyCompact(compact, false, true, "-v", null);
+    }
+    @Test
+    public void testShortOptionsWithSeparatorButNoValueFailsIfValueRequired() {
+        try {
+            CommandLine.populateCommand(new CompactFields(), "-rvo=".split(" "));
+            fail("Expected exception");
+        } catch (ParameterException ex) {
+            assertEquals("Missing required parameter for option '-o' (<outputFile>)", ex.getMessage());
+        }
+    }
+    @Test
+    public void testShortOptionsWithSeparatorButNoValueAssignsQuotedEmptyStringEvenIfNotLast() {
+        CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-ro=\"\" -v".split(" "));
+        verifyCompact(compact, true, true, "", null);
+    }
+    @Test
+    public void testShortOptionsWithColonSeparatorButNoValueAssignsQuotedEmptyStringEvenIfNotLast() {
+        CompactFields compact = new CompactFields();
+        CommandLine cmd = new CommandLine(compact);
+        cmd.setSeparator(":");
+        cmd.parse("-ro:\"\" -v".split(" "));
         verifyCompact(compact, true, true, "", null);
     }
     @Test
-    public void testShortOptionsWithSeparatorButNoValueAssignsEmptyStringIfLast() {
-        CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-rvo=".split(" "));
+    public void testShortOptionsWithSeparatorButNoValueAssignsEmptyQuotedStringIfLast() {
+        CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-rvo=\"\"".split(" "));
         verifyCompact(compact, true, true, "", null);
     }