Repository: apple/swift-argument-parser (swift)

Issue:
Confusing error output when passing a single-dash unknown option that contains some valid short label option names
<!--
    Thanks for contributing to the Swift Argument Parser!

    Before you submit your issue, please replace each paragraph
    below with the relevant details for your bug, and complete
    the steps in the checklist by placing an 'x' in each box:
    
    - [x] I've completed this task
    - [ ] This task isn't completed
-->

see relevant discussion on the swift forums here: https://forums.swift.org/t/why-does-swift-treat-double-dash-command-line-arguments-differently/77727/8

**ArgumentParser version:**
`main` branch (commit: a01856b)
**Swift version:**
swift-driver version: 1.115 Apple Swift version 6.0.2 (swiftlang-6.0.2.1.2 clang-1600.0.26.4)
Target: arm64-apple-macosx14.0

### Checklist
- [x] If possible, I've reproduced the issue using the `main` branch of this package
- [x] I've searched for [existing GitHub issues](https://github.com/apple/swift-argument-parser/issues)
  - found https://github.com/apple/swift-argument-parser/issues/23 which seems a bit similar in the high level description, but the details seem probably different

### Steps to Reproduce

run this test case in `ParsableArgumentsValidationTests`:

```swift
extension ParsableArgumentsValidationTests {
    func test_shouldMaybeProduceUnknownOptionErrorInsteadOfMissingValue() throws {
      struct SomeArgs: ParsableArguments {
        @Option(name: .shortAndLong)
        var x_arg: Int

        @Option(name: .shortAndLong)
        var z_arg: Int
      }

      // swap order of 'x' and 'z' in the string to get different error output
      AssertErrorMessage(
        SomeArgs.self,
        [
          "-should_maybe_be_an_unknown_option_error_but_is_a_missing_value_error_when_it_includes_x_or_z",
        ],
        "Unknown option '-should_maybe_be_an_unknown_option_error_but_is_a_missing_value_error_when_it_includes_x_or_z'"
      )
      // actually: "Missing value for '-x <x_arg>'"
    }
}
```

### Expected behavior
an error indicating the given option is unrecognized

### Actual behavior
an error is produced suggesting that the option name is valid but is missing a value

Pull request:
When a required option has a short name, longer single-dash arguments can end up incorrectly colliding and causing confusing error messages. This changes the error handling when a short-name option is missing a value to show both the missing-value error _and_ a message about the unrecognized longer/composite option.

Fixes #709.

### Checklist
- [x] I've added at least one test that validates that my change is working, if appropriate
- [x] I've followed the code style of the rest of the project
- [x] I've read the [Contribution Guidelines](https://github.com/apple/swift-argument-parser/blob/main/CONTRIBUTING.md)
- [x] I've updated the documentation if necessary

Patch:
diff --git a/Sources/ArgumentParser/Parsing/ArgumentSet.swift b/Sources/ArgumentParser/Parsing/ArgumentSet.swift
index c07e76cf4..8ec45dd52 100644
--- a/Sources/ArgumentParser/Parsing/ArgumentSet.swift
+++ b/Sources/ArgumentParser/Parsing/ArgumentSet.swift
@@ -263,6 +263,25 @@ struct LenientParser {
     command.configuration.subcommands
   }
 
