Show option-based errors before unexpected positional errors (#187)

This pushes any errors indicated by unexpected arguments after parsing
out to the same late position. We were previously stopping immediately
when the command is a leaf node; that isn't necessary and created an
awkward second error path.

Author: natecook1000

Sources/ArgumentParser/Parsing/ArgumentSet.swift

@@ -213,7 +213,7 @@ extension ArgumentSet {
   ///
   /// - Parameter all: The input (from the command line) that needs to be parsed
   /// - Parameter commandStack: commands that have been parsed
-  func lenientParse(_ all: SplitArguments) throws -> LenientParsedValues {
+  func lenientParse(_ all: SplitArguments) throws -> ParsedValues {
     // Create a local, mutable copy of the arguments:
     var inputArguments = all
 
@@ -355,21 +355,11 @@ extension ArgumentSet {
 
     // We have parsed all non-positional values at this point.
     // Next: parse / consume the positional values.
-    do {
-      var stripped = all
-      stripped.removeAll(in: usedOrigins)
-      try parsePositionalValues(from: stripped, into: &result)
-    } catch {
-      switch error {
-      case ParserError.unexpectedExtraValues:
-        // There were more positional values than we could parse.
-        // If we‘re using subcommands, that could be expected.
-        return .partial(result, error)
-      default:
-        throw error
-      }
-    }
-    return .success(result)
+    var unusedArguments = all
+    unusedArguments.removeAll(in: usedOrigins)
+    try parsePositionalValues(from: unusedArguments, into: &result)
+    
+    return result
   }
 }
 
@@ -450,16 +440,5 @@ extension ArgumentSet {
         try update([origin], nil, value, &result)
       } while argumentDefinition.isRepeatingPositional
     }
-    
-    // Finished with the defined arguments; are there leftover values to parse?
-    skipNonValues()
-    guard argumentStack.isEmpty else {
-      let extraValues: [(InputOrigin, String)] = argumentStack
-        .map { $0.0 }
-        .map {
-          (InputOrigin(element: $0), unusedInput.originalInput(at: $0)!)
-        }
-      throw ParserError.unexpectedExtraValues(extraValues)
-    }
   }
 }

Sources/ArgumentParser/Parsing/CommandParser.swift

@@ -110,19 +110,8 @@ extension CommandParser {
     // Build the argument set (i.e. information on how to parse):
     let commandArguments = ArgumentSet(currentNode.element)
 
-    // Parse the arguments into a ParsedValues:
-    let parsedResult = try commandArguments.lenientParse(split)
-    
-    let values: ParsedValues
-    switch parsedResult {
-    case .success(let v):
-      values = v
-    case .partial(let v, let e):
-      values = v
-      if currentNode.isLeaf {
-        throw e
-      }
-    }
+    // Parse the arguments, ignoring anything unexpected
+    let values = try commandArguments.lenientParse(split)
 
     // Decode the values from ParsedValues into the ParsableCommand:
     let decoder = ArgumentDecoder(values: values, previouslyDecoded: decodedArguments)

Sources/ArgumentParser/Parsing/ParsedValues.swift

@@ -43,11 +43,6 @@ struct ParsedValues {
   var originalInput: [String]
 }
 
-enum LenientParsedValues {
-  case success(ParsedValues)
-  case partial(ParsedValues, Swift.Error)
-}
-
 extension ParsedValues {
   mutating func set(_ new: Any, forKey key: InputKey, inputOrigin: InputOrigin) {
     set(Element(key: key, value: new, inputOrigin: inputOrigin))

Tests/ArgumentParserUnitTests/ErrorMessageTests.swift

@@ -174,3 +174,19 @@ extension ErrorMessageTests {
     AssertErrorMessage(OptOptions.self, ["-cbl"], "Value to be set with flag \'l\' in \'-cbl\' had already been set with flag \'c\' in \'-cbl\'")
   }
 }
+
+// MARK: -
+
+fileprivate struct Repeat: ParsableArguments {
+  @Option() var count: Int?
+  @Argument() var phrase: String
+}
+
+extension ErrorMessageTests {
+  func testBadOptionBeforeArgument() {
+    AssertErrorMessage(
+      Repeat.self,
+      ["--cont", "5", "Hello"],
+      "Unknown option '--cont'. Did you mean '--count'?")
+  }
+}