diff --git a/Sources/Arguments.swift b/Sources/Arguments.swift index d2876f99..450fef5a 100644 --- a/Sources/Arguments.swift +++ b/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) } } } diff --git a/Sources/CommandLine.swift b/Sources/CommandLine.swift index 10ab5b68..522a9594 100644 --- a/Sources/CommandLine.swift +++ b/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[.. 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), diff --git a/Tests/ArgumentsTests.swift b/Tests/ArgumentsTests.swift index bbf6f466..7347a0c4 100644 --- a/Tests/ArgumentsTests.swift +++ b/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'?") } } diff --git a/Tests/CommandLineTests.swift b/Tests/CommandLineTests.swift index 868aa3fd..abcbff32 100644 --- a/Tests/CommandLineTests.swift +++ b/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() } diff --git a/Tests/FormatterTests.swift b/Tests/FormatterTests.swift index ede5d979..0f1bf9d3 100644 --- a/Tests/FormatterTests.swift +++ b/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" + """) } }