swift-argument-parser icon indicating copy to clipboard operation
swift-argument-parser copied to clipboard

Wrong error when providing an argument to an unknown option

Open klaaspieter opened this issue 5 years ago • 5 comments

Given this argument parser:

fileprivate struct Qwz: ParsableArguments {
  @Option() var name: String?
}

When I call it with --nm Name, I expect Unknown option '--nm', but I get Unexpected argument 'Name'.

Failing test case:

extension ErrorMessageTests {
  func testMispelledArgument_1() {
    AssertErrorMessage(Qwz.self, ["--nme", "Me"], "Unknown option '--nme'.")
  }
}

klaaspieter avatar Feb 29 '20 18:02 klaaspieter

Running the test suite and the example commands I was unable to hit this function https://github.com/apple/swift-argument-parser/blob/598ba07a2f9d61e34f00c289305e64cf52abb224/Sources/ArgumentParser/Usage/UsageGenerator.swift#L250-L259.

Which (as mentioned in #10) seems to be because ParseError.unknownOption is swallowed here https://github.com/apple/swift-argument-parser/blob/e17dac8424d0275d082dacf00524f2c372d50bf8/Sources/ArgumentParser/Parsing/ArgumentSet.swift#L373-L375.

Changing that behavior leads to many tests breaking 😅. I believe however the current behavior is a bug and, especially considering the maturity of this project, this breaking change should be made to fix the behvaior.

klaaspieter avatar Feb 29 '20 18:02 klaaspieter

The reason we swallow the error there is that we let you define command structures like this:

struct Root: ParsableCommand {
    static let configuration = CommandConfiguration(subcommands: [Sub.self])
    @Option() var foo: Int

    struct Sub: ParsableCommand {
        @OptionGroup() var options: Root
        @Option() var bar: Int
    }
}

This allows specifying global options before the subcommand, like root --foo 5 sub --bar 10. It's not a great interface practice, IMO, but we're supporting this behavior to match existing commands. When parsing this, we need to be able to successfully parse the Root command without erroring on --bar 10 — that's why the check for leftover values is deferred until we're actually extracting the final parsed command: https://github.com/apple/swift-argument-parser/blob/f6ac7b8118ff5d1bc0faee7f37bf6f8fd8f95602/Sources/ArgumentParser/Parsing/CommandParser.swift#L79-L83

natecook1000 avatar Feb 29 '20 19:02 natecook1000

@klaaspieter It looks like #10 resolved this — did you have more to do? See 4d57c8c43ff5107e634a55b85bf53c674d06d24c

natecook1000 avatar Mar 02 '20 20:03 natecook1000

It didn't and I've been looking at a way to resolve this ever since 😅

For example I would expect the follow to return Unknown option '--knd'. Did you mean '--kind'?:

swift run math stats average --knd mean 1 2 3

Instead it returns:

Error: The value 'mean' is invalid for '<values>'
Usage: math stats average [--kind <kind>] [<values> ...]

Like I said I've been looking at this, but I may have bitten of more than I can chew 😬

klaaspieter avatar Mar 03 '20 16:03 klaaspieter

Ah yes, that's a tricky case! Since it doesn't find --knd in the argument set it doesn't know whether it's a flag or an option, so it can't skip over mean when trying to fill values... Do you want to update the initial description, or open this as a separate issue?

natecook1000 avatar Mar 03 '20 18:03 natecook1000