+  func errorForMissingValue(
+    _ originElement: InputOrigin.Element,
+    _ parsed: ParsedArgument
+  ) -> ParserError {
+    if case .argumentIndex(let index) = originElement,
+      index.subIndex != .complete,
+      let originalInput =
+        inputArguments
+        .originalInput(at: .argumentIndex(index.completeIndex))
+    {
+      let completeName = Name(originalInput[...])
+      return ParserError.missingValueOrUnknownCompositeOption(
+        InputOrigin(element: originElement), parsed.name, completeName)
+    } else {
+      return ParserError.missingValueForOption(
+        InputOrigin(element: originElement), parsed.name)
+    }
+  }
+
   mutating func parseValue(
     _ argument: ArgumentDefinition,
     _ parsed: ParsedArgument,
@@ -296,7 +315,7 @@ struct LenientParser {
         try update(origins, parsed.name, value, &result)
         usedOrigins.formUnion(origins)
       } else {
-        throw ParserError.missingValueForOption(origin, parsed.name)
+        throw errorForMissingValue(originElement, parsed)
       }
 
     case .scanningForValue:
@@ -322,7 +341,7 @@ struct LenientParser {
         try update(origins, parsed.name, value, &result)
         usedOrigins.formUnion(origins)
       } else {
-        throw ParserError.missingValueForOption(origin, parsed.name)
+        throw errorForMissingValue(originElement, parsed)
       }
 
     case .unconditional:
@@ -344,7 +363,7 @@ struct LenientParser {
           let (origin2, value) = inputArguments.popNextElementAsValue(
             after: originElement)
         else {
-          throw ParserError.missingValueForOption(origin, parsed.name)
+          throw errorForMissingValue(originElement, parsed)
         }
         let origins = origin.inserting(origin2)
         try update(origins, parsed.name, value, &result)
@@ -415,7 +434,7 @@ struct LenientParser {
         if foundAttachedValue {
           break
         } else {
-          throw ParserError.missingValueForOption(origin, parsed.name)
+          throw errorForMissingValue(originElement, parsed)
         }
       }
 
diff --git a/Sources/ArgumentParser/Parsing/InputOrigin.swift b/Sources/ArgumentParser/Parsing/InputOrigin.swift
index 69946e29a..b999edf4b 100644
--- a/Sources/ArgumentParser/Parsing/InputOrigin.swift
+++ b/Sources/ArgumentParser/Parsing/InputOrigin.swift
@@ -63,7 +63,11 @@ struct InputOrigin: Equatable, ExpressibleByArrayLiteral {
     Array(_elements).sorted()
   }
 
-  init() {
+  var firstElement: Element {
+    guard !elements.isEmpty else {
+      fatalError("Invalid 'InputOrigin' with no positions")
+    }
+    return elements[0]
   }
 
   init(elements: [Element]) {
diff --git a/Sources/ArgumentParser/Parsing/ParserError.swift b/Sources/ArgumentParser/Parsing/ParserError.swift
index 9e9129db7..18c28d2c9 100644
--- a/Sources/ArgumentParser/Parsing/ParserError.swift
+++ b/Sources/ArgumentParser/Parsing/ParserError.swift
@@ -26,6 +26,7 @@ enum ParserError: Error {
   case nonAlphanumericShortOption(Character)
   /// The option was there, but its value is missing, e.g. `--name` but no value for the `name`.
   case missingValueForOption(InputOrigin, Name)
+  case missingValueOrUnknownCompositeOption(InputOrigin, Name, Name)
   case unexpectedValueForOption(InputOrigin.Element, Name, String)
   case unexpectedExtraValues([(InputOrigin, String)])
   case duplicateExclusiveValues(
diff --git a/Sources/ArgumentParser/Usage/UsageGenerator.swift b/Sources/ArgumentParser/Usage/UsageGenerator.swift
index a3587a23a..338d49200 100644
--- a/Sources/ArgumentParser/Usage/UsageGenerator.swift
+++ b/Sources/ArgumentParser/Usage/UsageGenerator.swift
@@ -198,6 +198,10 @@ extension ErrorMessageGenerator {
       return unknownOptionMessage(origin: o, name: n)
     case .missingValueForOption(let o, let n):
       return missingValueForOptionMessage(origin: o, name: n)
+    case .missingValueOrUnknownCompositeOption(
+      let o, let shortName, let compositeName):
+      return missingValueOrUnknownCompositeOptionMessage(
+        origin: o, shortName: shortName, compositeName: compositeName)
     case .unexpectedValueForOption(let o, let n, let v):
       return unexpectedValueForOptionMessage(origin: o, name: n, value: v)
     case .unexpectedExtraValues(let v):
@@ -340,6 +344,23 @@ extension ErrorMessageGenerator {
     }
   }
 
+  func missingValueOrUnknownCompositeOptionMessage(
+    origin: InputOrigin,
+    shortName: Name,
+    compositeName: Name
+  ) -> String {
+    let unknownOptionMessage = unknownOptionMessage(
+      origin: origin.firstElement,
+      name: compositeName)
+    let missingValueMessage = missingValueForOptionMessage(
+      origin: origin,
+      name: shortName)
+    return """
+      \(unknownOptionMessage)
+         or: \(missingValueMessage) in '\(compositeName.synopsisString)'
+      """
+  }
+
   func unexpectedValueForOptionMessage(
     origin: InputOrigin.Element, name: Name, value: String
   ) -> String? {


Test patch:
diff --git a/Tests/ArgumentParserUnitTests/ParsableArgumentsValidationTests.swift b/Tests/ArgumentParserUnitTests/ParsableArgumentsValidationTests.swift
index 65c861dcc..ac7f21b21 100644
--- a/Tests/ArgumentParserUnitTests/ParsableArgumentsValidationTests.swift
+++ b/Tests/ArgumentParserUnitTests/ParsableArgumentsValidationTests.swift
@@ -535,3 +535,44 @@ final class ParsableArgumentsValidationTests: XCTestCase {
     }
   }
 }
+
+extension ParsableArgumentsValidationTests {
+  func testMissingValueForShortNameOptions() throws {
+    struct SomeArgs: ParsableArguments {
+      @Option(name: .shortAndLong)
+      var xArg: Int
+      @Option(name: .shortAndLong)
+      var zArg: Int
+      @Option(name: .customLong("long-with-x-or-y", withSingleDash: true))
+      var other: Int?
+    }
+
+    AssertErrorMessage(
+      SomeArgs.self,
+      ["-long_option_with_x_or_z"],
+      """
+      Unknown option '-long_option_with_x_or_z'
+         or: Missing value for '-x <x-arg>' in '-long_option_with_x_or_z'
+      """
+    )
+    // Including near-miss checking.
+    AssertErrorMessage(
+      SomeArgs.self,
+      ["-long-with-x-or-z"],
+      """
+      Unknown option '-long-with-x-or-z'. Did you mean '-long-with-x-or-y'?
+         or: Missing value for '-x <x-arg>' in '-long-with-x-or-z'
+      """
+    )
+    // Missing value for whole option.
+    AssertErrorMessage(
+      SomeArgs.self, ["-x", "-z", "2"],
+      "Missing value for '-x <x-arg>'"
+    )
+    // Standalone unexpected option.
+    AssertErrorMessage(
+      SomeArgs.self, ["-x", "1", "-z", "2", "-q"],
+      "Unknown option '-q'"
+    )
+  }
+}