Further improve error messaging for invalid options

Sources/Arguments.swift

@@ -83,7 +83,20 @@ extension Array where Element: Equatable {
 }
 
 extension String {
-    /// Find best match for the string in a list of options
+    /// Find single best match for the string in a list of options
+    /// If more than one match is equally good, return nil
+    func bestMatch(in options: [String]) -> String? {
+        let matches = bestMatches(in: options)
+        guard let best = matches.first else {
+            return nil
+        }
+        if matches.count > 1, editDistance(from: matches[1]) == editDistance(from: best) {
+            return nil
+        }
+        return best
+    }
+
+    /// Find best matches for the string in a list of options
     func bestMatches(in options: [String]) -> [String] {
         let lowercaseQuery = lowercased()
         // Sort matches by Levenshtein edit distance
@@ -305,10 +318,11 @@ func parseRules(_ rules: String) throws -> [String] {
             }
             throw FormatError.options("'\(proposedName)' is not a formatting rule")
         }
+        let message = "Unknown rule '\(proposedName)'"
         guard let match = proposedName.bestMatches(in: Array(allRules)).first else {
-            throw FormatError.options("Unknown rule '\(proposedName)'")
+            throw FormatError.options(message)
         }
-        throw FormatError.options("Unknown rule '\(proposedName)'. Did you mean '\(match)'?")
+        throw FormatError.options("\(message). Did you mean '\(match)'?")
     }
 }
 
@@ -675,12 +689,10 @@ public func applyFormatOptions(from args: [String: String], to formatOptions: in
             do {
                 try option.toOptions($0, &formatOptions)
             } catch {
-                if let argumentList = option.argumentList {
-                    throw FormatError.options("""
-                    Unsupported --\(option.argumentName) value '\($0)'. Valid options are \(argumentList)
-                    """)
+                guard let names = option.validArguments else {
+                    throw error
                 }
-                throw error
+                throw FormatError.invalidOption($0, for: option.argumentName, with: names)
             }
         }
     }

Sources/CommandLine.swift

