From 5035471a103a4969d76ea93456799198f5a67008 Mon Sep 17 00:00:00 2001 From: Remko Popma Date: Fri, 30 Mar 2018 21:15:51 +0900 Subject: [PATCH] [#313] Added method `CommandLine::setMaxArityIsMaxTotalParams` to configure the parser to use `arity` to limit the total number of values accumulated in an option or positional parameter. Also added tests for [#316] lenient mode where annotations are optional when extracting annotations. Closes #313. Closes #316. --- RELEASE-NOTES.md | 2 + docs/index.adoc | 30 +++++++- src/main/java/picocli/CommandLine.java | 68 ++++++++++++++--- .../java/picocli/CommandLineModelTest.java | 76 +++++++++++++++++++ src/test/java/picocli/CommandLineTest.java | 50 +++++++++++- 5 files changed, 213 insertions(+), 13 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index bc6773826..fb0e18287 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -4,6 +4,7 @@ ## Fixed issues +- [#313] Enhancement and API Change: add method `CommandLine::setMaxArityIsMaxTotalParams` to configure the parser to use `arity` to limit the total number of values accumulated in an option or positional parameter. - [#314] Enhancement and API Change: add method `CommandLine::setUsageHelpWidth` and `UsageMessageSpec::width` to set the max usage help message width. - [#316] Enhancement: Support lenient mode where annotations are optional when extracting annotations. @@ -18,6 +19,7 @@ See [3.0.0-alpha-1](https://github.com/remkop/picocli/releases/tag/v3.0.0-alpha- - Constructor `CommandLine.Help.TextTable(Ansi, int...)` is replaced with factory method `CommandLine.Help.TextTable.forColumnWidths(Ansi, int...)`. - Constructor `CommandLine.Help.TextTable(Ansi, Column...)` modifier changed from public to protected. - Added factory method `CommandLine.Help.TextTable.forColumns(Ansi, Column...)`. +- Renamed `CommandLine.MaxValuesforFieldExceededException` to `CommandLine.MaxValuesExceededException`. See [3.0.0-alpha-2](https://github.com/remkop/picocli/releases/tag/v3.0.0-alpha-2#3.0.0-alpha-2-breaking-changes). See [3.0.0-alpha-1](https://github.com/remkop/picocli/releases/tag/v3.0.0-alpha-1#3.0.0-alpha-1-breaking-changes). diff --git a/docs/index.adoc b/docs/index.adoc index 441f11f2d..3c266986d 100644 --- a/docs/index.adoc +++ b/docs/index.adoc @@ -655,6 +655,7 @@ ArityDemo -s A B C -f 1.0 2.0 /file1 /file2 Option `-s` has arity `"1..*"` but instead of consuming all parameters, the `-f` argument is recognized as a separate option. +TIP: It is also possible to <> to use `arity` to limit the total number of values accumulated in an option or positional parameter. === Default Arity If no `arity` is specified, the number of parameters depends on the field's type. @@ -773,7 +774,7 @@ The following command line arguments would be accepted: ---- -== Strict or Lenient Parsing +== Parser Configuration === Too Many Values @@ -791,6 +792,33 @@ Applications can change this by calling `CommandLine.setOverwrittenOptionsAllowe When overwritten options are allowed, the last specified value takes effect (the above input will set the `port` field to `8080`) and a WARN level message is printed to the console. (See <> for how to switch off the warnings.) +=== Max Arity Is Max Total Params + +By default, the <> of an option is the number of arguments _for each occurrence_ of the option. +For example, if option `-a` has `arity = "2"`, then the following is a perfectly valid command: +for each occurrence of the option, two option parameters are specified. +---- + -a 1 2 -a 3 4 -a 5 6 +---- +However, if `CommandLine.setMaxArityIsMaxTotalParams(true)` is called first, the above example would result in a `MaxValuesExceededException` because the total number of values (6) exceeds the arity of 2. + +Additionally, by default, when `maxArityIsMaxTotalParams` is `false`, arity is only applied _before_ the argument is <> into parts, +while when `maxArityIsMaxTotalParams` is set to `true`, validation is applied _after_ a command line argument has been split into parts. + +For example, if we have an option like this: +[source,java] +---- +@Option(name = "-a", arity = "1..2", split = ",") String[] values; +---- +By default, the following input would be a valid command: +---- + -a 1,2,3 +---- +By default, the option arity tells the parser to consume 1 to 2 arguments, and the option was followed by a single parameter, `"1,2,3"`, which is fine. + +However, if `maxArityIsMaxTotalParams` is set to true, the above example would result in a `MaxValuesExceededException` because the argument is split into 3 parts, which exceeds the max arity of 2. + + === Stop At Positional By default, positional parameters can be mixed with options on the command line, but this is not always desirable. From picocli 2.3, applications can call `CommandLine.setStopAtPositional(true)` diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java index 3043b96d9..3268ccb87 100644 --- a/src/main/java/picocli/CommandLine.java +++ b/src/main/java/picocli/CommandLine.java @@ -346,6 +346,49 @@ public CommandLine setPosixClusteredShortOptionsAllowed(boolean newValue) { return this; } + /** Returns whether the parser should throw an error when the total number of values accumulated in an option or + * positional parameter exceeds the max arity. The default is {@code false}. + *

By default, the arity of an option is the number of arguments for each occurrence of the option. + * For example, if option {@code -a} has arity=2, then {@code -a 1 2 -a 3 4 -a 5 6} is a perfectly valid + * command: for each occurrence of the option, two option parameters are specified. + *

+ * However, if {@link ParserSpec#maxArityIsMaxTotalParams() maxArityIsMaxTotalParams} is set to {@code true}, + * the above example would result in a {@link MaxValuesExceededException} because the total number of values (6) exceeds the arity of 2. + *

+ * Additionally, by default (when {@code maxArityIsMaxTotalParams} is {@code false}), arity is only applied before + * the argument is {@linkplain OptionSpec#splitRegex() split} into parts, while if {@code maxArityIsMaxTotalParams} is set + * to {@code true}, validation is applied after a command line argument has been split into parts. + *

+ * For example, we have an option {@code -a} with {@code arity = "1..2"} and {@code splitRegex = ","}, and the user + * specified {@code -a 1,2,3} on the command line. By default, the option arity tells the parser to consume + * 1 to 2 arguments, and the option was followed by a single argument, {@code "1,2,3"}, which is fine. + *

+ * However, if {@code maxArityIsMaxTotalParams} is set to {@code true}, the above example would result in a + * {@code MaxValuesExceededException} because the argument is split into 3 parts, which exceeds the max arity of 2. + *

+ * @return {@code true} if a {@link MaxValuesExceededException} should be thrown when the total number of values + * accumulated in an option or positional parameter exceeds the max arity, {@code false} otherwise + * @since 3.0 */ + public boolean isMaxArityIsMaxTotalParams() { return getCommandSpec().parser().maxArityIsMaxTotalParams(); } + + /** Sets whether the parser should throw an error when the total number of values accumulated in an option or + * positional parameter exceeds the max arity. The default is {@code false}. + *

The specified setting will be registered with this {@code CommandLine} and the full hierarchy of its + * subcommands and nested sub-subcommands at the moment this method is called. Subcommands added + * later will have the default setting. To ensure a setting is applied to all + * subcommands, call the setter last, after adding subcommands.

+ * @param newValue the new setting + * @return this {@code CommandLine} object, to allow method chaining + * @since 3.0 + */ + public CommandLine setMaxArityIsMaxTotalParams(boolean newValue) { + getCommandSpec().parser().maxArityIsMaxTotalParams(newValue); + for (CommandLine command : getCommandSpec().subcommands().values()) { + command.setMaxArityIsMaxTotalParams(newValue); + } + return this; + } + /** Returns whether the parser interprets the first positional parameter as "end of options" so the remaining * arguments are all treated as positional parameters. The default is {@code false}. * @return {@code true} if all values following the first positional parameter should be treated as positional parameters, {@code false} otherwise @@ -3163,6 +3206,7 @@ public static class ParserSpec { private boolean unmatchedArgumentsAllowed = false; private boolean expandAtFiles = true; private boolean posixClusteredShortOptionsAllowed = true; + private boolean maxArityIsMaxTotalParams = false; /** Returns the String to use as the separator between options and option parameters. {@code "="} by default, * initialized from {@link Command#separator()} if defined.*/ @@ -3174,6 +3218,7 @@ public static class ParserSpec { public boolean unmatchedArgumentsAllowed() { return unmatchedArgumentsAllowed; } public boolean expandAtFiles() { return expandAtFiles; } public boolean posixClusteredShortOptionsAllowed() { return posixClusteredShortOptionsAllowed; } + public boolean maxArityIsMaxTotalParams() { return maxArityIsMaxTotalParams; } /** Sets the String to use as the separator between options and option parameters. * @return this ParserSpec for method chaining */ @@ -3184,10 +3229,11 @@ public static class ParserSpec { public ParserSpec unmatchedArgumentsAllowed(boolean unmatchedArgumentsAllowed) { this.unmatchedArgumentsAllowed = unmatchedArgumentsAllowed; return this; } public ParserSpec expandAtFiles(boolean expandAtFiles) { this.expandAtFiles = expandAtFiles; return this; } public ParserSpec posixClusteredShortOptionsAllowed(boolean posixClusteredShortOptionsAllowed) { this.posixClusteredShortOptionsAllowed = posixClusteredShortOptionsAllowed; return this; } + public ParserSpec maxArityIsMaxTotalParams(boolean maxArityIsMaxTotalParams) { this.maxArityIsMaxTotalParams = maxArityIsMaxTotalParams; return this; } void initSeparator(String value) { if (initializable(separator, value, DEFAULT_SEPARATOR)) {separator = value;} } public String toString() { - return String.format("posixClusteredShortOptionsAllowed=%s, stopAtPositional=%s, stopAtUnmatched=%s, separator=%s, overwrittenOptionsAllowed=%s, unmatchedArgumentsAllowed=%s, expandAtFiles=%s", - posixClusteredShortOptionsAllowed, stopAtPositional, stopAtUnmatched, separator, overwrittenOptionsAllowed, unmatchedArgumentsAllowed, expandAtFiles); + return String.format("posixClusteredShortOptionsAllowed=%s, stopAtPositional=%s, stopAtUnmatched=%s, separator=%s, overwrittenOptionsAllowed=%s, unmatchedArgumentsAllowed=%s, expandAtFiles=%s, maxArityIsMaxTotalParams=%s", + posixClusteredShortOptionsAllowed, stopAtPositional, stopAtUnmatched, separator, overwrittenOptionsAllowed, unmatchedArgumentsAllowed, expandAtFiles, maxArityIsMaxTotalParams); } } /** Models the shared attributes of {@link OptionSpec} and {@link PositionalParamSpec}. @@ -4777,6 +4823,7 @@ private int applyValuesToMapField(ArgSpec argSpec, int originalSize = map.size(); consumeMapArguments(argSpec, arity, args, classes, keyConverter, valueConverter, map, argDescription); parseResult.add(argSpec, position); + checkMaxArityExceeded(arity, map.size(), argSpec, argDescription); return map.size() - originalSize; } @@ -4834,12 +4881,11 @@ private void consumeOneMapArgument(ArgSpec argSpec, argSpec.rawStringValues.add(raw); } - private void checkMaxArityExceeded(Range arity, int remainder, ArgSpec argSpec, String[] values) { - if (values.length <= remainder) { return; } - String desc = arity.max == remainder ? "" + remainder : arity + ", remainder=" + remainder; - throw new MaxValuesforFieldExceededException(CommandLine.this, optionDescription("", argSpec, -1) + - " max number of values (" + arity.max + ") exceeded: remainder is " + remainder + " but " + - values.length + " values were specified: " + Arrays.toString(values)); + private void checkMaxArityExceeded(Range arity, int size, ArgSpec argSpec, String argDescription) { + if (!commandSpec.parser().maxArityIsMaxTotalParams()) { return; } + if (size <= arity.max) { return; } + throw new MaxValuesExceededException(CommandLine.this, optionDescription("", argSpec, -1) + + " max number of values (" + arity.max + ") exceeded: " + size + " elements."); } private int applyValuesToArrayField(ArgSpec argSpec, @@ -4867,6 +4913,7 @@ private int applyValuesToArrayField(ArgSpec argSpec, Array.set(array, i, newValues.get(i)); } parseResult.add(argSpec, position); + checkMaxArityExceeded(arity, newValues.size(), argSpec, argDescription); return converted.size(); // return how many args were consumed } @@ -4890,6 +4937,7 @@ private int applyValuesToCollectionField(ArgSpec argSpec, } } parseResult.add(argSpec, position); + checkMaxArityExceeded(arity, collection.size(), argSpec, argDescription); return converted.size(); } @@ -7260,9 +7308,9 @@ public static class UnmatchedArgumentException extends ParameterException { public UnmatchedArgumentException(CommandLine commandLine, List args) { this(commandLine, "Unmatched argument" + (args.size() == 1 ? " " : "s ") + args); } } /** Exception indicating that more values were specified for an option or parameter than its {@link Option#arity() arity} allows. */ - public static class MaxValuesforFieldExceededException extends ParameterException { + public static class MaxValuesExceededException extends ParameterException { private static final long serialVersionUID = 6536145439570100641L; - public MaxValuesforFieldExceededException(CommandLine commandLine, String msg) { super(commandLine, msg); } + public MaxValuesExceededException(CommandLine commandLine, String msg) { super(commandLine, msg); } } /** Exception indicating that an option for a single-value option field has been specified multiple times on the command line. */ public static class OverwrittenOptionException extends ParameterException { diff --git a/src/test/java/picocli/CommandLineModelTest.java b/src/test/java/picocli/CommandLineModelTest.java index 27e916587..7c3f3282b 100644 --- a/src/test/java/picocli/CommandLineModelTest.java +++ b/src/test/java/picocli/CommandLineModelTest.java @@ -906,4 +906,80 @@ public void testMixinStandardHelpOptions_SettingToFalseRemovesHelpOptions() { assertTrue(spec.options().isEmpty()); assertFalse(spec.mixinStandardHelpOptions()); } + + @Test + public void testCommandSpec_forAnnotatedObject_requiresPicocliAnnotation() { + try { + CommandSpec.forAnnotatedObject(new Object()); + fail("Expected error"); + } catch (InitializationException ok) { + assertEquals("java.lang.Object is not a command: it has no @Command, @Option, @Parameters or @Unmatched annotations", ok.getMessage()); + } + } + + @Test + public void testCommandSpec_forAnnotatedObjectLenient_doesNotRequirePicocliAnnotation() { + CommandSpec.forAnnotatedObjectLenient(new Object()); // no error + } + + @Test + public void testCommandSpec_forAnnotatedObjectLenient_returnsEmptyCommandSpec() { + CommandSpec spec = CommandSpec.forAnnotatedObjectLenient(new Object()); + assertTrue(spec.optionsMap().isEmpty()); + assertTrue(spec.posixOptionsMap().isEmpty()); + assertTrue(spec.options().isEmpty()); + assertTrue(spec.positionalParameters().isEmpty()); + assertTrue(spec.unmatchedArgsBindings().isEmpty()); + assertTrue(spec.subcommands().isEmpty()); + assertTrue(spec.mixins().isEmpty()); + assertTrue(spec.requiredArgs().isEmpty()); + assertFalse(spec.mixinStandardHelpOptions()); + assertFalse(spec.helpCommand()); + assertEquals("
", spec.name()); + assertArrayEquals(new String[0], spec.version()); + assertNull(spec.versionProvider()); + } + + @Test + public void testParser_MaxArityIsMaxTotalParams_falseByDefault() { + assertFalse(CommandSpec.create().parser().maxArityIsMaxTotalParams()); + } + + @Test + public void testParser_MaxArityIsMaxTotalParams_singleArguments() { + CommandSpec cmd = CommandSpec.create().addOption(OptionSpec.builder("-x").arity("1..3").build()); + cmd.parser().maxArityIsMaxTotalParams(true); + + ParseResult parseResult = new CommandLine(cmd).parseArgs("-x 1 -x 2 -x 3".split(" ")); + assertEquals(Arrays.asList("1", "2", "3"), parseResult.rawOptionValues('x')); + assertArrayEquals(new String[]{"1", "2", "3"}, parseResult.optionValue('x', (String[]) null)); + + CommandSpec cmd2 = CommandSpec.create().addOption(OptionSpec.builder("-x").arity("1..3").build()); + cmd2.parser().maxArityIsMaxTotalParams(true); + try { + new CommandLine(cmd2).parseArgs("-x 1 -x 2 -x 3 -x 4".split(" ")); + fail("expected exception"); + } catch (MaxValuesExceededException ok) { + assertEquals("option '-x' max number of values (3) exceeded: 4 elements.", ok.getMessage()); + } + } + + @Test + public void testParser_MaxArityIsMaxTotalParams_split() { + CommandSpec cmd = CommandSpec.create().addOption(OptionSpec.builder("-x").arity("1..3").splitRegex(",").build()); + cmd.parser().maxArityIsMaxTotalParams(true); + + ParseResult parseResult = new CommandLine(cmd).parseArgs("-x", "1,2,3"); + assertEquals(Arrays.asList("1,2,3"), parseResult.rawOptionValues('x')); // raw is the original command line argument + assertArrayEquals(new String[]{"1", "2", "3"}, parseResult.optionValue('x', (String[]) null)); + + CommandSpec cmd2 = CommandSpec.create().addOption(OptionSpec.builder("-x").arity("1..3").splitRegex(",").build()); + cmd2.parser().maxArityIsMaxTotalParams(true); + try { + new CommandLine(cmd2).parseArgs("-x", "1,2,3,4"); + fail("expected exception"); + } catch (MaxValuesExceededException ok) { + assertEquals("option '-x' max number of values (3) exceeded: 4 elements.", ok.getMessage()); + } + } } diff --git a/src/test/java/picocli/CommandLineTest.java b/src/test/java/picocli/CommandLineTest.java index 238651e04..77cd48b56 100644 --- a/src/test/java/picocli/CommandLineTest.java +++ b/src/test/java/picocli/CommandLineTest.java @@ -663,6 +663,52 @@ public void testSetUsageHelpWidth_AfterSubcommandsAdded() { assertTrue(grandChildCount > 0); } + @Test + public void testParserMaxArityIsMaxTotalParams_BeforeSubcommandsAdded() { + @Command class TopLevel {} + CommandLine commandLine = new CommandLine(new TopLevel()); + assertEquals(false, commandLine.isMaxArityIsMaxTotalParams()); + commandLine.setMaxArityIsMaxTotalParams(true); + assertEquals(true, commandLine.isMaxArityIsMaxTotalParams()); + + int childCount = 0; + int grandChildCount = 0; + commandLine.addSubcommand("main", createNestedCommand()); + for (CommandLine sub : commandLine.getSubcommands().values()) { + childCount++; + assertEquals("subcommand added afterwards is not impacted", false, sub.isMaxArityIsMaxTotalParams()); + for (CommandLine subsub : sub.getSubcommands().values()) { + grandChildCount++; + assertEquals("subcommand added afterwards is not impacted", false, subsub.isMaxArityIsMaxTotalParams()); + } + } + assertTrue(childCount > 0); + assertTrue(grandChildCount > 0); + } + + @Test + public void testParserMaxArityIsMaxTotalParams_AfterSubcommandsAdded() { + @Command class TopLevel {} + CommandLine commandLine = new CommandLine(new TopLevel()); + commandLine.addSubcommand("main", createNestedCommand()); + assertEquals(false, commandLine.isMaxArityIsMaxTotalParams()); + commandLine.setMaxArityIsMaxTotalParams(true); + assertEquals(true, commandLine.isMaxArityIsMaxTotalParams()); + + int childCount = 0; + int grandChildCount = 0; + for (CommandLine sub : commandLine.getSubcommands().values()) { + childCount++; + assertEquals("subcommand added before IS impacted", true, sub.isMaxArityIsMaxTotalParams()); + for (CommandLine subsub : sub.getSubcommands().values()) { + grandChildCount++; + assertEquals("subsubcommand added before IS impacted", true, sub.isMaxArityIsMaxTotalParams()); + } + } + assertTrue(childCount > 0); + assertTrue(grandChildCount > 0); + } + @Test public void testOptionsMixedWithParameters() { CompactFields compact = CommandLine.populateCommand(new CompactFields(), "-r -v p1 -o out p2".split(" ")); @@ -844,7 +890,7 @@ public void testDebugOutputForDoubleDashSeparatesPositionalParameters() throws E "[picocli DEBUG] Could not register converter for java.nio.file.Path: java.lang.ClassNotFoundException: java.nio.file.Path%n"); String expected = String.format("" + "[picocli INFO] Parsing 6 command line args [-oout, --, -r, -v, p1, p2]%n" + - "[picocli DEBUG] Parser configuration: posixClusteredShortOptionsAllowed=true, stopAtPositional=false, stopAtUnmatched=false, separator=null, overwrittenOptionsAllowed=false, unmatchedArgumentsAllowed=false, expandAtFiles=true%n" + + "[picocli DEBUG] Parser configuration: posixClusteredShortOptionsAllowed=true, stopAtPositional=false, stopAtUnmatched=false, separator=null, overwrittenOptionsAllowed=false, unmatchedArgumentsAllowed=false, expandAtFiles=true, maxArityIsMaxTotalParams=false%n" + "[picocli DEBUG] Initializing %1$s$CompactFields: 3 options, 1 positional parameters, 0 required, 0 subcommands.%n" + "[picocli DEBUG] Processing argument '-oout'. Remainder=[--, -r, -v, p1, p2]%n" + "[picocli DEBUG] '-oout' cannot be separated into