From b287f782c419de1e55ef56d4547e0fd67b13f215 Mon Sep 17 00:00:00 2001 From: Remko Popma Date: Sun, 11 Feb 2018 22:19:02 +0900 Subject: [PATCH] #285 Vararg positional parameters should not consume options (cherry picked from commit e17afee) Closes #285 --- RELEASE-NOTES.md | 39 +++++++++++ src/main/java/picocli/CommandLine.java | 27 ++++--- .../java/picocli/CommandLineArityTest.java | 70 ++++++++++++++----- 3 files changed, 111 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 101156409..30ab43710 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,5 +1,44 @@ # picocli Release Notes +# 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/). + +## 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) + +## New and noteworthy + +This is a bugfix release and does not include any new features. + +## 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. + +## Fixed issues + +- [#285] Bugfix: Vararg positional parameters should not consume options. + +## Deprecations + +This release has no additional deprecations. + +## Potential breaking changes + +This release has no breaking changes. + + # 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 index bb91af483..c0aad94d5 100644 --- 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 ar private void parse(List parsedCommands, Stack 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 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 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. + *

+ * 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.

*/ + 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 index 6dd6d67b2..e7bfc7be1 100644 --- 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 parameters; @Option(names = "-o") List 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..* () 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 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 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 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 params; + } + Cmd cmd = CommandLine.populateCommand(new Cmd(), "foo", "--", "--", "--"); + assertEquals(Arrays.asList("foo", "--", "--"), cmd.params); + } }