Fix confusing error message w/ single-dash option (#728)

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.

Author: natecook1000

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)
         }
       }
 

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]) {

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(

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? {

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'"
+    )
+  }
+}