Fix issue with unconditional remaining properties in subcommands (#397)

* Propagate unconditional remaining arguments to higher commands

This changes the behavior of parsing when a subcommand includes an
argument array with an unconditionalRemaining parsing strategy, such
that parsing options stops when the subcommand is encountered, so
that the subcommand can pick up those additional options.

Author: natecook1000

Sources/ArgumentParser/Parsable Types/ParsableCommand.swift

@@ -31,6 +31,8 @@ public protocol ParsableCommand: ParsableArguments {
   mutating func run() throws
 }
 
+// MARK: - Default implementations
+
 extension ParsableCommand {
   public static var _commandName: String {
     configuration.commandName ??
@@ -105,3 +107,22 @@ extension ParsableCommand {
     self.main(nil)
   }
 }
+
+// MARK: - Internal API
+
+extension ParsableCommand {
+  /// `true` if this command contains any array arguments that are declared
+  /// with `.unconditionalRemaining`.
+  internal static var includesUnconditionalArguments: Bool {
+    ArgumentSet(self).contains(where: {
+      $0.isRepeatingPositional && $0.parsingStrategy == .allRemainingInput
+    })
+  }
+  
+  /// `true` if this command's default subcommand contains any array arguments
+  /// that are declared with `.unconditionalRemaining`. This is `false` if
+  /// there's no default subcommand.
+  internal static var defaultIncludesUnconditionalArguments: Bool {
+    configuration.defaultSubcommand?.includesUnconditionalArguments == true
+  }
+}

Sources/ArgumentParser/Parsing/ArgumentSet.swift

@@ -192,19 +192,24 @@ extension ArgumentDefinition {
 
 // MARK: - Parsing from SplitArguments
 extension ArgumentSet {
-  /// Parse the given input (`SplitArguments`) for the given `commandStack` of previously parsed commands.
+  /// Parse the given input for this set of defined arguments.
   ///
-  /// This method will gracefully fail if there are extra arguments that it doesn’t understand. Hence the
-  /// *lenient* name. If so, it will return `.partial`.
+  /// This method will consume only the arguments that it understands. If any
+  /// arguments are declared to capture all remaining input, or a subcommand
+  /// is configured as such, parsing stops on the first positional argument or
+  /// unrecognized dash-prefixed argument.
   ///
-  /// When dealing with commands, this will be called iteratively in order to find
-  /// the matching command(s).
-  ///
-  /// - Parameter all: The input (from the command line) that needs to be parsed
-  /// - Parameter commandStack: commands that have been parsed
-  func lenientParse(_ all: SplitArguments) throws -> ParsedValues {
+  /// - Parameter input: The input that needs to be parsed.
+  /// - Parameter subcommands: Any subcommands of the current command.
+  /// - Parameter defaultCapturesAll: `true` if the default subcommand has an
+  ///   argument that captures all remaining input.
+  func lenientParse(
+    _ input: SplitArguments,
+    subcommands: [ParsableCommand.Type],
+    defaultCapturesAll: Bool
+  ) throws -> ParsedValues {
     // Create a local, mutable copy of the arguments:
-    var inputArguments = all
+    var inputArguments = input
 
     func parseValue(
       _ argument: ArgumentDefinition,
@@ -340,11 +345,11 @@ extension ArgumentSet {
     // captures all remaining input, we use a different behavior, where we
     // shortcut out at the first sign of a positional argument or unrecognized
     // option/flag label.
-    let capturesAll = self.contains(where: { arg in
+    let capturesAll = defaultCapturesAll || self.contains(where: { arg in
       arg.isRepeatingPositional && arg.parsingStrategy == .allRemainingInput
     })
 
-    var result = ParsedValues(elements: [:], originalInput: all.originalInput)
+    var result = ParsedValues(elements: [:], originalInput: input.originalInput)
     var allUsedOrigins = InputOrigin()
 
     try setInitialValues(into: &result)
@@ -359,7 +364,19 @@ extension ArgumentSet {
       }
 
       switch next.value {
-      case .value:
+      case .value(let argument):
+        // Special handling for matching subcommand names. We generally want
+        // parsing to skip over unrecognized input, but if the current
+        // command or the matched subcommand captures all remaining input,
+        // then we want to break out of parsing at this point.
+        if let matchedSubcommand = subcommands.first(where: { $0._commandName == argument }) {
+          if !matchedSubcommand.includesUnconditionalArguments && defaultCapturesAll {
+            continue ArgumentLoop
+          } else if matchedSubcommand.includesUnconditionalArguments {
+            break ArgumentLoop
+          }
+        }
+        
         // If we're capturing all, the first positional value represents the
         // start of positional input.
         if capturesAll { break ArgumentLoop }
@@ -403,7 +420,7 @@ extension ArgumentSet {
 
     // We have parsed all non-positional values at this point.
     // Next: parse / consume the positional values.
-    var unusedArguments = all
+    var unusedArguments = input
     unusedArguments.removeAll(in: allUsedOrigins)
     try parsePositionalValues(from: unusedArguments, into: &result)
 

Sources/ArgumentParser/Parsing/CommandParser.swift

@@ -126,8 +126,11 @@ extension CommandParser {
     let commandArguments = ArgumentSet(currentNode.element)
 
     // Parse the arguments, ignoring anything unexpected
-    let values = try commandArguments.lenientParse(split)
-    
+    let values = try commandArguments.lenientParse(
+      split,
+      subcommands: currentNode.element.configuration.subcommands,
+      defaultCapturesAll: currentNode.element.defaultIncludesUnconditionalArguments)
+
     // Decode the values from ParsedValues into the ParsableCommand:
     let decoder = ArgumentDecoder(values: values, previouslyDecoded: decodedArguments)
     var decodedResult: ParsableCommand

Tests/ArgumentParserEndToEndTests/DefaultSubcommandEndToEndTests.swift

@@ -69,3 +69,110 @@ extension DefaultSubcommandEndToEndTests {
     XCTAssertThrowsError(try Main.parseAsRoot(["qux"]))
   }
 }
+
+extension DefaultSubcommandEndToEndTests {
+  fileprivate struct MyCommand: ParsableCommand {
+    static var configuration = CommandConfiguration(
+      subcommands: [Plugin.self, NonDefault.self, Other.self],
+      defaultSubcommand: Plugin.self
+    )
+    
+    @OptionGroup
+    var options: CommonOptions
+  }
+  
+  fileprivate struct CommonOptions: ParsableArguments {
+    @Flag(name: [.customLong("verbose"), .customShort("v")],
+          help: "Enable verbose aoutput.")
+    var verbose = false
+  }
+  
+  fileprivate struct Plugin: ParsableCommand {
+    @OptionGroup var options: CommonOptions
+    @Argument var pluginName: String
+    
+    @Argument(parsing: .unconditionalRemaining)
+    var pluginArguments: [String] = []
+  }
+  
+  fileprivate struct NonDefault: ParsableCommand {
+    @OptionGroup var options: CommonOptions
+    @Argument var pluginName: String
+    
+    @Argument(parsing: .unconditionalRemaining)
+    var pluginArguments: [String] = []
+  }
+  
+  fileprivate struct Other: ParsableCommand {
+    @OptionGroup var options: CommonOptions
+  }
+  
+  func testRemainingDefaultImplicit() throws {
+    AssertParseCommand(MyCommand.self, Plugin.self, ["my-plugin"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, [])
+      XCTAssertEqual(plugin.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, Plugin.self, ["my-plugin", "--verbose"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
+      XCTAssertEqual(plugin.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, Plugin.self, ["--verbose", "my-plugin", "--verbose"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
+      XCTAssertEqual(plugin.options.verbose, true)
+    }
+  }
+
+  func testRemainingDefaultExplicit() throws {
+    AssertParseCommand(MyCommand.self, Plugin.self, ["plugin", "my-plugin"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, [])
+      XCTAssertEqual(plugin.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, Plugin.self, ["plugin", "my-plugin", "--verbose"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
+      XCTAssertEqual(plugin.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, Plugin.self, ["--verbose", "plugin", "my-plugin", "--verbose"]) { plugin in
+      XCTAssertEqual(plugin.pluginName, "my-plugin")
+      XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
+      XCTAssertEqual(plugin.options.verbose, true)
+    }
+  }
+
+  func testRemainingNonDefault() throws {
+    AssertParseCommand(MyCommand.self, NonDefault.self, ["non-default", "my-plugin"]) { nondef in
+      XCTAssertEqual(nondef.pluginName, "my-plugin")
+      XCTAssertEqual(nondef.pluginArguments, [])
+      XCTAssertEqual(nondef.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, NonDefault.self, ["non-default", "my-plugin", "--verbose"]) { nondef in
+      XCTAssertEqual(nondef.pluginName, "my-plugin")
+      XCTAssertEqual(nondef.pluginArguments, ["--verbose"])
+      XCTAssertEqual(nondef.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, NonDefault.self, ["--verbose", "non-default", "my-plugin", "--verbose"]) { nondef in
+      XCTAssertEqual(nondef.pluginName, "my-plugin")
+      XCTAssertEqual(nondef.pluginArguments, ["--verbose"])
+      XCTAssertEqual(nondef.options.verbose, true)
+    }
+  }
+
+  func testRemainingDefaultOther() throws {
+    AssertParseCommand(MyCommand.self, Other.self, ["other"]) { other in
+      XCTAssertEqual(other.options.verbose, false)
+    }
+    AssertParseCommand(MyCommand.self, Other.self, ["other", "--verbose"]) { other in
+      XCTAssertEqual(other.options.verbose, true)
+    }
+  }
+  
+  func testRemainingDefaultFailure() {
+    XCTAssertThrowsError(try MyCommand.parseAsRoot([]))
+    XCTAssertThrowsError(try MyCommand.parseAsRoot(["--verbose"]))
+    XCTAssertThrowsError(try MyCommand.parseAsRoot(["plugin", "--verbose", "my-plugin"]))
+  }
+}