Skip to content

Commit 1a5b1b3

Browse files
natecook1000natecook1000
authored
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.
1 parent 9a71866 commit 1a5b1b3

File tree

4 files changed

+164
-16
lines changed

4 files changed

+164
-16
lines changed

‎Sources/ArgumentParser/Parsable Types/ParsableCommand.swift‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ public protocol ParsableCommand: ParsableArguments {
3131
mutating func run() throws
3232
}
3333

34+
// MARK: - Default implementations
35+
3436
extension ParsableCommand {
3537
public static var _commandName: String {
3638
configuration.commandName ??
@@ -105,3 +107,22 @@ extension ParsableCommand {
105107
self.main(nil)
106108
}
107109
}
110+
111+
// MARK: - Internal API
112+
113+
extension ParsableCommand {
114+
/// `true` if this command contains any array arguments that are declared
115+
/// with `.unconditionalRemaining`.
116+
internal static var includesUnconditionalArguments: Bool {
117+
ArgumentSet(self).contains(where: {
118+
$0.isRepeatingPositional && $0.parsingStrategy == .allRemainingInput
119+
})
120+
}
121+
122+
/// `true` if this command's default subcommand contains any array arguments
123+
/// that are declared with `.unconditionalRemaining`. This is `false` if
124+
/// there's no default subcommand.
125+
internal static var defaultIncludesUnconditionalArguments: Bool {
126+
configuration.defaultSubcommand?.includesUnconditionalArguments == true
127+
}
128+
}

‎Sources/ArgumentParser/Parsing/ArgumentSet.swift‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -192,19 +192,24 @@ extension ArgumentDefinition {
192192

193193
// MARK: - Parsing from SplitArguments
194194
extension ArgumentSet {
195-
/// Parse the given input (`SplitArguments`) for the given `commandStack` of previously parsed commands.
195+
/// Parse the given input for this set of defined arguments.
196196
///
197-
/// This method will gracefully fail if there are extra arguments that it doesn’t understand. Hence the
198-
/// *lenient* name. If so, it will return `.partial`.
197+
/// This method will consume only the arguments that it understands. If any
198+
/// arguments are declared to capture all remaining input, or a subcommand
199+
/// is configured as such, parsing stops on the first positional argument or
200+
/// unrecognized dash-prefixed argument.
199201
///
200-
/// When dealing with commands, this will be called iteratively in order to find
201-
/// the matching command(s).
202-
///
203-
/// - Parameter all: The input (from the command line) that needs to be parsed
204-
/// - Parameter commandStack: commands that have been parsed
205-
func lenientParse(_ all: SplitArguments) throws -> ParsedValues {
202+
/// - Parameter input: The input that needs to be parsed.
203+
/// - Parameter subcommands: Any subcommands of the current command.
204+
/// - Parameter defaultCapturesAll: `true` if the default subcommand has an
205+
/// argument that captures all remaining input.
206+
func lenientParse(
207+
_ input: SplitArguments,
208+
subcommands: [ParsableCommand.Type],
209+
defaultCapturesAll: Bool
210+
) throws -> ParsedValues {
206211
// Create a local, mutable copy of the arguments:
207-
var inputArguments = all
212+
var inputArguments = input
208213

209214
func parseValue(
210215
_ argument: ArgumentDefinition,
@@ -340,11 +345,11 @@ extension ArgumentSet {
340345
// captures all remaining input, we use a different behavior, where we
341346
// shortcut out at the first sign of a positional argument or unrecognized
342347
// option/flag label.
343-
let capturesAll = self.contains(where: { arg in
348+
let capturesAll = defaultCapturesAll || self.contains(where: { arg in
344349
arg.isRepeatingPositional && arg.parsingStrategy == .allRemainingInput
345350
})
346351

347-
var result = ParsedValues(elements: [:], originalInput: all.originalInput)
352+
var result = ParsedValues(elements: [:], originalInput: input.originalInput)
348353
var allUsedOrigins = InputOrigin()
349354

350355
try setInitialValues(into: &result)
@@ -359,7 +364,19 @@ extension ArgumentSet {
359364
}
360365

361366
switch next.value {
362-
case .value:
367+
case .value(let argument):
368+
// Special handling for matching subcommand names. We generally want
369+
// parsing to skip over unrecognized input, but if the current
370+
// command or the matched subcommand captures all remaining input,
371+
// then we want to break out of parsing at this point.
372+
if let matchedSubcommand = subcommands.first(where: { $0._commandName == argument }) {
373+
if !matchedSubcommand.includesUnconditionalArguments && defaultCapturesAll {
374+
continue ArgumentLoop
375+
} else if matchedSubcommand.includesUnconditionalArguments {
376+
break ArgumentLoop
377+
}
378+
}
379+
363380
// If we're capturing all, the first positional value represents the
364381
// start of positional input.
365382
if capturesAll { break ArgumentLoop }
@@ -403,7 +420,7 @@ extension ArgumentSet {
403420

404421
// We have parsed all non-positional values at this point.
405422
// Next: parse / consume the positional values.
406-
var unusedArguments = all
423+
var unusedArguments = input
407424
unusedArguments.removeAll(in: allUsedOrigins)
408425
try parsePositionalValues(from: unusedArguments, into: &result)
409426

‎Sources/ArgumentParser/Parsing/CommandParser.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,11 @@ extension CommandParser {
126126
let commandArguments = ArgumentSet(currentNode.element)
127127

128128
// Parse the arguments, ignoring anything unexpected
129-
let values = try commandArguments.lenientParse(split)
130-
129+
let values = try commandArguments.lenientParse(
130+
split,
131+
subcommands: currentNode.element.configuration.subcommands,
132+
defaultCapturesAll: currentNode.element.defaultIncludesUnconditionalArguments)
133+
131134
// Decode the values from ParsedValues into the ParsableCommand:
132135
let decoder = ArgumentDecoder(values: values, previouslyDecoded: decodedArguments)
133136
var decodedResult: ParsableCommand

‎Tests/ArgumentParserEndToEndTests/DefaultSubcommandEndToEndTests.swift‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,110 @@ extension DefaultSubcommandEndToEndTests {
6969
XCTAssertThrowsError(try Main.parseAsRoot(["qux"]))
7070
}
7171
}
72+
73+
extension DefaultSubcommandEndToEndTests {
74+
fileprivate struct MyCommand: ParsableCommand {
75+
static var configuration = CommandConfiguration(
76+
subcommands: [Plugin.self, NonDefault.self, Other.self],
77+
defaultSubcommand: Plugin.self
78+
)
79+
80+
@OptionGroup
81+
var options: CommonOptions
82+
}
83+
84+
fileprivate struct CommonOptions: ParsableArguments {
85+
@Flag(name: [.customLong("verbose"), .customShort("v")],
86+
help: "Enable verbose aoutput.")
87+
var verbose = false
88+
}
89+
90+
fileprivate struct Plugin: ParsableCommand {
91+
@OptionGroup var options: CommonOptions
92+
@Argument var pluginName: String
93+
94+
@Argument(parsing: .unconditionalRemaining)
95+
var pluginArguments: [String] = []
96+
}
97+
98+
fileprivate struct NonDefault: ParsableCommand {
99+
@OptionGroup var options: CommonOptions
100+
@Argument var pluginName: String
101+
102+
@Argument(parsing: .unconditionalRemaining)
103+
var pluginArguments: [String] = []
104+
}
105+
106+
fileprivate struct Other: ParsableCommand {
107+
@OptionGroup var options: CommonOptions
108+
}
109+
110+
func testRemainingDefaultImplicit() throws {
111+
AssertParseCommand(MyCommand.self, Plugin.self, ["my-plugin"]) { plugin in
112+
XCTAssertEqual(plugin.pluginName, "my-plugin")
113+
XCTAssertEqual(plugin.pluginArguments, [])
114+
XCTAssertEqual(plugin.options.verbose, false)
115+
}
116+
AssertParseCommand(MyCommand.self, Plugin.self, ["my-plugin", "--verbose"]) { plugin in
117+
XCTAssertEqual(plugin.pluginName, "my-plugin")
118+
XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
119+
XCTAssertEqual(plugin.options.verbose, false)
120+
}
121+
AssertParseCommand(MyCommand.self, Plugin.self, ["--verbose", "my-plugin", "--verbose"]) { plugin in
122+
XCTAssertEqual(plugin.pluginName, "my-plugin")
123+
XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
124+
XCTAssertEqual(plugin.options.verbose, true)
125+
}
126+
}
127+
128+
func testRemainingDefaultExplicit() throws {
129+
AssertParseCommand(MyCommand.self, Plugin.self, ["plugin", "my-plugin"]) { plugin in
130+
XCTAssertEqual(plugin.pluginName, "my-plugin")
131+
XCTAssertEqual(plugin.pluginArguments, [])
132+
XCTAssertEqual(plugin.options.verbose, false)
133+
}
134+
AssertParseCommand(MyCommand.self, Plugin.self, ["plugin", "my-plugin", "--verbose"]) { plugin in
135+
XCTAssertEqual(plugin.pluginName, "my-plugin")
136+
XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
137+
XCTAssertEqual(plugin.options.verbose, false)
138+
}
139+
AssertParseCommand(MyCommand.self, Plugin.self, ["--verbose", "plugin", "my-plugin", "--verbose"]) { plugin in
140+
XCTAssertEqual(plugin.pluginName, "my-plugin")
141+
XCTAssertEqual(plugin.pluginArguments, ["--verbose"])
142+
XCTAssertEqual(plugin.options.verbose, true)
143+
}
144+
}
145+
146+
func testRemainingNonDefault() throws {
147+
AssertParseCommand(MyCommand.self, NonDefault.self, ["non-default", "my-plugin"]) { nondef in
148+
XCTAssertEqual(nondef.pluginName, "my-plugin")
149+
XCTAssertEqual(nondef.pluginArguments, [])
150+
XCTAssertEqual(nondef.options.verbose, false)
151+
}
152+
AssertParseCommand(MyCommand.self, NonDefault.self, ["non-default", "my-plugin", "--verbose"]) { nondef in
153+
XCTAssertEqual(nondef.pluginName, "my-plugin")
154+
XCTAssertEqual(nondef.pluginArguments, ["--verbose"])
155+
XCTAssertEqual(nondef.options.verbose, false)
156+
}
157+
AssertParseCommand(MyCommand.self, NonDefault.self, ["--verbose", "non-default", "my-plugin", "--verbose"]) { nondef in
158+
XCTAssertEqual(nondef.pluginName, "my-plugin")
159+
XCTAssertEqual(nondef.pluginArguments, ["--verbose"])
160+
XCTAssertEqual(nondef.options.verbose, true)
161+
}
162+
}
163+
164+
func testRemainingDefaultOther() throws {
165+
AssertParseCommand(MyCommand.self, Other.self, ["other"]) { other in
166+
XCTAssertEqual(other.options.verbose, false)
167+
}
168+
AssertParseCommand(MyCommand.self, Other.self, ["other", "--verbose"]) { other in
169+
XCTAssertEqual(other.options.verbose, true)
170+
}
171+
}
172+
173+
func testRemainingDefaultFailure() {
174+
XCTAssertThrowsError(try MyCommand.parseAsRoot([]))
175+
XCTAssertThrowsError(try MyCommand.parseAsRoot(["--verbose"]))
176+
XCTAssertThrowsError(try MyCommand.parseAsRoot(["plugin", "--verbose", "my-plugin"]))
177+
}
178+
}

0 commit comments

Comments
 (0)