From 1fcd308cc4f339d03f2c1198190aca3ed1fcf4fc Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Mon, 2 Mar 2020 14:42:06 -0600 Subject: [PATCH] Add support for exiting without printing an error. This adds an `ExitCode` error type that stores an exit code, and updates the exit machinery to (1) use `ExitCode` as the source for exit codes, and (2) silently exit when the provided error is an `ExitCode` instance. --- Documentation/05 Validation and Errors.md | 22 +++++++++++- Examples/math/main.swift | 28 +++++++++++++++ Sources/ArgumentParser/CMakeLists.txt | 2 +- .../{ValidationError.swift => Errors.swift} | 31 ++++++++++++++++ .../Parsable Types/ParsableArguments.swift | 12 ++++--- .../ArgumentParser/Usage/MessageInfo.swift | 25 +++++++------ Sources/TestHelpers/TestHelpers.swift | 9 ++--- .../ValidationEndToEndTests.swift | 36 +++++++++++++++++-- Tests/ExampleTests/MathExampleTests.swift | 25 +++++++++++-- Tests/ExampleTests/RepeatExampleTests.swift | 6 ++-- Tests/ExampleTests/RollDiceExampleTests.swift | 4 +-- 11 files changed, 166 insertions(+), 34 deletions(-) rename Sources/ArgumentParser/Parsable Properties/{ValidationError.swift => Errors.swift} (70%) diff --git a/Documentation/05 Validation and Errors.md b/Documentation/05 Validation and Errors.md index ffe845383..b9bf00e55 100644 --- a/Documentation/05 Validation and Errors.md +++ b/Documentation/05 Validation and Errors.md @@ -57,7 +57,7 @@ hey ## Handling Post-Validation Errors -The `ValidationError` type is a special `ArgumentParser` error — a validation error's message is always accompanied by an appropriate usage string. You can throw other errors, from either the `validate()` or `run()` method to indicate that something has gone wrong that isn't validation-specific. +The `ValidationError` type is a special `ArgumentParser` error — a validation error's message is always accompanied by an appropriate usage string. You can throw other errors, from either the `validate()` or `run()` method to indicate that something has gone wrong that isn't validation-specific. Errors that conform to `CustomStringConvertible` or `LocalizedError` provide the best experience for users. ```swift struct LineCount: ParsableCommand { @@ -80,3 +80,23 @@ The throwing `String(contentsOfFile:encoding:)` initializer fails when the user Error: The file “non-existing-file.swift” couldn’t be opened because there is no such file. ``` + +If you print your error output yourself, you still need to throw an error from `validate()` or `run()`, so that your command exits with the appropriate exit code. To avoid printing an extra error message, use the `ExitCode` error, which has static properties for success, failure, and validation errors, or lets you specify a specific exit code. + +```swift +struct RuntimeError: Error, CustomStringConvertible { + var description: String +} + +struct Example: ParsableCommand { + @Argument() var inputFile: String + + func run() throws { + if !ExampleCore.processFile(inputFile) { + // ExampleCore.processFile(_:) prints its own errors + // and returns `false` on failure. + throw ExitCode.failure + } + } +} +``` diff --git a/Examples/math/main.swift b/Examples/math/main.swift index 4d3dcfd4e..379a66594 100644 --- a/Examples/math/main.swift +++ b/Examples/math/main.swift @@ -186,6 +186,34 @@ extension Math.Statistics { @Argument(help: "A group of floating-point values to operate on.") var values: [Double] + + // These args and the validation method are for testing exit codes: + @Flag(help: .hidden) + var testSuccessExitCode: Bool + @Flag(help: .hidden) + var testFailureExitCode: Bool + @Flag(help: .hidden) + var testValidationExitCode: Bool + @Option(help: .hidden) + var testCustomExitCode: Int32? + + func validate() throws { + if testSuccessExitCode { + throw ExitCode.success + } + + if testFailureExitCode { + throw ExitCode.failure + } + + if testValidationExitCode { + throw ExitCode.validationFailure + } + + if let exitCode = testCustomExitCode { + throw ExitCode(exitCode) + } + } } } diff --git a/Sources/ArgumentParser/CMakeLists.txt b/Sources/ArgumentParser/CMakeLists.txt index bf6715e6d..11e3c4937 100644 --- a/Sources/ArgumentParser/CMakeLists.txt +++ b/Sources/ArgumentParser/CMakeLists.txt @@ -1,11 +1,11 @@ add_library(ArgumentParser "Parsable Properties/Argument.swift" "Parsable Properties/ArgumentHelp.swift" + "Parsable Properties/Errors.swift" "Parsable Properties/Flag.swift" "Parsable Properties/NameSpecification.swift" "Parsable Properties/Option.swift" "Parsable Properties/OptionGroup.swift" - "Parsable Properties/ValidationError.swift" "Parsable Types/CommandConfiguration.swift" "Parsable Types/ExpressibleByArgument.swift" diff --git a/Sources/ArgumentParser/Parsable Properties/ValidationError.swift b/Sources/ArgumentParser/Parsable Properties/Errors.swift similarity index 70% rename from Sources/ArgumentParser/Parsable Properties/ValidationError.swift rename to Sources/ArgumentParser/Parsable Properties/Errors.swift index ac6ef6ae2..47c9bc564 100644 --- a/Sources/ArgumentParser/Parsable Properties/ValidationError.swift +++ b/Sources/ArgumentParser/Parsable Properties/Errors.swift @@ -9,6 +9,14 @@ // //===----------------------------------------------------------------------===// +#if canImport(Glibc) +import Glibc +#elseif canImport(Darwin) +import Darwin +#elseif canImport(MSVCRT) +import MSVCRT +#endif + /// An error type that is presented to the user as an error with parsing their /// command-line input. public struct ValidationError: Error, CustomStringConvertible { @@ -24,6 +32,29 @@ public struct ValidationError: Error, CustomStringConvertible { } } +/// An error type that only includes an exit code. +/// +/// If you're printing custom errors messages yourself, you can throw this error +/// to specify the exit code without adding any additional output to standard +/// out or standard error. +public struct ExitCode: Error { + var code: Int32 + + /// Creates a new `ExitCode` with the given code. + public init(_ code: Int32) { + self.code = code + } + + /// An exit code that indicates successful completion of a command. + public static let success = ExitCode(EXIT_SUCCESS) + + /// An exit code that indicates that the command failed. + public static let failure = ExitCode(EXIT_FAILURE) + + /// An exit code that indicates that the user provided invalid input. + public static let validationFailure = ExitCode(EX_USAGE) +} + /// An error type that represents a clean (i.e. non-error state) exit of the /// utility. /// diff --git a/Sources/ArgumentParser/Parsable Types/ParsableArguments.swift b/Sources/ArgumentParser/Parsable Types/ParsableArguments.swift index 6091038b5..bf8816242 100644 --- a/Sources/ArgumentParser/Parsable Types/ParsableArguments.swift +++ b/Sources/ArgumentParser/Parsable Types/ParsableArguments.swift @@ -139,14 +139,16 @@ extension ParsableArguments { withError error: Error? = nil ) -> Never { guard let error = error else { - _exit(EXIT_SUCCESS) + _exit(ExitCode.success.code) } let messageInfo = MessageInfo(error: error, type: self) - if messageInfo.shouldExitCleanly { - print(messageInfo.fullText) - } else { - print(messageInfo.fullText, to: &standardError) + if !messageInfo.fullText.isEmpty { + if messageInfo.shouldExitCleanly { + print(messageInfo.fullText) + } else { + print(messageInfo.fullText, to: &standardError) + } } _exit(messageInfo.exitCode) } diff --git a/Sources/ArgumentParser/Usage/MessageInfo.swift b/Sources/ArgumentParser/Usage/MessageInfo.swift index bbee2a28b..1bf1b5422 100644 --- a/Sources/ArgumentParser/Usage/MessageInfo.swift +++ b/Sources/ArgumentParser/Usage/MessageInfo.swift @@ -14,7 +14,7 @@ enum MessageInfo { case help(text: String) case validation(message: String, usage: String) - case other(message: String) + case other(message: String, exitCode: Int32) init(error: Error, type: ParsableArguments.Type) { var commandStack: [ParsableCommand.Type] @@ -61,16 +61,18 @@ enum MessageInfo { case .message(let message): self = .help(text: message) } + case let error as ExitCode: + self = .other(message: "", exitCode: error.code) case let error as LocalizedError where error.errorDescription != nil: - self = .other(message: error.errorDescription!) + self = .other(message: error.errorDescription!, exitCode: EXIT_FAILURE) default: - self = .other(message: String(describing: error)) + self = .other(message: String(describing: error), exitCode: EXIT_FAILURE) } } else if let parserError = parserError { let message = ArgumentSet(commandStack.last!).helpMessage(for: parserError) self = .validation(message: message, usage: usage) } else { - self = .other(message: String(describing: error)) + self = .other(message: String(describing: error), exitCode: EXIT_FAILURE) } } @@ -80,7 +82,7 @@ enum MessageInfo { return text case .validation(message: let message, usage: _): return message - case .other(message: let message): + case .other(let message, _): return message } } @@ -90,9 +92,10 @@ enum MessageInfo { case .help(text: let text): return text case .validation(message: let message, usage: let usage): - return "Error: \(message)\n\(usage)" - case .other(message: let message): - return "Error: \(message)" + let errorMessage = message.isEmpty ? "" : "Error: \(message)\n" + return errorMessage + usage + case .other(let message, _): + return message.isEmpty ? "" : "Error: \(message)" } } @@ -105,9 +108,9 @@ enum MessageInfo { var exitCode: Int32 { switch self { - case .help: return EXIT_SUCCESS - case .validation: return EX_USAGE - case .other: return EXIT_FAILURE + case .help: return ExitCode.success.code + case .validation: return ExitCode.validationFailure.code + case .other(_, let exitCode): return exitCode } } } diff --git a/Sources/TestHelpers/TestHelpers.swift b/Sources/TestHelpers/TestHelpers.swift index e5ce6ff4b..6caa691b7 100644 --- a/Sources/TestHelpers/TestHelpers.swift +++ b/Sources/TestHelpers/TestHelpers.swift @@ -136,7 +136,7 @@ extension XCTest { public func AssertExecuteCommand( command: String, expected: String? = nil, - shouldError: Bool = false, + exitCode: Int32 = 0, file: StaticString = #file, line: UInt = #line) { let splitCommand = command.split(separator: " ") @@ -171,10 +171,7 @@ extension XCTest { if let expected = expected { AssertEqualStringsIgnoringTrailingWhitespace(expected, errorActual + outputActual, file: file, line: line) } - if shouldError { - XCTAssertNotEqual(process.terminationStatus, 0, file: file, line: line) - } else { - XCTAssertEqual(process.terminationStatus, 0, file: file, line: line) - } + + XCTAssertEqual(process.terminationStatus, exitCode, file: file, line: line) } } diff --git a/Tests/EndToEndTests/ValidationEndToEndTests.swift b/Tests/EndToEndTests/ValidationEndToEndTests.swift index 473fb9a59..8854c73ce 100644 --- a/Tests/EndToEndTests/ValidationEndToEndTests.swift +++ b/Tests/EndToEndTests/ValidationEndToEndTests.swift @@ -28,6 +28,10 @@ fileprivate enum UserValidationError: LocalizedError { } fileprivate struct Foo: ParsableArguments { + static var usageString: String = """ + Usage: foo [--count ] [ ...] [--version] [--throw] + """ + @Option() var count: Int? @@ -40,6 +44,15 @@ fileprivate struct Foo: ParsableArguments { @Flag(name: [.customLong("throw")]) var throwCustomError: Bool + @Flag(help: .hidden) + var showUsageOnly: Bool + + @Flag(help: .hidden) + var failValidationSilently: Bool + + @Flag(help: .hidden) + var failSilently: Bool + mutating func validate() throws { if version { throw CleanExit.message("0.0.1") @@ -56,6 +69,18 @@ fileprivate struct Foo: ParsableArguments { if throwCustomError { throw UserValidationError.userValidationError } + + if showUsageOnly { + throw ValidationError("") + } + + if failValidationSilently { + throw ExitCode.validationFailure + } + + if failSilently { + throw ExitCode.failure + } } } @@ -81,7 +106,7 @@ extension ValidationEndToEndTests { AssertErrorMessage(Foo.self, [], "Must specify at least one name.") AssertFullErrorMessage(Foo.self, [], """ Error: Must specify at least one name. - Usage: foo [--count ] [ ...] [--version] [--throw] + \(Foo.usageString) """) AssertErrorMessage(Foo.self, ["--count", "3", "Joe"], """ @@ -89,7 +114,7 @@ extension ValidationEndToEndTests { """) AssertFullErrorMessage(Foo.self, ["--count", "3", "Joe"], """ Error: Number of names (1) doesn't match count (3). - Usage: foo [--count ] [ ...] [--version] [--throw] + \(Foo.usageString) """) } @@ -97,4 +122,11 @@ extension ValidationEndToEndTests { // verify that error description is printed if avaiable via LocalizedError AssertErrorMessage(Foo.self, ["--throw", "Joe"], UserValidationError.userValidationError.errorDescription!) } + + func testEmptyErrorValidation() { + AssertErrorMessage(Foo.self, ["--show-usage-only", "Joe"], "") + AssertFullErrorMessage(Foo.self, ["--show-usage-only", "Joe"], Foo.usageString) + AssertFullErrorMessage(Foo.self, ["--fail-validation-silently", "Joe"], "") + AssertFullErrorMessage(Foo.self, ["--fail-silently", "Joe"], "") + } } diff --git a/Tests/ExampleTests/MathExampleTests.swift b/Tests/ExampleTests/MathExampleTests.swift index b090c2793..8758de642 100644 --- a/Tests/ExampleTests/MathExampleTests.swift +++ b/Tests/ExampleTests/MathExampleTests.swift @@ -105,9 +105,28 @@ final class MathExampleTests: XCTestCase { Error: Please provide at least one value to calculate the mode. Usage: math stats average [--kind ] [ ...] """, - shouldError: true) + exitCode: EX_USAGE) } + func testMath_ExitCodes() throws { + AssertExecuteCommand( + command: "math stats quantiles --test-success-exit-code", + expected: "", + exitCode: EXIT_SUCCESS) + AssertExecuteCommand( + command: "math stats quantiles --test-failure-exit-code", + expected: "", + exitCode: EXIT_FAILURE) + AssertExecuteCommand( + command: "math stats quantiles --test-validation-exit-code", + expected: "", + exitCode: EX_USAGE) + AssertExecuteCommand( + command: "math stats quantiles --test-custom-exit-code 42", + expected: "", + exitCode: 42) + } + func testMath_Fail() throws { AssertExecuteCommand( command: "math --foo", @@ -115,7 +134,7 @@ final class MathExampleTests: XCTestCase { Error: Unknown option '--foo' Usage: math add [--hex-output] [ ...] """, - shouldError: true) + exitCode: EX_USAGE) AssertExecuteCommand( command: "math ZZZ", @@ -123,6 +142,6 @@ final class MathExampleTests: XCTestCase { Error: The value 'ZZZ' is invalid for '' Usage: math add [--hex-output] [ ...] """, - shouldError: true) + exitCode: EX_USAGE) } } diff --git a/Tests/ExampleTests/RepeatExampleTests.swift b/Tests/ExampleTests/RepeatExampleTests.swift index 6da5b2e04..4bbf4eb96 100644 --- a/Tests/ExampleTests/RepeatExampleTests.swift +++ b/Tests/ExampleTests/RepeatExampleTests.swift @@ -48,7 +48,7 @@ final class RepeatExampleTests: XCTestCase { Error: Missing expected argument '' Usage: repeat [--count ] [--include-counter] """, - shouldError: true) + exitCode: EX_USAGE) AssertExecuteCommand( command: "repeat hello --count", @@ -56,7 +56,7 @@ final class RepeatExampleTests: XCTestCase { Error: Missing value for '--count ' Usage: repeat [--count ] [--include-counter] """, - shouldError: true) + exitCode: EX_USAGE) AssertExecuteCommand( command: "repeat hello --count ZZZ", @@ -64,6 +64,6 @@ final class RepeatExampleTests: XCTestCase { Error: The value 'ZZZ' is invalid for '--count ' Usage: repeat [--count ] [--include-counter] """, - shouldError: true) + exitCode: EX_USAGE) } } diff --git a/Tests/ExampleTests/RollDiceExampleTests.swift b/Tests/ExampleTests/RollDiceExampleTests.swift index aa80d3155..d9e3ce4a1 100644 --- a/Tests/ExampleTests/RollDiceExampleTests.swift +++ b/Tests/ExampleTests/RollDiceExampleTests.swift @@ -41,7 +41,7 @@ final class RollDiceExampleTests: XCTestCase { Error: Missing value for '--times ' Usage: roll [--times ] [--sides ] [--seed ] [--verbose] """, - shouldError: true) + exitCode: EX_USAGE) AssertExecuteCommand( command: "roll --times ZZZ", @@ -49,6 +49,6 @@ final class RollDiceExampleTests: XCTestCase { Error: The value 'ZZZ' is invalid for '--times ' Usage: roll [--times ] [--sides ] [--seed ] [--verbose] """, - shouldError: true) + exitCode: EX_USAGE) } }