Skip to content

Include help text in error message when validation fails #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions Sources/ArgumentParser/Usage/MessageInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

enum MessageInfo {
case help(text: String)
case validation(message: String, usage: String)
case validation(message: String, usage: String, help: String)
case other(message: String, exitCode: Int32)

init(error: Error, type: ParsableArguments.Type) {
Expand Down Expand Up @@ -82,7 +82,7 @@ enum MessageInfo {
if case .userValidationError(let error) = parserError {
switch error {
case let error as ValidationError:
self = .validation(message: error.message, usage: usage)
self = .validation(message: error.message, usage: usage, help: "")
case let error as CleanExit:
switch error {
case .helpRequest(let command):
Expand All @@ -109,8 +109,10 @@ enum MessageInfo {
guard case ParserError.noArguments = parserError else { return usage }
return "\n" + HelpGenerator(commandStack: [type.asCommand]).rendered()
}()
let message = ArgumentSet(commandStack.last!).helpMessage(for: parserError)
self = .validation(message: message, usage: usage)
let argumentSet = ArgumentSet(commandStack.last!)
let message = argumentSet.errorDescription(error: parserError) ?? ""
let helpAbstract = argumentSet.helpDescription(error: parserError) ?? ""
self = .validation(message: message, usage: usage, help: helpAbstract)
} else {
self = .other(message: String(describing: error), exitCode: EXIT_FAILURE)
}
Expand All @@ -120,7 +122,7 @@ enum MessageInfo {
switch self {
case .help(text: let text):
return text
case .validation(message: let message, usage: _):
case .validation(message: let message, usage: _, help: _):
return message
case .other(let message, _):
return message
Expand All @@ -131,9 +133,10 @@ enum MessageInfo {
switch self {
case .help(text: let text):
return text
case .validation(message: let message, usage: let usage):
case .validation(message: let message, usage: let usage, help: let help):
let helpMessage = help.isEmpty ? "" : "Help: \(help)\n"
let errorMessage = message.isEmpty ? "" : "\(args._errorLabel): \(message)\n"
return errorMessage + usage
return errorMessage + helpMessage + usage
case .other(let message, _):
return message.isEmpty ? "" : "\(args._errorLabel): \(message)"
}
Expand Down
41 changes: 37 additions & 4 deletions Sources/ArgumentParser/Usage/UsageGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,6 @@ extension ArgumentDefinition {
}

extension ArgumentSet {
func helpMessage(for error: Swift.Error) -> String {
return errorDescription(error: error) ?? ""
}

/// Will generate a descriptive help message if possible.
///
/// If no descriptive help message can be generated, `nil` will be returned.
Expand All @@ -163,6 +159,19 @@ extension ArgumentSet {
return nil
}
}

func helpDescription(error: Swift.Error) -> String? {
switch error {
case let parserError as ParserError:
return ErrorMessageGenerator(arguments: self, error: parserError)
.makeHelpMessage()
case let commandError as CommandError:
return ErrorMessageGenerator(arguments: self, error: commandError.parserError)
.makeHelpMessage()
default:
return nil
}
}
}

struct ErrorMessageGenerator {
Expand Down Expand Up @@ -223,6 +232,15 @@ extension ErrorMessageGenerator {
}
}
}

func makeHelpMessage() -> String? {
switch error {
case .unableToParseValue(let o, let n, let v, forKey: let k, originalError: let e):
return unableToParseHelpMessage(origin: o, name: n, value: v, key: k, error: e)
default:
return nil
}
}
}

extension ErrorMessageGenerator {
Expand Down Expand Up @@ -363,6 +381,21 @@ extension ErrorMessageGenerator {
}
}

func unableToParseHelpMessage(origin: InputOrigin, name: Name?, value: String, key: InputKey, error: Error?) -> String {
guard let abstract = help(for: key)?.help?.abstract else { return "" }

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

switch (name, valueName) {
case let (n?, v?):
return "\(n.synopsisString) <\(v)> \(abstract)"
case let (_, v?):
return "<\(v)> \(abstract)"
case (_, _):
return ""
}
}

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

Expand Down
20 changes: 12 additions & 8 deletions Tests/ArgumentParserEndToEndTests/TransformEndToEndTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fileprivate struct FooOption: Convert, ParsableArguments {
Usage: foo_option --string <int_str>
See 'foo_option --help' for more information.
"""
static var help: String = "Help: --string <int_str> Convert string to integer\n"

@Option(help: ArgumentHelp("Convert string to integer", valueName: "int_str"),
transform: { try convert($0) })
Expand All @@ -52,6 +53,7 @@ fileprivate struct BarOption: Convert, ParsableCommand {
Usage: bar-option [--strings <int_str> ...]
See 'bar-option --help' for more information.
"""
static var help: String = "Help: --strings <int_str> Convert a list of strings to an array of integers\n"

@Option(help: ArgumentHelp("Convert a list of strings to an array of integers", valueName: "int_str"),
transform: { try convert($0) })
Expand All @@ -69,11 +71,11 @@ extension TransformEndToEndTests {
}

func testSingleOptionValidation_Fail_CustomErrorMessage() throws {
AssertFullErrorMessage(FooOption.self, ["--string", "Forty Two"], "Error: The value 'Forty Two' is invalid for '--string <int_str>': Could not transform to an Int.\n" + FooOption.usageString)
AssertFullErrorMessage(FooOption.self, ["--string", "Forty Two"], "Error: The value 'Forty Two' is invalid for '--string <int_str>': Could not transform to an Int.\n" + FooOption.help + FooOption.usageString)
}

func testSingleOptionValidation_Fail_DefaultErrorMessage() throws {
AssertFullErrorMessage(FooOption.self, ["--string", "4827"], "Error: The value '4827' is invalid for '--string <int_str>': outOfBounds\n" + FooOption.usageString)
AssertFullErrorMessage(FooOption.self, ["--string", "4827"], "Error: The value '4827' is invalid for '--string <int_str>': outOfBounds\n" + FooOption.help + FooOption.usageString)
}

// MARK: Arrays
Expand All @@ -85,11 +87,11 @@ extension TransformEndToEndTests {
}

func testOptionArrayValidation_Fail_CustomErrorMessage() throws {
AssertFullErrorMessage(BarOption.self, ["--strings", "Forty Two", "--strings", "72", "--strings", "99"], "Error: The value 'Forty Two' is invalid for '--strings <int_str>': Could not transform to an Int.\n" + BarOption.usageString)
AssertFullErrorMessage(BarOption.self, ["--strings", "Forty Two", "--strings", "72", "--strings", "99"], "Error: The value 'Forty Two' is invalid for '--strings <int_str>': Could not transform to an Int.\n" + BarOption.help + BarOption.usageString)
}

func testOptionArrayValidation_Fail_DefaultErrorMessage() throws {
AssertFullErrorMessage(BarOption.self, ["--strings", "4827", "--strings", "72", "--strings", "99"], "Error: The value '4827' is invalid for '--strings <int_str>': outOfBounds\n" + BarOption.usageString)
AssertFullErrorMessage(BarOption.self, ["--strings", "4827", "--strings", "72", "--strings", "99"], "Error: The value '4827' is invalid for '--strings <int_str>': outOfBounds\n" + BarOption.help + BarOption.usageString)
}
}

Expand All @@ -101,6 +103,7 @@ fileprivate struct FooArgument: Convert, ParsableArguments {
Usage: foo_argument <int_str>
See 'foo_argument --help' for more information.
"""
static var help: String = "Help: <int_str> Convert string to integer\n"

enum FooError: Error {
case outOfBounds
Expand All @@ -117,6 +120,7 @@ fileprivate struct BarArgument: Convert, ParsableCommand {
Usage: bar-argument [<int_str> ...]
See 'bar-argument --help' for more information.
"""
static var help: String = "Help: <int_str> Convert a list of strings to an array of integers\n"

@Argument(help: ArgumentHelp("Convert a list of strings to an array of integers", valueName: "int_str"),
transform: { try convert($0) })
Expand All @@ -134,11 +138,11 @@ extension TransformEndToEndTests {
}

func testArgumentValidation_Fail_CustomErrorMessage() throws {
AssertFullErrorMessage(FooArgument.self, ["Forty Two"], "Error: The value 'Forty Two' is invalid for '<int_str>': Could not transform to an Int.\n" + FooArgument.usageString)
AssertFullErrorMessage(FooArgument.self, ["Forty Two"], "Error: The value 'Forty Two' is invalid for '<int_str>': Could not transform to an Int.\n" + FooArgument.help + FooArgument.usageString)
}

func testArgumentValidation_Fail_DefaultErrorMessage() throws {
AssertFullErrorMessage(FooArgument.self, ["4827"], "Error: The value '4827' is invalid for '<int_str>': outOfBounds\n" + FooArgument.usageString)
AssertFullErrorMessage(FooArgument.self, ["4827"], "Error: The value '4827' is invalid for '<int_str>': outOfBounds\n" + FooArgument.help + FooArgument.usageString)
}

// MARK: Arrays
Expand All @@ -150,10 +154,10 @@ extension TransformEndToEndTests {
}

func testArgumentArrayValidation_Fail_CustomErrorMessage() throws {
AssertFullErrorMessage(BarArgument.self, ["Forty Two", "72", "99"], "Error: The value 'Forty Two' is invalid for '<int_str>': Could not transform to an Int.\n" + BarArgument.usageString)
AssertFullErrorMessage(BarArgument.self, ["Forty Two", "72", "99"], "Error: The value 'Forty Two' is invalid for '<int_str>': Could not transform to an Int.\n" + BarArgument.help + BarArgument.usageString)
}

func testArgumentArrayValidation_Fail_DefaultErrorMessage() throws {
AssertFullErrorMessage(BarArgument.self, ["4827", "72", "99"], "Error: The value '4827' is invalid for '<int_str>': outOfBounds\n" + BarArgument.usageString)
AssertFullErrorMessage(BarArgument.self, ["4827", "72", "99"], "Error: The value '4827' is invalid for '<int_str>': outOfBounds\n" + BarArgument.help + BarArgument.usageString)
}
}
1 change: 1 addition & 0 deletions Tests/ArgumentParserExampleTests/MathExampleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ final class MathExampleTests: XCTestCase {
command: "math ZZZ",
expected: """
Error: The value 'ZZZ' is invalid for '<values>'
Help: <values> A group of integers to operate on.
Usage: math add [--hex-output] [<values> ...]
See 'math add --help' for more information.
""",
Expand Down
1 change: 1 addition & 0 deletions Tests/ArgumentParserExampleTests/RepeatExampleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ final class RepeatExampleTests: XCTestCase {
command: "repeat hello --count ZZZ",
expected: """
Error: The value 'ZZZ' is invalid for '--count <count>'
Help: --count <count> The number of times to repeat 'phrase'.
Usage: repeat [--count <count>] [--include-counter] <phrase>
See 'repeat --help' for more information.
""",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ final class RollDiceExampleTests: XCTestCase {
command: "roll --times ZZZ",
expected: """
Error: The value 'ZZZ' is invalid for '--times <n>'
Help: --times <n> Rolls the dice <n> times.
Usage: roll [--times <n>] [--sides <m>] [--seed <seed>] [--verbose]
See 'roll --help' for more information.
""",
Expand Down