Skip to content

Diagnose unsupported options and add descriptions for option-parsing errors #39

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 2 commits into from
Dec 16, 2019
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
6 changes: 3 additions & 3 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ public struct Driver {
public init(
args: [String],
env: [String: String] = ProcessEnv.vars,
diagnosticsHandler: @escaping DiagnosticsEngine.DiagnosticsHandler = Driver.stderrDiagnosticsHandler
diagnosticsEngine: DiagnosticsEngine = DiagnosticsEngine(handlers: [Driver.stderrDiagnosticsHandler])
) throws {
self.env = env

self.diagnosticEngine = DiagnosticsEngine(handlers: [diagnosticsHandler])
self.diagnosticEngine = diagnosticsEngine

if case .subcommand = try Self.invocationRunMode(forArgs: args).mode {
throw Error.subcommandPassedToDriver
Expand All @@ -205,7 +205,7 @@ public struct Driver {

self.driverKind = try Self.determineDriverKind(args: &args)
self.optionTable = OptionTable()
self.parsedOptions = try optionTable.parse(Array(args), forInteractiveMode: self.driverKind == .interactive)
self.parsedOptions = try optionTable.parse(Array(args), for: self.driverKind)

let explicitTarget = (self.parsedOptions.getLastArgument(.target)?.asSingle)
.map {
Expand Down
30 changes: 27 additions & 3 deletions Sources/SwiftDriver/Options/OptionParsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,34 @@
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//
public enum OptionParseError : Error, Equatable {
import TSCBasic

public enum OptionParseError : Error, Equatable, DiagnosticData {
case unknownOption(index: Int, argument: String)
case missingArgument(index: Int, argument: String)
case unsupportedOption(index: Int, argument: String, option: Option, currentDriverKind: DriverKind)

public var description: String {
switch self {
case let .unknownOption(index: _, argument: arg):
return "unknown argument: '\(arg)'"
case let .missingArgument(index: _, argument: arg):
return "missing argument value for '\(arg)'"
case let .unsupportedOption(index: _, argument: arg, option: option, currentDriverKind: driverKind):
// TODO: This logic to choose the recommended kind is copied from the C++
// driver and could be improved.
let recommendedDriverKind: DriverKind = option.attributes.contains(.noBatch) ? .interactive : .batch
return "option '\(arg)' is not supported by '\(driverKind.usage)'; did you mean to use '\(recommendedDriverKind.usage)'?"
}
}
}

extension OptionTable {
/// Parse the given command-line arguments into a set of options.
///
/// Throws an error if the command line contains any errors.
public func parse(_ arguments: [String],
forInteractiveMode isInteractiveMode: Bool = false) throws -> ParsedOptions {
for driverKind: DriverKind) throws -> ParsedOptions {
var trie = PrefixTrie<String.UTF8View, Option>()
for opt in options {
trie[opt.spelling.utf8] = opt
Expand All @@ -42,7 +59,7 @@ extension OptionTable {
parsedOptions.addInput(argument)

// In interactive mode, synthesize a "--" argument for all args after the first input.
if isInteractiveMode && index < arguments.endIndex {
if driverKind == .interactive && index < arguments.endIndex {
parsedOptions.addOption(.DASHDASH, argument: .multiple(Array(arguments[index...])))
break
}
Expand All @@ -59,6 +76,13 @@ extension OptionTable {
index: index - 1, argument: argument)
}

// Make sure this option is supported by the current driver kind.
guard option.isAccepted(by: driverKind) else {
throw OptionParseError.unsupportedOption(
index: index - 1, argument: argument, option: option,
currentDriverKind: driverKind)
}

// Translate the argument
switch option.kind {
case .input:
Expand Down
5 changes: 4 additions & 1 deletion Sources/swift-driver/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import TSCBasic
import TSCUtility

var intHandler: InterruptHandler?
let diagnosticsEngine = DiagnosticsEngine(handlers: [Driver.stderrDiagnosticsHandler])

do {
let processSet = ProcessSet()
Expand All @@ -39,7 +40,7 @@ do {
try exec(path: subcommandPath?.pathString ?? "", args: Array(arguments.dropFirst()))
}

var driver = try Driver(args: arguments)
var driver = try Driver(args: arguments, diagnosticsEngine: diagnosticsEngine)
let resolver = try ArgsResolver()
try driver.run(resolver: resolver, processSet: processSet)

Expand All @@ -48,6 +49,8 @@ do {
}
} catch Diagnostics.fatalError {
exit(EXIT_FAILURE)
} catch let diagnosticData as DiagnosticData {
diagnosticsEngine.emit(.error(diagnosticData))
} catch {
print("error: \(error)")
exit(EXIT_FAILURE)
Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fileprivate func assertDriverDiagnostics(
let matcher = DiagnosticVerifier()
defer { matcher.verify(file: file, line: line) }

var driver = try Driver(args: args, env: env, diagnosticsHandler: matcher.emit(_:))
var driver = try Driver(args: args, env: env, diagnosticsEngine: DiagnosticsEngine(handlers: [matcher.emit(_:)]))
try body(&driver, matcher)
}

Expand Down
21 changes: 15 additions & 6 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,38 @@ final class SwiftDriverTests: XCTestCase {
let results = try options.parse([
"input1", "-color-diagnostics", "-Ifoo", "-I", "bar spaces",
"-I=wibble", "input2", "-module-name", "main",
"-sanitize=a,b,c", "--", "-foo", "-bar"])
"-sanitize=a,b,c", "--", "-foo", "-bar"], for: .batch)
XCTAssertEqual(results.description,
"input1 -color-diagnostics -I foo -I 'bar spaces' -I=wibble input2 -module-name main -sanitize=a,b,c -- -foo -bar")
}

func testParseErrors() {
let options = OptionTable()

XCTAssertThrowsError(try options.parse(["-unrecognized"])) { error in
XCTAssertThrowsError(try options.parse(["-unrecognized"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .unknownOption(index: 0, argument: "-unrecognized"))
}

XCTAssertThrowsError(try options.parse(["-I"])) { error in
XCTAssertThrowsError(try options.parse(["-I"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .missingArgument(index: 0, argument: "-I"))
}

XCTAssertThrowsError(try options.parse(["-color-diagnostics", "-I"])) { error in
XCTAssertThrowsError(try options.parse(["-color-diagnostics", "-I"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .missingArgument(index: 1, argument: "-I"))
}

XCTAssertThrowsError(try options.parse(["-module-name"])) { error in
XCTAssertThrowsError(try options.parse(["-module-name"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .missingArgument(index: 0, argument: "-module-name"))
}

XCTAssertThrowsError(try options.parse(["-o"], for: .interactive)) { error in
XCTAssertEqual(error as? OptionParseError, .unsupportedOption(index: 0, argument: "-o", option: .o, currentDriverKind: .interactive))
}

XCTAssertThrowsError(try options.parse(["-repl"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .unsupportedOption(index: 0, argument: "-repl", option: .repl, currentDriverKind: .batch))
}

}

func testInvocationRunModes() throws {
Expand Down Expand Up @@ -293,7 +302,7 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertEqual(driver.moduleName, "main")
}

try assertNoDriverDiagnostics(args: "swiftc", "-repl") { driver in
try assertNoDriverDiagnostics(args: "swift", "-repl") { driver in
XCTAssertNil(driver.moduleOutput)
XCTAssertEqual(driver.moduleName, "REPL")
}
Expand Down