@@ -78,9 +78,9 @@ private func print(_ message: String, as type: CLI.OutputType = .info) {
 private func printWarnings(_ errors: [Error]) -> Bool {
     var containsError = false
     for error in errors {
-        var errorMessage = "\(error)"
-        if !".?!".contains(errorMessage.last ?? " ") {
-            errorMessage += "."
+        var message = "\(error)"
+        if !".?!".contains(message.last ?? " ") {
+            message += "."
         }
         let isError: Bool
         switch error as? FormatError {
@@ -90,13 +90,13 @@ private func printWarnings(_ errors: [Error]) -> Bool {
             isError = true
         case nil:
             isError = true
-            errorMessage = error.localizedDescription
+            message = error.localizedDescription
         }
         if isError {
             containsError = true
-            print("error: \(errorMessage)", as: .error)
+            print("error: \(message)", as: .error)
         } else {
-            print("warning: \(errorMessage)", as: .warning)
+            print("warning: \(message)", as: .warning)
         }
     }
     return containsError
@@ -428,13 +428,12 @@ func processArguments(_ args: [String], environment: [String: String] = [:], in
                 named: identifier,
                 environment: environment
             ) else {
-                var message = "'\(identifier)' is not a valid reporter"
-                // swiftformat:disable:next --preferKeyPath
-                let names = Reporters.all.map { $0.name }
-                if let match = identifier.bestMatches(in: names).first {
-                    message += " (did you mean '\(match)'?)"
-                }
-                throw FormatError.options(message)
+                throw FormatError.invalidOption(
+                    identifier,
+                    for: "reporter",
+                    // swiftformat:disable:next --preferKeyPath
+                    with: Reporters.all.map { $0.name }
+                )
             }
             return reporter
         } ?? reportURL.flatMap {
@@ -894,11 +893,11 @@ func processArguments(_ args: [String], environment: [String: String] = [:], in
     } catch {
         _ = printWarnings(errors)
         // Fatal error
-        var errorMessage = "\(error)"
-        if ![".", "?", "!"].contains(errorMessage.last ?? " ") {
-            errorMessage += "."
+        var message = "\(error)"
+        if ![".", "?", "!"].contains(message.last ?? " ") {
+            message += "."
         }
-        print("error: \(errorMessage)", as: .error)
+        print("error: \(message)", as: .error)
         return .error
     }
 }
@@ -1278,20 +1277,25 @@ func processInput(_ inputURLs: [URL],
                 }
             } catch {
                 if verbose {
-                    var errorMessage = "\(error)"
-                    if !".?!".contains(errorMessage.last ?? " ") {
-                        errorMessage += "."
+                    var message = "\(error)"
+                    if !".?!".contains(message.last ?? " ") {
+                        message += "."
                     }
-                    print("-- error: \(errorMessage)", as: .error)
+                    print("-- error: \(message)", as: .error)
                 }
                 return {
                     outputFlags.filesChecked += 1
                     showConfigurationWarnings(options)
                     switch error {
-                    case let FormatError.parsing(string):
-                        throw FormatError.parsing("\(string) in \(inputURL.path)")
-                    case let FormatError.writing(string):
-                        throw FormatError.writing("\(string) in \(inputURL.path)")
+                    case let FormatError.parsing(message):
+                        if let range = message.range(of: ". Valid options") ?? message.range(of: ". Did you mean") {
+                            throw FormatError.parsing("""
+                            \(message[..<range.lowerBound]) in \(inputURL.path)\(message[range.lowerBound...])
+                            """)
+                        }
+                        throw FormatError.parsing("\(message) in \(inputURL.path)")
+                    case let FormatError.writing(message):
+                        throw FormatError.writing("\(message) in \(inputURL.path)")
                     default:
                         throw error
                     }

Sources/Formatter.swift

@@ -323,7 +323,7 @@ public class Formatter: NSObject {
     func fatalError(_ error: String, at tokenIndex: Int) {
         let line = originalLine(at: tokenIndex)
         var message: String
-        if let range = error.range(of: ". Valid options") {
+        if let range = error.range(of: ". Valid options") ?? error.range(of: ". Did you mean") {
             message = "\(error[..<range.lowerBound]) on line \(line)\(error[range.lowerBound...])"
         } else {
             message = "\(error) on line \(line)"

Sources/OptionDescriptor.swift

@@ -66,18 +66,23 @@ class OptionDescriptor {
         return true
     }
 
-    /// Formatted list of valid arguments (for boolean or enum-type options)
-    var argumentList: String? {
+    /// List of valid arguments (for boolean or enum-type options)
+    var validArguments: [String]? {
         switch type {
         case let .binary(true: trueValue, false: falseValue):
-            return [trueValue, falseValue].formattedList(default: defaultArgument)
+            return [trueValue, falseValue]
         case let .enum(values):
-            return values.formattedList(default: defaultArgument)
+            return values
         case .array, .set, .int, .text:
             return nil
         }
     }
 
+    /// Formatted list of valid arguments (for boolean or enum-type options)
+    var argumentList: String? {
+        validArguments?.formattedList(default: defaultArgument)
+    }
+
     /// Designated initializer
     private init(
         argumentName: String,
@@ -794,7 +799,7 @@ struct _Descriptors {
     let hoistPatternLet = OptionDescriptor(
         argumentName: "pattern-let",
         displayName: "Pattern Let",
-        help: "Placement of let/var in patterns: \"hoist\" (default) or \"inline\"",
+        help: "Placement of let/var in patterns:",
         keyPath: \.hoistPatternLet,
         trueValues: ["hoist"],
         falseValues: ["inline"]
@@ -965,15 +970,13 @@ struct _Descriptors {
         displayName: "Modifier Order",
         help: "Comma-delimited list of modifiers in preferred order",
         keyPath: \FormatOptions.modifierOrder,
-        validate: {
-            guard _FormatRules.mapModifiers($0) != nil else {
-                let names = _FormatRules.allModifiers
-                    + _FormatRules.semanticModifierGroups
-                let error = "'\($0)' is not a valid modifier"
-                guard let match = $0.bestMatches(in: names).first else {
-                    throw FormatError.options(error)
-                }
-                throw FormatError.options("\(error) (did you mean '\(match)'?)")
+        validate: { modifier in
+            guard _FormatRules.mapModifiers(modifier) != nil else {
+                throw FormatError.invalidOption(
+                    modifier,
+                    for: "modifier-order",
+                    with: _FormatRules.allModifiers + _FormatRules.semanticModifierGroups
+                )
             }
         }
     )
@@ -1087,11 +1090,11 @@ struct _Descriptors {
             }
             for type in order {
                 guard let concrete = VisibilityCategory(rawValue: type) else {
-                    let errorMessage = "'\(type)' is not a valid parameter for --visibility-order"
-                    guard let match = type.bestMatches(in: VisibilityCategory.allCases.map(\.rawValue)).first else {
-                        throw FormatError.options(errorMessage)
-                    }
-                    throw FormatError.options(errorMessage + ". Did you mean '\(match)?'")
+                    throw FormatError.invalidOption(
+                        type,
+                        for: "visibility-order",
+                        with: VisibilityCategory.allCases.map(\.rawValue)
+                    )
                 }
             }
         }
@@ -1101,15 +1104,13 @@ struct _Descriptors {
         displayName: "Organization Order For Declaration Types",
         help: "Order for declaration type groups inside declaration",
         keyPath: \.typeOrder,
-        validateArray: { order in
-            for type in order {
-                guard let concrete = DeclarationType(rawValue: type) else {
-                    let errorMessage = "'\(type)' is not a valid parameter for --type-order"
-                    guard let match = type.bestMatches(in: DeclarationType.allCases.map(\.rawValue)).first else {
-                        throw FormatError.options(errorMessage)
-                    }
-                    throw FormatError.options(errorMessage + ". Did you mean '\(match)?'")
-                }
+        validate: { type in
+            guard let concrete = DeclarationType(rawValue: type) else {
+                throw FormatError.invalidOption(
+                    type,
+                    for: "type-order",
+                    with: DeclarationType.allCases.map(\.rawValue)
+                )
             }
         }
     )
@@ -1238,7 +1239,7 @@ struct _Descriptors {
     let closureVoidReturn = OptionDescriptor(
         argumentName: "closure-void",
         displayName: "Closure Void",
-        help: "Explicit Void return types in closures: \"remove\" (default) or \"preserve\"",
+        help: "Explicit Void return types in closures:",
         keyPath: \.closureVoidReturn
     )
     let enumNamespaces = OptionDescriptor(
@@ -1452,7 +1453,7 @@ struct _Descriptors {
     let experimentalRules = OptionDescriptor(
         argumentName: "experimental",
         displayName: "Experimental Rules",
-        help: "Experimental rules: \"enabled\" or \"disabled\" (default)",
+        help: "Experimental rules:",
         deprecationMessage: "Use --enable to opt-in to rules individually.",
         keyPath: \.experimentalRules,
         trueValues: ["enabled", "true"],
@@ -1461,7 +1462,7 @@ struct _Descriptors {
     let varAttributes = OptionDescriptor(
         argumentName: "var-attributes",
         displayName: "Var Attributes",
-        help: "Property @attributes: \"preserve\", \"prev-line\", or \"same-line\"",
+        help: "Property @attributes:",
         deprecationMessage: "Use with `--storedvarattributes` or `--computedvarattributes` instead.",
         keyPath: \.varAttributes
     )

Sources/SwiftFormat.swift

@@ -78,6 +78,18 @@ public enum FormatError: Error, CustomStringConvertible, LocalizedError, CustomN
     case parsing(String)
     case options(String)
 
+    static func invalidOption(
+        _ option: String,
+        for argumentName: String,
+        with validOptions: [String]
+    ) -> Self {
+        let message = "Unsupported --\(argumentName) value '\(option)'"
+        guard let match = option.bestMatch(in: validOptions) else {
+            return .options("\(message). Valid options are \(validOptions.formattedList())")
+        }
+        return .options("\(message). Did you mean '\(match)'?")
+    }
+
     public var description: String {
         switch self {
         case let .reading(string),

Tests/ArgumentsTests.swift

@@ -808,7 +808,7 @@ class ArgumentsTests: XCTestCase {
 
     func testParseInvalidModifierOrderOption() throws {
         XCTAssertThrowsError(try Options(["modifier-order": "unknowned"], in: "")) { error in
-            XCTAssertEqual("\(error)", "'unknowned' is not a valid modifier (did you mean 'unowned'?) in --modifier-order")
+            XCTAssertEqual("\(error)", "Unsupported --modifier-order value 'unknowned'. Did you mean 'unowned'?")
         }
     }
 

Tests/CommandLineTests.swift

@@ -715,7 +715,7 @@ class CommandLineTests: XCTestCase {
                 case .raw, .warning, .info:
                     break
                 case .error:
-                    XCTAssert(message.contains("did you mean 'github-actions-log'?"))
+                    XCTAssert(message.contains("Did you mean 'github-actions-log'?"))
                 case .content, .success:
                     XCTFail()
                 }

Tests/FormatterTests.swift

@@ -449,9 +449,20 @@ class FormatterTests: XCTestCase {
         // swiftformat:options --else-position prev-line
         """
         XCTAssertThrowsError(try format(input, rules: FormatRules.default).output) { error in
-            XCTAssert("\(error)".hasPrefix("""
-            Unsupported --else-position value 'prev-line' on line 1. Valid options are 
-            """))
+            XCTAssertEqual("\(error)", """
+            Unsupported --else-position value 'prev-line' on line 1. Valid options are "same-line" or "next-line"
+            """)
+        }
+    }
+
+    func testInvalidEnumOptionValue2() {
+        let input = """
+        // swiftformat:options --else-position next
+        """
+        XCTAssertThrowsError(try format(input, rules: FormatRules.default).output) { error in
+            XCTAssertEqual("\(error)", """
+            Unsupported --else-position value 'next' on line 1. Did you mean 'next-line'?
+            """)
         }
     }
 
@@ -460,9 +471,9 @@ class FormatterTests: XCTestCase {
         // swiftformat:options --allman always
         """
         XCTAssertThrowsError(try format(input, rules: FormatRules.default).output) { error in
-            XCTAssert("\(error)".hasPrefix("""
-            Unsupported --allman value 'always' on line 1. Valid options are 
-            """))
+            XCTAssertEqual("\(error)", """
+            Unsupported --allman value 'always' on line 1. Valid options are "true" or "false"
+            """)
         }
     }