Add new options for parsing: ignore and skip (#379)

* Make method possible override

Simpler variation of #378

* Un-hide this method that may be needed in override.

* Add tests

* Simplify non-happy path

* Add test for "legacy" behaviour

when there was one boolean doing this or that only

* Drop line

* Switch to enum

As not all combos make sense.

* Use vararg for args

Author: cstamas

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;
 
@@ -177,9 +203,19 @@ static int indexOfEqual(final String token) {
     /**
      * 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;
 
+    /**
+     * Action to happen when "non option" token is discovered.
+     *
+     * @since 1.10.0
+     */
+    protected NonOptionAction nonOptionAction;
+
     /** The token currently processed. */
     protected String currentToken;
 
@@ -356,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));
@@ -558,7 +594,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)) {
@@ -582,17 +618,31 @@ 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
+     * @since 1.10.0
      */
-    private void handleUnknownToken(final String token) throws ParseException {
-        if (token.startsWith("-") && token.length() > 1 && !stopAtNonOption) {
+    protected void handleUnknownToken(final String token) throws ParseException {
+        if (token.startsWith("-") && token.length() > 1 && nonOptionAction == NonOptionAction.THROW) {
             throw new UnrecognizedOptionException("Unrecognized option: " + token, token);
         }
-        cmd.addArg(token);
-        if (stopAtNonOption) {
+        if (!token.startsWith("-") || token.equals("-") || token.length() > 1 && nonOptionAction != NonOptionAction.IGNORE) {
+            addArg(token);
+        }
+        if (nonOptionAction == NonOptionAction.STOP) {
             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.
      *
@@ -681,6 +731,9 @@ public CommandLine parse(final Options options, final String[] arguments) throws
         return parse(options, arguments, null);
     }
 
+    /**
+     * @see #parse(Options, Properties, NonOptionAction, String[])
+     */
     @Override
     public CommandLine parse(final Options options, final String[] arguments, final boolean stopAtNonOption) throws ParseException {
         return parse(options, arguments, null, stopAtNonOption);
@@ -711,11 +764,29 @@ 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, Properties, NonOptionAction, String[])
      */
     public CommandLine parse(final Options options, final String[] arguments, final Properties properties, final boolean stopAtNonOption)
         throws ParseException {
+        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 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 Properties properties, final NonOptionAction nonOptionAction, final String... arguments)
+            throws ParseException {
         this.options = options;
-        this.stopAtNonOption = stopAtNonOption;
+        this.nonOptionAction = nonOptionAction;
         skipParsing = false;
         currentOption = null;
         expectedOpts = new ArrayList<>(options.getRequiredOptions());

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,184 @@ public void setUp() {
         parser = new DefaultParser();
     }
 
+    @Test
+    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();
+        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, null, DefaultParser.NonOptionAction.SKIP, args);
+        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, null, DefaultParser.NonOptionAction.THROW, args);
+        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 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();
+
+        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, null, DefaultParser.NonOptionAction.SKIP, args);
+        assertEquals(2, baseCommandLine.getOptions().length);
+        assertEquals(4, baseCommandLine.getArgs().length);
+
+        UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class,
+                () -> parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args));
+        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, null, DefaultParser.NonOptionAction.IGNORE, args);
+        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, null, DefaultParser.NonOptionAction.THROW, args);
+        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, null, DefaultParser.NonOptionAction.IGNORE, args);
+        assertEquals(2, baseCommandLine.getOptions().length);
+        assertEquals(2, baseCommandLine.getArgs().length);
+
+        UnrecognizedOptionException e = assertThrows(UnrecognizedOptionException.class,
+                () -> parser.parse(specificOptions, null, DefaultParser.NonOptionAction.THROW, args));
+        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);
+        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