Skip to content

Commit

Permalink
List valid options in error messages (#382)
Browse files Browse the repository at this point in the history
When an option value fails to parse, no custom error message is
provided, and a list of valid candidate values is available, include the
list as part of the error message.

Addresses #344.
  • Loading branch information
dduan committed Jan 5, 2022
1 parent e73578d commit 90f76c1
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 7 deletions.
31 changes: 27 additions & 4 deletions Sources/ArgumentParser/Usage/UsageGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,9 @@ extension ErrorMessageGenerator {
}

func unableToParseValueMessage(origin: InputOrigin, name: Name?, value: String, key: InputKey, error: Error?) -> String {
let valueName = arguments(for: key).first?.valueName

let argumentValue = arguments(for: key).first
let valueName = argumentValue?.valueName

// We want to make the "best effort" in producing a custom error message.
// We favour `LocalizedError.errorDescription` and fall back to
// `CustomStringConvertible`. To opt in, return your custom error message
Expand All @@ -407,10 +408,10 @@ extension ErrorMessageGenerator {
case let err?:
return ": " + String(describing: err)
default:
return ""
return argumentValue?.formattedValueList ?? ""
}
}()

switch (name, valueName) {
case let (n?, v?):
return "The value '\(value)' is invalid for '\(n.synopsisString) <\(v)>'\(customErrorMessage)"
Expand All @@ -423,3 +424,25 @@ extension ErrorMessageGenerator {
}
}
}

private extension ArgumentDefinition {
var formattedValueList: String {
if help.allValues.isEmpty {
return ""
}

if help.allValues.count < 6 {
let quotedValues = help.allValues.map { "'\($0)'" }
let validList: String
if quotedValues.count <= 2 {
validList = quotedValues.joined(separator: " and ")
} else {
validList = quotedValues.dropLast().joined(separator: ", ") + " or \(quotedValues.last!)"
}
return ". Please provide one of \(validList)."
} else {
let bulletValueList = help.allValues.map { " - \($0)" }.joined(separator: "\n")
return ". Please provide one of the following:\n\(bulletValueList)"
}
}
}
41 changes: 38 additions & 3 deletions Tests/ArgumentParserUnitTests/ErrorMessageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,53 @@ extension ErrorMessageTests {
}

fileprivate struct Foo: ParsableArguments {
enum Format: String, Equatable, Decodable, ExpressibleByArgument {
enum Format: String, Equatable, Decodable, ExpressibleByArgument, CaseIterable {
case text
case json
case csv
}

enum Name: String, Equatable, Decodable, ExpressibleByArgument, CaseIterable {
case bruce
case clint
case hulk
case natasha
case steve
case thor
case tony
}
@Option(name: [.short, .long])
var format: Format
@Option(name: [.short, .long])
var name: Name?
}

extension ErrorMessageTests {
func testWrongEnumValue() {
AssertErrorMessage(Foo.self, ["--format", "png"], "The value 'png' is invalid for '--format <format>'")
AssertErrorMessage(Foo.self, ["-f", "png"], "The value 'png' is invalid for '-f <format>'")
AssertErrorMessage(Foo.self, ["--format", "png"], "The value 'png' is invalid for '--format <format>'. Please provide one of 'text', 'json' or 'csv'.")
AssertErrorMessage(Foo.self, ["-f", "png"], "The value 'png' is invalid for '-f <format>'. Please provide one of 'text', 'json' or 'csv'.")
AssertErrorMessage(Foo.self, ["-f", "text", "--name", "loki"],
"""
The value 'loki' is invalid for '--name <name>'. Please provide one of the following:
- bruce
- clint
- hulk
- natasha
- steve
- thor
- tony
""")
AssertErrorMessage(Foo.self, ["-f", "text", "-n", "loki"],
"""
The value 'loki' is invalid for '-n <name>'. Please provide one of the following:
- bruce
- clint
- hulk
- natasha
- steve
- thor
- tony
""")
}
}

Expand Down

0 comments on commit 90f76c1

Please sign in to comment.