From 61cf3d295769e677d0af3c9a9c1e3e30c1dd800e Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 18:50:48 +0200 Subject: [PATCH 1/8] Make method possible override Simpler variation of #378 --- src/main/java/org/apache/commons/cli/DefaultParser.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 7d846cb3b..b9c89af76 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -582,8 +582,9 @@ private void handleToken(final String token) throws ParseException { * the remaining tokens are added as-is in the arguments of the command line. * * @param token the command line token to handle + * @throws ParseException if parsing should fail */ - private void handleUnknownToken(final String token) throws ParseException { + protected void handleUnknownToken(final String token) throws ParseException { if (token.startsWith("-") && token.length() > 1 && !stopAtNonOption) { throw new UnrecognizedOptionException("Unrecognized option: " + token, token); } From 3f7d699c7414bd41f4439654efda558e985daa7d Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 20:11:48 +0200 Subject: [PATCH 2/8] Un-hide this method that may be needed in override. --- .../java/org/apache/commons/cli/DefaultParser.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index b9c89af76..c74112586 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -558,7 +558,7 @@ private void handleToken(final String token) throws ParseException { if (token != null) { currentToken = token; if (skipParsing) { - cmd.addArg(token); + addArg(token); } else if ("--".equals(token)) { skipParsing = true; } else if (currentOption != null && currentOption.acceptsArg() && isArgument(token)) { @@ -588,12 +588,22 @@ protected void handleUnknownToken(final String token) throws ParseException { if (token.startsWith("-") && token.length() > 1 && !stopAtNonOption) { throw new UnrecognizedOptionException("Unrecognized option: " + token, token); } - cmd.addArg(token); + addArg(token); if (stopAtNonOption) { skipParsing = true; } } + /** + * Adds token to command line {@link CommandLine#addArg(String)}. + * + * @param token the unrecognized option/argument. + * @since 1.10.0 + */ + protected void addArg(final String token) { + cmd.addArg(token); + } + /** * Tests if the token is a valid argument. * From 36d7d5937e1c5f0099a576692493e59e447ba463 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 20:52:01 +0200 Subject: [PATCH 3/8] Add tests --- .../org/apache/commons/cli/DefaultParser.java | 38 ++++++++- .../apache/commons/cli/DefaultParserTest.java | 78 +++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index c74112586..fa7e3654c 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -175,11 +175,19 @@ static int indexOfEqual(final String token) { protected Options options; /** - * Flag indicating how unrecognized tokens are handled. {@code true} to stop the parsing and add the remaining - * tokens to the args list. {@code false} to throw an exception. + * Flag indicating how unrecognized tokens are handled: {@code true} to stop the parsing and add the remaining + * tokens to the args list. {@code false} add current token to the arg list and continue parsing. */ protected boolean stopAtNonOption; + /** + * Flag indicating how unrecognized tokens are handled: {@code true} to abort parsing by throwing {@link ParseException}. + * {@code false} to ignore it. + * + * @since 1.10.0 + */ + protected boolean throwAtNonOption = true; + /** The token currently processed. */ protected String currentToken; @@ -583,9 +591,10 @@ private void handleToken(final String token) throws ParseException { * * @param token the command line token to handle * @throws ParseException if parsing should fail + * @since 1.10.0 */ protected void handleUnknownToken(final String token) throws ParseException { - if (token.startsWith("-") && token.length() > 1 && !stopAtNonOption) { + if (token.startsWith("-") && token.length() > 1 && throwAtNonOption) { throw new UnrecognizedOptionException("Unrecognized option: " + token, token); } addArg(token); @@ -692,6 +701,9 @@ public CommandLine parse(final Options options, final String[] arguments) throws return parse(options, arguments, null); } + /** + * @see #parse(Options, String[], Properties, boolean, boolean) + */ @Override public CommandLine parse(final Options options, final String[] arguments, final boolean stopAtNonOption) throws ParseException { return parse(options, arguments, null, stopAtNonOption); @@ -722,11 +734,31 @@ public CommandLine parse(final Options options, final String[] arguments, final * * @return the list of atomic option and value tokens * @throws ParseException if there are any problems encountered while parsing the command line tokens. + * @see #parse(Options, String[], Properties, boolean, boolean) */ public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption) throws ParseException { + return parse(options, arguments, properties, stopAtNonOption, !stopAtNonOption); + } + + /** + * Parses the arguments according to the specified options and properties. + * + * @param options the specified Options + * @param arguments the command line arguments + * @param properties command line option name-value pairs + * @param stopAtNonOption see {@link #stopAtNonOption}. + * @param throwAtNonOption see {@link #throwAtNonOption}. + * + * @return the list of atomic option and value tokens + * @throws ParseException if there are any problems encountered while parsing the command line tokens. + * @since 1.10.0 + */ + public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption, + final boolean throwAtNonOption) throws ParseException { this.options = options; this.stopAtNonOption = stopAtNonOption; + this.throwAtNonOption = throwAtNonOption; skipParsing = false; currentOption = null; expectedOpts = new ArrayList<>(options.getRequiredOptions()); diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index 02a8f4894..545c9c3a8 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashSet; @@ -157,6 +158,83 @@ public void setUp() { parser = new DefaultParser(); } + @Test + void chainingParsersHappyPath() throws ParseException { + Option a = Option.builder().option("a").longOpt("first-letter").build(); + Option b = Option.builder().option("b").longOpt("second-letter").build(); + Option c = Option.builder().option("c").longOpt("third-letter").build(); + Option d = Option.builder().option("d").longOpt("fourth-letter").build(); + + Options baseOptions = new Options(); + baseOptions.addOption(a); + baseOptions.addOption(b); + Options specificOptions = new Options(); + specificOptions.addOption(a); + specificOptions.addOption(b); + specificOptions.addOption(c); + specificOptions.addOption(d); + + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2"}; + + DefaultParser parser = new DefaultParser(); + + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, false, false); + assertEquals(2, baseCommandLine.getOptions().length); + assertEquals(4, baseCommandLine.getArgs().length); + assertTrue(baseCommandLine.hasOption("a")); + assertTrue(baseCommandLine.hasOption("b")); + assertFalse(baseCommandLine.hasOption("c")); + assertFalse(baseCommandLine.hasOption("d")); + assertFalse(baseCommandLine.getArgList().contains("-a")); + assertFalse(baseCommandLine.getArgList().contains("-b")); + assertTrue(baseCommandLine.getArgList().contains("-c")); + assertTrue(baseCommandLine.getArgList().contains("-d")); + assertTrue(baseCommandLine.getArgList().contains("arg1")); + assertTrue(baseCommandLine.getArgList().contains("arg2")); + + CommandLine specificCommandLine = parser.parse(specificOptions, args, null, false, true); + assertEquals(4, specificCommandLine.getOptions().length); + assertEquals(2, specificCommandLine.getArgs().length); + assertTrue(specificCommandLine.hasOption("a")); + assertTrue(specificCommandLine.hasOption("b")); + assertTrue(specificCommandLine.hasOption("c")); + assertTrue(specificCommandLine.hasOption("d")); + assertFalse(specificCommandLine.getArgList().contains("-a")); + assertFalse(specificCommandLine.getArgList().contains("-b")); + assertFalse(specificCommandLine.getArgList().contains("-c")); + assertFalse(specificCommandLine.getArgList().contains("-d")); + assertTrue(specificCommandLine.getArgList().contains("arg1")); + assertTrue(specificCommandLine.getArgList().contains("arg2")); + } + + @Test + void chainingParsersNonHappyPath() throws ParseException { + Option a = Option.builder().option("a").longOpt("first-letter").build(); + Option b = Option.builder().option("b").longOpt("second-letter").build(); + Option c = Option.builder().option("c").longOpt("third-letter").build(); + Option d = Option.builder().option("d").longOpt("fourth-letter").build(); + + Options baseOptions = new Options(); + baseOptions.addOption(a); + baseOptions.addOption(b); + Options specificOptions = new Options(); + specificOptions.addOption(a); + specificOptions.addOption(b); + specificOptions.addOption(c); + specificOptions.addOption(d); + + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2", "--rogue-option"}; + + DefaultParser parser = new DefaultParser(); + + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, false, false); + assertEquals(2, baseCommandLine.getOptions().length); + assertEquals(5, baseCommandLine.getArgs().length); + + UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, () -> parser.parse(specificOptions, args, null, false, true)); + assertTrue(e.getMessage().contains("--rogue-option")); + } + @Test void testBuilder() { // @formatter:off From 4e654087d18f84f6166d984c6de7f296a2f0157c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 21:01:42 +0200 Subject: [PATCH 4/8] Simplify non-happy path --- .../java/org/apache/commons/cli/DefaultParserTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index 545c9c3a8..7ec71191e 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -212,7 +212,6 @@ void chainingParsersNonHappyPath() throws ParseException { Option a = Option.builder().option("a").longOpt("first-letter").build(); Option b = Option.builder().option("b").longOpt("second-letter").build(); Option c = Option.builder().option("c").longOpt("third-letter").build(); - Option d = Option.builder().option("d").longOpt("fourth-letter").build(); Options baseOptions = new Options(); baseOptions.addOption(a); @@ -221,18 +220,17 @@ void chainingParsersNonHappyPath() throws ParseException { specificOptions.addOption(a); specificOptions.addOption(b); specificOptions.addOption(c); - specificOptions.addOption(d); - String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2", "--rogue-option"}; + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2"}; // -d is rogue option DefaultParser parser = new DefaultParser(); CommandLine baseCommandLine = parser.parse(baseOptions, args, null, false, false); assertEquals(2, baseCommandLine.getOptions().length); - assertEquals(5, baseCommandLine.getArgs().length); + assertEquals(4, baseCommandLine.getArgs().length); UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, () -> parser.parse(specificOptions, args, null, false, true)); - assertTrue(e.getMessage().contains("--rogue-option")); + assertTrue(e.getMessage().contains("-d")); } @Test From f74b1b7e21e7a8d84e40af7c38e862750eb46c12 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 21:06:41 +0200 Subject: [PATCH 5/8] Add test for "legacy" behaviour when there was one boolean doing this or that only --- .../apache/commons/cli/DefaultParserTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index 7ec71191e..7559e3e86 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -233,6 +233,33 @@ void chainingParsersNonHappyPath() throws ParseException { assertTrue(e.getMessage().contains("-d")); } + @Test + void legacyStopAtNonOption() throws ParseException { + Option a = Option.builder().option("a").longOpt("first-letter").build(); + Option b = Option.builder().option("b").longOpt("second-letter").build(); + Option c = Option.builder().option("c").longOpt("third-letter").build(); + + Options options = new Options(); + options.addOption(a); + options.addOption(b); + options.addOption(c); + + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2"}; // -d is rogue option + + DefaultParser parser = new DefaultParser(); + + CommandLine commandLine = parser.parse(options, args, null, true); + assertEquals(3, commandLine.getOptions().length); + assertEquals(3, commandLine.getArgs().length); + assertFalse(commandLine.getArgList().contains("-c")); + assertTrue(commandLine.getArgList().contains("-d")); + assertTrue(commandLine.getArgList().contains("arg1")); + assertTrue(commandLine.getArgList().contains("arg2")); + + UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, () -> parser.parse(options, args, null, false)); + assertTrue(e.getMessage().contains("-d")); + } + @Test void testBuilder() { // @formatter:off From 46b169dc5d9be7ea5f1edcd1a604eb8f3a18595c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 9 Jun 2025 21:07:31 +0200 Subject: [PATCH 6/8] Drop line --- src/test/java/org/apache/commons/cli/DefaultParserTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index 7559e3e86..420ecd4d2 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -251,7 +251,6 @@ void legacyStopAtNonOption() throws ParseException { CommandLine commandLine = parser.parse(options, args, null, true); assertEquals(3, commandLine.getOptions().length); assertEquals(3, commandLine.getArgs().length); - assertFalse(commandLine.getArgList().contains("-c")); assertTrue(commandLine.getArgList().contains("-d")); assertTrue(commandLine.getArgList().contains("arg1")); assertTrue(commandLine.getArgList().contains("arg2")); From 281ffb6e93cf72498c6f39e50dd79410d59f6df6 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 10 Jun 2025 11:05:03 +0200 Subject: [PATCH 7/8] Switch to enum As not all combos make sense. --- .../org/apache/commons/cli/DefaultParser.java | 64 +++++++++---- .../apache/commons/cli/DefaultParserTest.java | 89 +++++++++++++++++-- 2 files changed, 129 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index fa7e3654c..a3bfa62b9 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -168,6 +168,32 @@ static int indexOfEqual(final String token) { return token.indexOf(Char.EQUAL); } + /** + * Enum representing possible actions that may be done when "non option" is discovered during parsing. + * + * @since 1.10.0 + */ + public enum NonOptionAction { + /** + * Parsing continues and current token is ignored. + */ + IGNORE, + /** + * Parsing continues and current token is added to command line arguments. + */ + SKIP, + /** + * Parsing will stop and remaining tokens are added to command line arguments. + * Equivalent of {@code stopAtNonOption = true}. + */ + STOP, + /** + * Parsing will abort and exception is thrown. + * Equivalent of {@code stopAtNonOption = false}. + */ + THROW; + } + /** The command-line instance. */ protected CommandLine cmd; @@ -175,18 +201,20 @@ static int indexOfEqual(final String token) { protected Options options; /** - * Flag indicating how unrecognized tokens are handled: {@code true} to stop the parsing and add the remaining - * tokens to the args list. {@code false} add current token to the arg list and continue parsing. + * Flag indicating how unrecognized tokens are handled. {@code true} to stop the parsing and add the remaining + * tokens to the args list. {@code false} to throw an exception. + * + * @deprecated Use {@link #nonOptionAction} instead. This field is unused, and left for binary compatibility reasons. */ + @Deprecated protected boolean stopAtNonOption; /** - * Flag indicating how unrecognized tokens are handled: {@code true} to abort parsing by throwing {@link ParseException}. - * {@code false} to ignore it. + * Action to happen when "non option" token is discovered. * * @since 1.10.0 */ - protected boolean throwAtNonOption = true; + protected NonOptionAction nonOptionAction; /** The token currently processed. */ protected String currentToken; @@ -364,7 +392,7 @@ protected void handleConcatenatedOptions(final String token) throws ParseExcepti for (int i = 1; i < token.length(); i++) { final String ch = String.valueOf(token.charAt(i)); if (!options.hasOption(ch)) { - handleUnknownToken(stopAtNonOption && i > 1 ? token.substring(i) : token); + handleUnknownToken(nonOptionAction == NonOptionAction.STOP && i > 1 ? token.substring(i) : token); break; } handleOption(options.getOption(ch)); @@ -594,11 +622,13 @@ private void handleToken(final String token) throws ParseException { * @since 1.10.0 */ protected void handleUnknownToken(final String token) throws ParseException { - if (token.startsWith("-") && token.length() > 1 && throwAtNonOption) { + if (token.startsWith("-") && token.length() > 1 && nonOptionAction == NonOptionAction.THROW) { throw new UnrecognizedOptionException("Unrecognized option: " + token, token); } - addArg(token); - if (stopAtNonOption) { + if (!token.startsWith("-") || token.equals("-") || token.length() > 1 && nonOptionAction != NonOptionAction.IGNORE) { + addArg(token); + } + if (nonOptionAction == NonOptionAction.STOP) { skipParsing = true; } } @@ -702,7 +732,7 @@ public CommandLine parse(final Options options, final String[] arguments) throws } /** - * @see #parse(Options, String[], Properties, boolean, boolean) + * @see #parse(Options, String[], Properties, NonOptionAction) */ @Override public CommandLine parse(final Options options, final String[] arguments, final boolean stopAtNonOption) throws ParseException { @@ -734,11 +764,11 @@ public CommandLine parse(final Options options, final String[] arguments, final * * @return the list of atomic option and value tokens * @throws ParseException if there are any problems encountered while parsing the command line tokens. - * @see #parse(Options, String[], Properties, boolean, boolean) + * @see #parse(Options, String[], Properties, NonOptionAction) */ public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption) throws ParseException { - return parse(options, arguments, properties, stopAtNonOption, !stopAtNonOption); + return parse(options, arguments, properties, stopAtNonOption ? NonOptionAction.STOP : NonOptionAction.THROW); } /** @@ -747,18 +777,16 @@ public CommandLine parse(final Options options, final String[] arguments, final * @param options the specified Options * @param arguments the command line arguments * @param properties command line option name-value pairs - * @param stopAtNonOption see {@link #stopAtNonOption}. - * @param throwAtNonOption see {@link #throwAtNonOption}. + * @param nonOptionAction see {@link NonOptionAction}. * * @return the list of atomic option and value tokens * @throws ParseException if there are any problems encountered while parsing the command line tokens. * @since 1.10.0 */ - public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption, - final boolean throwAtNonOption) throws ParseException { + public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final NonOptionAction nonOptionAction) + throws ParseException { this.options = options; - this.stopAtNonOption = stopAtNonOption; - this.throwAtNonOption = throwAtNonOption; + this.nonOptionAction = nonOptionAction; skipParsing = false; currentOption = null; expectedOpts = new ArrayList<>(options.getRequiredOptions()); diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index 420ecd4d2..e55ca8d77 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -159,7 +159,7 @@ public void setUp() { } @Test - void chainingParsersHappyPath() throws ParseException { + void chainingParsersSkipHappyPath() throws ParseException { Option a = Option.builder().option("a").longOpt("first-letter").build(); Option b = Option.builder().option("b").longOpt("second-letter").build(); Option c = Option.builder().option("c").longOpt("third-letter").build(); @@ -178,7 +178,7 @@ void chainingParsersHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, false, false); + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.SKIP); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(4, baseCommandLine.getArgs().length); assertTrue(baseCommandLine.hasOption("a")); @@ -192,7 +192,7 @@ void chainingParsersHappyPath() throws ParseException { assertTrue(baseCommandLine.getArgList().contains("arg1")); assertTrue(baseCommandLine.getArgList().contains("arg2")); - CommandLine specificCommandLine = parser.parse(specificOptions, args, null, false, true); + CommandLine specificCommandLine = parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW); assertEquals(4, specificCommandLine.getOptions().length); assertEquals(2, specificCommandLine.getArgs().length); assertTrue(specificCommandLine.hasOption("a")); @@ -208,7 +208,7 @@ void chainingParsersHappyPath() throws ParseException { } @Test - void chainingParsersNonHappyPath() throws ParseException { + void chainingParsersSkipNonHappyPath() throws ParseException { Option a = Option.builder().option("a").longOpt("first-letter").build(); Option b = Option.builder().option("b").longOpt("second-letter").build(); Option c = Option.builder().option("c").longOpt("third-letter").build(); @@ -225,11 +225,88 @@ void chainingParsersNonHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, false, false); + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.SKIP); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(4, baseCommandLine.getArgs().length); - UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, () -> parser.parse(specificOptions, args, null, false, true)); + UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, + () -> parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW)); + assertTrue(e.getMessage().contains("-d")); + } + + @Test + void chainingParsersIgnoreHappyPath() throws ParseException { + Option a = Option.builder().option("a").longOpt("first-letter").build(); + Option b = Option.builder().option("b").longOpt("second-letter").build(); + Option c = Option.builder().option("c").longOpt("third-letter").build(); + Option d = Option.builder().option("d").longOpt("fourth-letter").build(); + + Options baseOptions = new Options(); + baseOptions.addOption(a); + baseOptions.addOption(b); + Options specificOptions = new Options(); + specificOptions.addOption(a); + specificOptions.addOption(b); + specificOptions.addOption(c); + specificOptions.addOption(d); + + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2"}; + + DefaultParser parser = new DefaultParser(); + + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.IGNORE); + assertEquals(2, baseCommandLine.getOptions().length); + assertEquals(2, baseCommandLine.getArgs().length); + assertTrue(baseCommandLine.hasOption("a")); + assertTrue(baseCommandLine.hasOption("b")); + assertFalse(baseCommandLine.hasOption("c")); + assertFalse(baseCommandLine.hasOption("d")); + assertFalse(baseCommandLine.getArgList().contains("-a")); + assertFalse(baseCommandLine.getArgList().contains("-b")); + assertFalse(baseCommandLine.getArgList().contains("-c")); + assertFalse(baseCommandLine.getArgList().contains("-d")); + assertTrue(baseCommandLine.getArgList().contains("arg1")); + assertTrue(baseCommandLine.getArgList().contains("arg2")); + + CommandLine specificCommandLine = parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW); + assertEquals(4, specificCommandLine.getOptions().length); + assertEquals(2, specificCommandLine.getArgs().length); + assertTrue(specificCommandLine.hasOption("a")); + assertTrue(specificCommandLine.hasOption("b")); + assertTrue(specificCommandLine.hasOption("c")); + assertTrue(specificCommandLine.hasOption("d")); + assertFalse(specificCommandLine.getArgList().contains("-a")); + assertFalse(specificCommandLine.getArgList().contains("-b")); + assertFalse(specificCommandLine.getArgList().contains("-c")); + assertFalse(specificCommandLine.getArgList().contains("-d")); + assertTrue(specificCommandLine.getArgList().contains("arg1")); + assertTrue(specificCommandLine.getArgList().contains("arg2")); + } + + @Test + void chainingParsersIgnoreNonHappyPath() throws ParseException { + Option a = Option.builder().option("a").longOpt("first-letter").build(); + Option b = Option.builder().option("b").longOpt("second-letter").build(); + Option c = Option.builder().option("c").longOpt("third-letter").build(); + + Options baseOptions = new Options(); + baseOptions.addOption(a); + baseOptions.addOption(b); + Options specificOptions = new Options(); + specificOptions.addOption(a); + specificOptions.addOption(b); + specificOptions.addOption(c); + + String[] args = {"-a", "-b", "-c", "-d", "arg1", "arg2"}; // -d is rogue option + + DefaultParser parser = new DefaultParser(); + + CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.IGNORE); + assertEquals(2, baseCommandLine.getOptions().length); + assertEquals(2, baseCommandLine.getArgs().length); + + UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, + () -> parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW)); assertTrue(e.getMessage().contains("-d")); } From 1ebf62bc06f47236c778c32e96a9dc784e2431f7 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 10 Jun 2025 14:02:24 +0200 Subject: [PATCH 8/8] Use vararg for args --- .../org/apache/commons/cli/DefaultParser.java | 10 +++++----- .../apache/commons/cli/DefaultParserTest.java | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index a3bfa62b9..bfd7c5f1a 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -732,7 +732,7 @@ public CommandLine parse(final Options options, final String[] arguments) throws } /** - * @see #parse(Options, String[], Properties, NonOptionAction) + * @see #parse(Options, Properties, NonOptionAction, String[]) */ @Override public CommandLine parse(final Options options, final String[] arguments, final boolean stopAtNonOption) throws ParseException { @@ -764,26 +764,26 @@ public CommandLine parse(final Options options, final String[] arguments, final * * @return the list of atomic option and value tokens * @throws ParseException if there are any problems encountered while parsing the command line tokens. - * @see #parse(Options, String[], Properties, NonOptionAction) + * @see #parse(Options, Properties, NonOptionAction, String[]) */ public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption) throws ParseException { - return parse(options, arguments, properties, stopAtNonOption ? NonOptionAction.STOP : NonOptionAction.THROW); + return parse(options, properties, stopAtNonOption ? NonOptionAction.STOP : NonOptionAction.THROW, arguments); } /** * Parses the arguments according to the specified options and properties. * * @param options the specified Options - * @param arguments the command line arguments * @param properties command line option name-value pairs * @param nonOptionAction see {@link NonOptionAction}. + * @param arguments the command line arguments * * @return the list of atomic option and value tokens * @throws ParseException if there are any problems encountered while parsing the command line tokens. * @since 1.10.0 */ - public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final NonOptionAction nonOptionAction) + public CommandLine parse(final Options options, final Properties properties, final NonOptionAction nonOptionAction, final String... arguments) throws ParseException { this.options = options; this.nonOptionAction = nonOptionAction; diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index e55ca8d77..35f96d5ba 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -178,7 +178,7 @@ void chainingParsersSkipHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.SKIP); + CommandLine baseCommandLine = parser.parse(baseOptions, null, DefaultParser.NonOptionAction.SKIP, args); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(4, baseCommandLine.getArgs().length); assertTrue(baseCommandLine.hasOption("a")); @@ -192,7 +192,7 @@ void chainingParsersSkipHappyPath() throws ParseException { assertTrue(baseCommandLine.getArgList().contains("arg1")); assertTrue(baseCommandLine.getArgList().contains("arg2")); - CommandLine specificCommandLine = parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW); + CommandLine specificCommandLine = parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args); assertEquals(4, specificCommandLine.getOptions().length); assertEquals(2, specificCommandLine.getArgs().length); assertTrue(specificCommandLine.hasOption("a")); @@ -225,12 +225,12 @@ void chainingParsersSkipNonHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.SKIP); + CommandLine baseCommandLine = parser.parse(baseOptions, null, DefaultParser.NonOptionAction.SKIP, args); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(4, baseCommandLine.getArgs().length); UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, - () -> parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW)); + () -> parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args)); assertTrue(e.getMessage().contains("-d")); } @@ -254,7 +254,7 @@ void chainingParsersIgnoreHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.IGNORE); + CommandLine baseCommandLine = parser.parse(baseOptions, null, DefaultParser.NonOptionAction.IGNORE, args); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(2, baseCommandLine.getArgs().length); assertTrue(baseCommandLine.hasOption("a")); @@ -268,7 +268,7 @@ void chainingParsersIgnoreHappyPath() throws ParseException { assertTrue(baseCommandLine.getArgList().contains("arg1")); assertTrue(baseCommandLine.getArgList().contains("arg2")); - CommandLine specificCommandLine = parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW); + CommandLine specificCommandLine = parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args); assertEquals(4, specificCommandLine.getOptions().length); assertEquals(2, specificCommandLine.getArgs().length); assertTrue(specificCommandLine.hasOption("a")); @@ -301,12 +301,12 @@ void chainingParsersIgnoreNonHappyPath() throws ParseException { DefaultParser parser = new DefaultParser(); - CommandLine baseCommandLine = parser.parse(baseOptions, args, null, DefaultParser.NonOptionAction.IGNORE); + CommandLine baseCommandLine = parser.parse(baseOptions, null, DefaultParser.NonOptionAction.IGNORE, args); assertEquals(2, baseCommandLine.getOptions().length); assertEquals(2, baseCommandLine.getArgs().length); UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class, - () -> parser.parse(specificOptions, args, null, DefaultParser.NonOptionAction.THROW)); + () -> parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args)); assertTrue(e.getMessage().contains("-d")); }