Swift ArgumentParser: why do top-level arguments clobber subcommand arguments?

Related Projects ArgumentParser
1

i was very puzzled this afternoon, at why swift-argument-parser was silently dropping command line arguments in subcommands. in particular, given this reduced test program:

import ArgumentParser

public
struct XCommand:Decodable
{
    @Option(name: [.customShort("u")])
    var u:String = "foo"

    public
    init()
    {
    }
}

extension XCommand:AsyncParsableCommand
{
    public
    static let configuration:CommandConfiguration = .init(commandName: "x")

    public
    func run() throws
    {
        print("x: u = \(self.u)")
    }
}

struct Main
{
    @Option(name: [.customShort("u")])
    var u:String = "foo"
}
@main
extension Main:AsyncParsableCommand
{
    static var configuration:CommandConfiguration
    {
        .init(commandName: "main", subcommands: [XCommand.self])
    }

    mutating
    func run() async throws
    {
        print("main: u = \(self.u)")
    }
}

i would have expected app x -u bar to print x: u = bar, and not the default value foo. but this is not what happens. instead, the var u argument from the outer command steals the arguments that are meant for the subcommand.

$ .build/debug/sap-lost-arg x -u bar 
x: u = foo

why does it do this?

2

ArgumentParser parses commands from the root level downward, and because flags/options from commands and subcommands can be specified anywhere in the list, an option that matches at the top level will be captured before a subcommand is parsed.

In general, I encourage people to put all their argument/flag/options on leaf commands, using @OptionGroup to share common arguments across different subcommands. An alternative, if you do want to have the -u option available on both the root command and the subcommand, is to use @OptionGroup var main: Main in the subcommand instead of defining a new option – that will provide access to the bar value that was parsed into Main.

2 Likes
3

Like Nate said, if I had a shared parameter, I’d put it in an @OptionGroup:

import ArgumentParser

struct SharedOptions: ParsableArguments {
    @Option(name: [.long, .short], help: "The user to greet.")
    var user: String = "foo"
}

struct XCommand: AsyncParsableCommand {
    @OptionGroup var options: SharedOptions

    static let configuration = CommandConfiguration(commandName: "x")

    func run() async throws {
        print("x: Hello user \(options.user)")
    }
}

@main
struct Main: AsyncParsableCommand {
    @OptionGroup var options: SharedOptions

    @Flag(name: [.long, .short], help: "Print the version number.")
    var version: Bool = false

    static let configuration = CommandConfiguration(commandName: "main", subcommands: [XCommand.self])

    func run() async throws {
        if version {
            print("0.1")
        }
        print("main: Hello user \(options.user)")
    }
}

[I've modified the above to use static let configuration = … instead of var to avoid Swift 6 warnings.]

4

That foo is the default value for u:String = "foo" in the XCommand struct, not the outer struct. I c hanged var u:String = "foo" in XCommand to var u:String = "XComandFoo" and ran your code:

release> ./LeafOnly x -u bar
x: u = XComandFoo

Your point is still valid: adding @Option to a command messes stuff up at its subcommand level.

5

I fixed the minor typo, changing the var to let so it would build. Then I ran your code (skip
to the end for the issue).

Sorry for setting up the package as "LeafOnly" instead of "main", I was running out of
names.

release> ./LeafOnly --help
USAGE: main [--user <user>] [--version] <subcommand>

OPTIONS:
  -u, --user <user>       The user to greet. (default: foo)
  -v, --version           Print the version number.
  -h, --help              Show help information.

SUBCOMMANDS:
  x

  See 'main help <subcommand>' for detailed help.

Worked fine. Following the advice:

release> ./LeafOnly help x
USAGE: main x [--user <user>]

OPTIONS:
  -u, --user <user>       The user to greet. (default: foo)
  -h, --help              Show help information.

Looks good, as does this:

release> ./LeafOnly x --help
USAGE: main x [--user <user>]

OPTIONS:
  -u, --user <user>       The user to greet. (default: foo)
  -h, --help              Show help information.

All good. But I did not expect this:

release> ./LeafOnly x --user bar
USAGE: main x [--user <user>]

OPTIONS:
  -u, --user <user>       The user to greet. (default: foo)
  -h, --help              Show help information.

I don't know if this is a bug or a programmer error (i.e., you should have known that this would not work - as disclosed in the documentation?) Either way I think something should be done.
Maybe this has a quick fix or I missed something.

6

Intriguing. It compiles (and works) for me with either var or let. But if it’s telling you to use let, then go ahead and do so.

7

Sorry, I should not have assumed a typo. I assumed you were running Swift 6.0, or
something that would fail because with var "'configuration' is not concurrency-safe because it is nonisolated global shared mutable state". (Also, since I make so many typos myself, it is my immediate response).

In any case, that's all beside the point. Your example uncovered more undefined behavior which is what concerns me.

1 Like
8

Did you declare a run() method for this subcommand? This looks like the default implementation for a command.

9

We’d have to see your full reproducible example to comment further. [My apologies, but I now see that you have done that.] What I can say is that ArgumentParser is pretty weak in terms of meaningful diagnostic messages to help resolve incorrect implementations. When there is a misconfiguration, it frequently just shows the “usage” message.

E.g., a mistake that I stumbled across was having an async implementation of run but using ParsableCommand instead of AsyncParsableCommand. It just shows the “usage” message when it cannot resolve run. It provides no useful diagnostic information in these cases. We’re left to figure it out on our own.

FWIW, when I suggested in issue #561 that some diagnostic messages would be quite helpful, Nate suggested that this wasn’t quite as trivial as it might seem:

I guess my reaction is why wouldn’t the default implementation within the protocol say something about “No applicable run found.” Are there really situations where you would want the default implementation behavior in lieu of our own run? Or are there scenarios where we wouldn’t want to implement run and want to enjoy this default implementation behavior? But I’ll defer to Nate on this one.

10

We definitely don't want the default instead of your own implementation, but the default is nice for commands that have commands that aren't end points, but have one or more subcommands (or nested subcommands) that do the actual work. In those cases, those intermediate subcommands don't require a run() implementation and the default behavior of printing help (that includes their subcommands) is a nice simplification.

1 Like
11

First, thanks for your time in engaging in this thread. Your input is always great.

Second, would you be open to a PR that clarified the documentation a bit? I’m thinking of a “note” that warns developers that if they get “usage” text rather than executing run, that this is either because either:

  • They failed to provide a run implementation;
  • There were some non-optional options defined, but were not provided at the command line (or should the documentation just say “don’t do that!”; the whole idea of an “option” that is required feels like an anti-pattern; lol); or
  • They are using ParsableCommand with an asynchronous run implementation.

FWIW, we have already added a comment on that last point, but I wonder if further clarification might be warranted.

And if you are open to such a PR, are there other scenarios that might result in the unintentional “usage” message that you’d like to add to the list?

I think that Options declared in a command with subcommands can cause issues might reveal some other weird edge cases.

1 Like