From 6d46af51449d4936844ae223b3b778da595d9864 Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sun, 6 Sep 2020 11:28:57 -0700 Subject: [PATCH 1/5] Validate profile generation args --- Sources/SwiftDriver/Driver/Driver.swift | 25 ++++++++++++++++++ Tests/SwiftDriverTests/SwiftDriverTests.swift | 26 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/Sources/SwiftDriver/Driver/Driver.swift b/Sources/SwiftDriver/Driver/Driver.swift index 4c2817bdd..579a323d5 100644 --- a/Sources/SwiftDriver/Driver/Driver.swift +++ b/Sources/SwiftDriver/Driver/Driver.swift @@ -27,6 +27,7 @@ public struct Driver { case unableToLoadOutputFileMap(String) case unableToDecodeFrontendTargetInfo case failedToRetrieveFrontendTargetInfo + case missingProfilingData(String) // Explicit Module Build Failures case malformedModuleDependency(String, String) case missingPCMArguments(String) @@ -55,6 +56,8 @@ public struct Driver { return "could not decode frontend target info; compiler driver and frontend executables may be incompatible" case .failedToRetrieveFrontendTargetInfo: return "failed to retrieve frontend target info" + case .missingProfilingData(let arg): + return "no profdata file exists at '\(arg)'" // Explicit Module Build Failures case .malformedModuleDependency(let moduleName, let errorDescription): return "Malformed Module Dependency: \(moduleName), \(errorDescription)" @@ -328,6 +331,9 @@ public struct Driver { self.numParallelJobs = Self.determineNumParallelJobs(&parsedOptions, diagnosticsEngine: diagnosticEngine, env: env) try Self.validateWarningControlArgs(&parsedOptions) + try Self.validateProfilingArgs(&parsedOptions, + fileSystem: fileSystem, + workingDirectory: workingDirectory) Self.validateCoverageArgs(&parsedOptions, diagnosticsEngine: diagnosticEngine) try toolchain.validateArgs(&parsedOptions, targetTriple: self.frontendTargetInfo.target.triple, @@ -1565,6 +1571,25 @@ extension Driver { } } + static func validateProfilingArgs(_ parsedOptions: inout ParsedOptions, + fileSystem: FileSystem, + workingDirectory: AbsolutePath?) throws { + if parsedOptions.hasArgument(.profileGenerate) && + parsedOptions.hasArgument(.profileUse) { + throw Error.conflictingOptions(.profileGenerate, .profileUse) + } + + if let profileArgs = parsedOptions.getLastArgument(.profileUse)?.asMultiple, + let workingDirectory = workingDirectory ?? fileSystem.currentWorkingDirectory { + for profilingData in profileArgs { + guard fileSystem.exists(AbsolutePath(profilingData, + relativeTo: workingDirectory)) else { + throw Error.missingProfilingData(profilingData) + } + } + } + } + private static func validateCoverageArgs(_ parsedOptions: inout ParsedOptions, diagnosticsEngine: DiagnosticsEngine) { for coveragePrefixMap in parsedOptions.arguments(for: .coveragePrefixMap) { let value = coveragePrefixMap.argument.asSingle diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index d0f5bde0c..31045cbdf 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1664,6 +1664,32 @@ final class SwiftDriverTests: XCTestCase { try assertNoDriverDiagnostics(args: "swiftc", "-c", "-target", "x86_64-apple-macosx10.14", "-link-objc-runtime", "foo.swift") } + func testProfileArgValidation() throws { + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-profile-generate", "-profile-use=profile.profdata"])) { + XCTAssertEqual($0 as? Driver.Error, .conflictingOptions(.profileGenerate, .profileUse)) + } + + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-profile-use=profile.profdata"])) { + XCTAssertEqual($0 as? Driver.Error, .missingProfilingData("profile.profdata")) + } + + try withTemporaryDirectory { path in + try localFileSystem.writeFileContents(path.appending(component: "profile.profdata"), bytes: .init()) + XCTAssertNoThrow(try Driver(args: ["swiftc", "-working-directory", path.pathString, "foo.swift", "-profile-use=profile.profdata"])) + } + + try withTemporaryDirectory { path in + try localFileSystem.writeFileContents(path.appending(component: "profile.profdata"), bytes: .init()) + XCTAssertThrowsError(try Driver(args: ["swiftc", "-working-directory", path.pathString, "foo.swift", + "-profile-use=profile.profdata,profile2.profdata"])) { + guard case Driver.Error.missingProfilingData = $0 else { + XCTFail() + return + } + } + } + } + // Test cases ported from Driver/macabi-environment.swift func testDarwinSDKVersioning() throws { try withTemporaryDirectory { tmpDir in From fa217a6289c43cf8a9171002f382f1596d6c5a70 Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sun, 6 Sep 2020 12:06:27 -0700 Subject: [PATCH 2/5] Validate conditional compilation args --- Sources/SwiftDriver/Driver/Driver.swift | 24 +++++++++++++++++++ Tests/SwiftDriverTests/SwiftDriverTests.swift | 16 +++++++++++++ 2 files changed, 40 insertions(+) diff --git a/Sources/SwiftDriver/Driver/Driver.swift b/Sources/SwiftDriver/Driver/Driver.swift index 579a323d5..4822c67e1 100644 --- a/Sources/SwiftDriver/Driver/Driver.swift +++ b/Sources/SwiftDriver/Driver/Driver.swift @@ -28,6 +28,9 @@ public struct Driver { case unableToDecodeFrontendTargetInfo case failedToRetrieveFrontendTargetInfo case missingProfilingData(String) + case cannotAssignToConditionalCompilationFlag(String) + case conditionalCompilationFlagHasRedundantPrefix(String) + case conditionalCompilationFlagIsNotValidIdentifier(String) // Explicit Module Build Failures case malformedModuleDependency(String, String) case missingPCMArguments(String) @@ -71,6 +74,12 @@ public struct Driver { return "unable to load output file map '\(path)': no such file or directory" case .missingExternalDependency(let moduleName): return "Missing External dependency info for module: \(moduleName)" + case .cannotAssignToConditionalCompilationFlag(let name): + return "conditional compilation flags do not have values in Swift; they are either present or absent (rather than '\(name)')" + case .conditionalCompilationFlagHasRedundantPrefix(let name): + return "invalid argument '-D\(name)'; did you provide a redundant '-D' in your build settings?" + case .conditionalCompilationFlagIsNotValidIdentifier(let name): + return "conditional compilation flags must be valid Swift identifiers (rather than '\(name)')" } } } @@ -334,6 +343,7 @@ public struct Driver { try Self.validateProfilingArgs(&parsedOptions, fileSystem: fileSystem, workingDirectory: workingDirectory) + try Self.validateCompilationConditionArgs(&parsedOptions) Self.validateCoverageArgs(&parsedOptions, diagnosticsEngine: diagnosticEngine) try toolchain.validateArgs(&parsedOptions, targetTriple: self.frontendTargetInfo.target.triple, @@ -1590,6 +1600,20 @@ extension Driver { } } + static func validateCompilationConditionArgs(_ parsedOptions: inout ParsedOptions) throws { + for arg in parsedOptions.arguments(for: .D).map(\.argument.asSingle) { + guard !arg.contains("=") else { + throw Error.cannotAssignToConditionalCompilationFlag(arg) + } + guard !arg.hasPrefix("-D") else { + throw Error.conditionalCompilationFlagHasRedundantPrefix(arg) + } + guard arg.sd_isSwiftIdentifier else { + throw Error.conditionalCompilationFlagIsNotValidIdentifier(arg) + } + } + } + private static func validateCoverageArgs(_ parsedOptions: inout ParsedOptions, diagnosticsEngine: DiagnosticsEngine) { for coveragePrefixMap in parsedOptions.arguments(for: .coveragePrefixMap) { let value = coveragePrefixMap.argument.asSingle diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index 31045cbdf..7d8e8b57c 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1690,6 +1690,22 @@ final class SwiftDriverTests: XCTestCase { } } + func testConditionalCompilationArgValidation() throws { + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-DFOO=BAR"])) { + XCTAssertEqual($0 as? Driver.Error, .cannotAssignToConditionalCompilationFlag("FOO=BAR")) + } + + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-D-DFOO"])) { + XCTAssertEqual($0 as? Driver.Error, .conditionalCompilationFlagHasRedundantPrefix("-DFOO")) + } + + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-Dnot-an-identifier"])) { + XCTAssertEqual($0 as? Driver.Error, .conditionalCompilationFlagIsNotValidIdentifier("not-an-identifier")) + } + + XCTAssertNoThrow(try Driver(args: ["swiftc", "foo.swift", "-DFOO"])) + } + // Test cases ported from Driver/macabi-environment.swift func testDarwinSDKVersioning() throws { try withTemporaryDirectory { tmpDir in From 5982e805167040211096fabc2ae5bc49e9b5e56f Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sun, 6 Sep 2020 12:19:51 -0700 Subject: [PATCH 3/5] Validate framework search path args --- Sources/SwiftDriver/Driver/Driver.swift | 24 ++++++++++++++----- Tests/SwiftDriverTests/SwiftDriverTests.swift | 16 +++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftDriver/Driver/Driver.swift b/Sources/SwiftDriver/Driver/Driver.swift index 4822c67e1..001f144c2 100644 --- a/Sources/SwiftDriver/Driver/Driver.swift +++ b/Sources/SwiftDriver/Driver/Driver.swift @@ -31,6 +31,7 @@ public struct Driver { case cannotAssignToConditionalCompilationFlag(String) case conditionalCompilationFlagHasRedundantPrefix(String) case conditionalCompilationFlagIsNotValidIdentifier(String) + case frameworkSearchPathIncludesExtension(String) // Explicit Module Build Failures case malformedModuleDependency(String, String) case missingPCMArguments(String) @@ -61,6 +62,14 @@ public struct Driver { return "failed to retrieve frontend target info" case .missingProfilingData(let arg): return "no profdata file exists at '\(arg)'" + case .cannotAssignToConditionalCompilationFlag(let name): + return "conditional compilation flags do not have values in Swift; they are either present or absent (rather than '\(name)')" + case .conditionalCompilationFlagHasRedundantPrefix(let name): + return "invalid argument '-D\(name)'; did you provide a redundant '-D' in your build settings?" + case .conditionalCompilationFlagIsNotValidIdentifier(let name): + return "conditional compilation flags must be valid Swift identifiers (rather than '\(name)')" + case .frameworkSearchPathIncludesExtension(let arg): + return "framework search path ends in \".framework\"; add directory containing framework instead: \(arg)" // Explicit Module Build Failures case .malformedModuleDependency(let moduleName, let errorDescription): return "Malformed Module Dependency: \(moduleName), \(errorDescription)" @@ -74,12 +83,6 @@ public struct Driver { return "unable to load output file map '\(path)': no such file or directory" case .missingExternalDependency(let moduleName): return "Missing External dependency info for module: \(moduleName)" - case .cannotAssignToConditionalCompilationFlag(let name): - return "conditional compilation flags do not have values in Swift; they are either present or absent (rather than '\(name)')" - case .conditionalCompilationFlagHasRedundantPrefix(let name): - return "invalid argument '-D\(name)'; did you provide a redundant '-D' in your build settings?" - case .conditionalCompilationFlagIsNotValidIdentifier(let name): - return "conditional compilation flags must be valid Swift identifiers (rather than '\(name)')" } } } @@ -344,6 +347,7 @@ public struct Driver { fileSystem: fileSystem, workingDirectory: workingDirectory) try Self.validateCompilationConditionArgs(&parsedOptions) + try Self.validateFrameworkSearchPathArgs(&parsedOptions) Self.validateCoverageArgs(&parsedOptions, diagnosticsEngine: diagnosticEngine) try toolchain.validateArgs(&parsedOptions, targetTriple: self.frontendTargetInfo.target.triple, @@ -1614,6 +1618,14 @@ extension Driver { } } + static func validateFrameworkSearchPathArgs(_ parsedOptions: inout ParsedOptions) throws { + for arg in parsedOptions.arguments(for: .F, .Fsystem).map(\.argument.asSingle) { + if arg.hasSuffix(".framework") || arg.hasSuffix(".framework/") { + throw Error.frameworkSearchPathIncludesExtension(arg) + } + } + } + private static func validateCoverageArgs(_ parsedOptions: inout ParsedOptions, diagnosticsEngine: DiagnosticsEngine) { for coveragePrefixMap in parsedOptions.arguments(for: .coveragePrefixMap) { let value = coveragePrefixMap.argument.asSingle diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index 7d8e8b57c..bf12ed709 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1706,6 +1706,22 @@ final class SwiftDriverTests: XCTestCase { XCTAssertNoThrow(try Driver(args: ["swiftc", "foo.swift", "-DFOO"])) } + func testFrameworkSearchPathArgValidation() throws { + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework"])) { + XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework")) + } + + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework/"])) { + XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework/")) + } + + XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/xyz.framework"])) { + XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework")) + } + + XCTAssertNoThrow(try Driver(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/"])) + } + // Test cases ported from Driver/macabi-environment.swift func testDarwinSDKVersioning() throws { try withTemporaryDirectory { tmpDir in From c9f696dd26819ab2957ac4d72bd29505def84b39 Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sun, 6 Sep 2020 14:06:48 -0700 Subject: [PATCH 4/5] Don't skip argument validation steps if an earlier step failed --- Sources/SwiftDriver/Driver/Driver.swift | 51 ++++++++-------- Sources/swift-driver/main.swift | 4 ++ .../Helpers/AssertDiagnostics.swift | 1 + Tests/SwiftDriverTests/SwiftDriverTests.swift | 60 ++++++++++--------- 4 files changed, 65 insertions(+), 51 deletions(-) diff --git a/Sources/SwiftDriver/Driver/Driver.swift b/Sources/SwiftDriver/Driver/Driver.swift index 001f144c2..578df9481 100644 --- a/Sources/SwiftDriver/Driver/Driver.swift +++ b/Sources/SwiftDriver/Driver/Driver.swift @@ -342,12 +342,13 @@ public struct Driver { self.numThreads = Self.determineNumThreads(&parsedOptions, compilerMode: compilerMode, diagnosticsEngine: diagnosticEngine) self.numParallelJobs = Self.determineNumParallelJobs(&parsedOptions, diagnosticsEngine: diagnosticEngine, env: env) - try Self.validateWarningControlArgs(&parsedOptions) - try Self.validateProfilingArgs(&parsedOptions, - fileSystem: fileSystem, - workingDirectory: workingDirectory) - try Self.validateCompilationConditionArgs(&parsedOptions) - try Self.validateFrameworkSearchPathArgs(&parsedOptions) + Self.validateWarningControlArgs(&parsedOptions, diagnosticEngine: diagnosticEngine) + Self.validateProfilingArgs(&parsedOptions, + fileSystem: fileSystem, + workingDirectory: workingDirectory, + diagnosticEngine: diagnosticEngine) + Self.validateCompilationConditionArgs(&parsedOptions, diagnosticEngine: diagnosticEngine) + Self.validateFrameworkSearchPathArgs(&parsedOptions, diagnosticEngine: diagnosticEngine) Self.validateCoverageArgs(&parsedOptions, diagnosticsEngine: diagnosticEngine) try toolchain.validateArgs(&parsedOptions, targetTriple: self.frontendTargetInfo.target.triple, @@ -1578,50 +1579,52 @@ extension Diagnostic.Message { // MARK: Miscellaneous Argument Validation extension Driver { - static func validateWarningControlArgs(_ parsedOptions: inout ParsedOptions) throws { + static func validateWarningControlArgs(_ parsedOptions: inout ParsedOptions, + diagnosticEngine: DiagnosticsEngine) { if parsedOptions.hasArgument(.suppressWarnings) && parsedOptions.hasFlag(positive: .warningsAsErrors, negative: .noWarningsAsErrors, default: false) { - throw Error.conflictingOptions(.warningsAsErrors, .suppressWarnings) + diagnosticEngine.emit(Error.conflictingOptions(.warningsAsErrors, .suppressWarnings)) } } static func validateProfilingArgs(_ parsedOptions: inout ParsedOptions, fileSystem: FileSystem, - workingDirectory: AbsolutePath?) throws { + workingDirectory: AbsolutePath?, + diagnosticEngine: DiagnosticsEngine) { if parsedOptions.hasArgument(.profileGenerate) && parsedOptions.hasArgument(.profileUse) { - throw Error.conflictingOptions(.profileGenerate, .profileUse) + diagnosticEngine.emit(Error.conflictingOptions(.profileGenerate, .profileUse)) } if let profileArgs = parsedOptions.getLastArgument(.profileUse)?.asMultiple, let workingDirectory = workingDirectory ?? fileSystem.currentWorkingDirectory { for profilingData in profileArgs { - guard fileSystem.exists(AbsolutePath(profilingData, - relativeTo: workingDirectory)) else { - throw Error.missingProfilingData(profilingData) + if !fileSystem.exists(AbsolutePath(profilingData, + relativeTo: workingDirectory)) { + diagnosticEngine.emit(Error.missingProfilingData(profilingData)) } } } } - static func validateCompilationConditionArgs(_ parsedOptions: inout ParsedOptions) throws { + static func validateCompilationConditionArgs(_ parsedOptions: inout ParsedOptions, + diagnosticEngine: DiagnosticsEngine) { for arg in parsedOptions.arguments(for: .D).map(\.argument.asSingle) { - guard !arg.contains("=") else { - throw Error.cannotAssignToConditionalCompilationFlag(arg) - } - guard !arg.hasPrefix("-D") else { - throw Error.conditionalCompilationFlagHasRedundantPrefix(arg) - } - guard arg.sd_isSwiftIdentifier else { - throw Error.conditionalCompilationFlagIsNotValidIdentifier(arg) + if arg.contains("=") { + diagnosticEngine.emit(Error.cannotAssignToConditionalCompilationFlag(arg)) + } else if arg.hasPrefix("-D") { + diagnosticEngine.emit(Error.conditionalCompilationFlagHasRedundantPrefix(arg)) + } else if !arg.sd_isSwiftIdentifier { + diagnosticEngine.emit(Error.conditionalCompilationFlagIsNotValidIdentifier(arg)) } } } - static func validateFrameworkSearchPathArgs(_ parsedOptions: inout ParsedOptions) throws { + static func validateFrameworkSearchPathArgs(_ parsedOptions: inout ParsedOptions, + diagnosticEngine: DiagnosticsEngine) { for arg in parsedOptions.arguments(for: .F, .Fsystem).map(\.argument.asSingle) { if arg.hasSuffix(".framework") || arg.hasSuffix(".framework/") { - throw Error.frameworkSearchPathIncludesExtension(arg) + diagnosticEngine.emit(Error.frameworkSearchPathIncludesExtension(arg)) } } } diff --git a/Sources/swift-driver/main.swift b/Sources/swift-driver/main.swift index 46044c09d..34b112596 100644 --- a/Sources/swift-driver/main.swift +++ b/Sources/swift-driver/main.swift @@ -47,6 +47,10 @@ do { var driver = try Driver(args: arguments, diagnosticsEngine: diagnosticsEngine, executor: executor) + // FIXME: The following check should be at the end of Driver.init, but current + // usage of the DiagnosticVerifier in tests makes this difficult. + guard !driver.diagnosticEngine.hasErrors else { throw Diagnostics.fatalError } + let jobs = try driver.planBuild() try driver.run(jobs: jobs) diff --git a/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift b/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift index 897ace7c8..029637e6b 100644 --- a/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift +++ b/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift @@ -13,6 +13,7 @@ import XCTest import SwiftDriver import TSCBasic +import TSCUtility func assertDriverDiagnostics( args: [String], diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index bf12ed709..4641fd7c3 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1665,61 +1665,67 @@ final class SwiftDriverTests: XCTestCase { } func testProfileArgValidation() throws { - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-profile-generate", "-profile-use=profile.profdata"])) { - XCTAssertEqual($0 as? Driver.Error, .conflictingOptions(.profileGenerate, .profileUse)) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-profile-generate", "-profile-use=profile.profdata"]) { + $1.expect(.error(Driver.Error.conflictingOptions(.profileGenerate, .profileUse))) + $1.expect(.error(Driver.Error.missingProfilingData("profile.profdata"))) } - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-profile-use=profile.profdata"])) { - XCTAssertEqual($0 as? Driver.Error, .missingProfilingData("profile.profdata")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-profile-use=profile.profdata"]) { + $1.expect(.error(Driver.Error.missingProfilingData("profile.profdata"))) } try withTemporaryDirectory { path in try localFileSystem.writeFileContents(path.appending(component: "profile.profdata"), bytes: .init()) - XCTAssertNoThrow(try Driver(args: ["swiftc", "-working-directory", path.pathString, "foo.swift", "-profile-use=profile.profdata"])) + try assertNoDriverDiagnostics(args: "swiftc", "-working-directory", path.pathString, "foo.swift", "-profile-use=profile.profdata") } try withTemporaryDirectory { path in try localFileSystem.writeFileContents(path.appending(component: "profile.profdata"), bytes: .init()) - XCTAssertThrowsError(try Driver(args: ["swiftc", "-working-directory", path.pathString, "foo.swift", - "-profile-use=profile.profdata,profile2.profdata"])) { - guard case Driver.Error.missingProfilingData = $0 else { - XCTFail() - return - } + try assertDriverDiagnostics(args: ["swiftc", "-working-directory", path.pathString, "foo.swift", + "-profile-use=profile.profdata,profile2.profdata"]) { + $1.expect(.error(Driver.Error.missingProfilingData(path.appending(component: "profile2.profdata").pathString))) } } } func testConditionalCompilationArgValidation() throws { - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-DFOO=BAR"])) { - XCTAssertEqual($0 as? Driver.Error, .cannotAssignToConditionalCompilationFlag("FOO=BAR")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-DFOO=BAR"]) { + $1.expect(.error(Driver.Error.cannotAssignToConditionalCompilationFlag("FOO=BAR"))) } - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-D-DFOO"])) { - XCTAssertEqual($0 as? Driver.Error, .conditionalCompilationFlagHasRedundantPrefix("-DFOO")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-D-DFOO"]) { + $1.expect(.error(Driver.Error.conditionalCompilationFlagHasRedundantPrefix("-DFOO"))) } - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-Dnot-an-identifier"])) { - XCTAssertEqual($0 as? Driver.Error, .conditionalCompilationFlagIsNotValidIdentifier("not-an-identifier")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-Dnot-an-identifier"]) { + $1.expect(.error(Driver.Error.conditionalCompilationFlagIsNotValidIdentifier("not-an-identifier"))) } - XCTAssertNoThrow(try Driver(args: ["swiftc", "foo.swift", "-DFOO"])) + try assertNoDriverDiagnostics(args: "swiftc", "foo.swift", "-DFOO") } func testFrameworkSearchPathArgValidation() throws { - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework"])) { - XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework"]) { + $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) } - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework/"])) { - XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework/")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework/"]) { + $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework/"))) } - XCTAssertThrowsError(try Driver(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/xyz.framework"])) { - XCTAssertEqual($0 as? Driver.Error, .frameworkSearchPathIncludesExtension("/some/dir/xyz.framework")) + try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/xyz.framework"]) { + $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) } - XCTAssertNoThrow(try Driver(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/"])) + try assertNoDriverDiagnostics(args: "swiftc", "foo.swift", "-Fsystem", "/some/dir/") + } + + func testMultipleValidationFailures() throws { + try assertDiagnostics { engine, verifier in + verifier.expect(.error(Driver.Error.conditionalCompilationFlagIsNotValidIdentifier("not-an-identifier"))) + verifier.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) + _ = try Driver(args: ["swiftc", "foo.swift", "-Dnot-an-identifier", "-F/some/dir/xyz.framework"], diagnosticsEngine: engine) + } } // Test cases ported from Driver/macabi-environment.swift @@ -2368,8 +2374,8 @@ final class SwiftDriverTests: XCTestCase { } do { - XCTAssertThrowsError(try Driver(args: ["swift", "-no-warnings-as-errors", "-warnings-as-errors", "-suppress-warnings", "foo.swift"])) { - XCTAssertEqual($0 as? Driver.Error, Driver.Error.conflictingOptions(.warningsAsErrors, .suppressWarnings)) + try assertDriverDiagnostics(args: ["swift", "-no-warnings-as-errors", "-warnings-as-errors", "-suppress-warnings", "foo.swift"]) { + $1.expect(.error(Driver.Error.conflictingOptions(.warningsAsErrors, .suppressWarnings))) } } From c21e733dfc643a3d2aff1e31bfe6ec706c687d53 Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sun, 6 Sep 2020 15:28:42 -0700 Subject: [PATCH 5/5] Change attempted compilation condition assignment and bad -F argument diagnostics from errors to warnings --- Sources/SwiftDriver/Driver/Driver.swift | 16 ++++++++-------- .../Helpers/AssertDiagnostics.swift | 1 - Tests/SwiftDriverTests/SwiftDriverTests.swift | 10 +++++----- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Sources/SwiftDriver/Driver/Driver.swift b/Sources/SwiftDriver/Driver/Driver.swift index 578df9481..4cd839374 100644 --- a/Sources/SwiftDriver/Driver/Driver.swift +++ b/Sources/SwiftDriver/Driver/Driver.swift @@ -28,10 +28,8 @@ public struct Driver { case unableToDecodeFrontendTargetInfo case failedToRetrieveFrontendTargetInfo case missingProfilingData(String) - case cannotAssignToConditionalCompilationFlag(String) case conditionalCompilationFlagHasRedundantPrefix(String) case conditionalCompilationFlagIsNotValidIdentifier(String) - case frameworkSearchPathIncludesExtension(String) // Explicit Module Build Failures case malformedModuleDependency(String, String) case missingPCMArguments(String) @@ -62,14 +60,10 @@ public struct Driver { return "failed to retrieve frontend target info" case .missingProfilingData(let arg): return "no profdata file exists at '\(arg)'" - case .cannotAssignToConditionalCompilationFlag(let name): - return "conditional compilation flags do not have values in Swift; they are either present or absent (rather than '\(name)')" case .conditionalCompilationFlagHasRedundantPrefix(let name): return "invalid argument '-D\(name)'; did you provide a redundant '-D' in your build settings?" case .conditionalCompilationFlagIsNotValidIdentifier(let name): return "conditional compilation flags must be valid Swift identifiers (rather than '\(name)')" - case .frameworkSearchPathIncludesExtension(let arg): - return "framework search path ends in \".framework\"; add directory containing framework instead: \(arg)" // Explicit Module Build Failures case .malformedModuleDependency(let moduleName, let errorDescription): return "Malformed Module Dependency: \(moduleName), \(errorDescription)" @@ -1575,6 +1569,12 @@ extension Diagnostic.Message { static var error_bridging_header_module_interface: Diagnostic.Message { .error("using bridging headers with module interfaces is unsupported") } + static func warning_cannot_assign_to_compilation_condition(name: String) -> Diagnostic.Message { + .warning("conditional compilation flags do not have values in Swift; they are either present or absent (rather than '\(name)')") + } + static func warning_framework_search_path_includes_extension(path: String) -> Diagnostic.Message { + .warning("framework search path ends in \".framework\"; add directory containing framework instead: \(path)") + } } // MARK: Miscellaneous Argument Validation @@ -1611,7 +1611,7 @@ extension Driver { diagnosticEngine: DiagnosticsEngine) { for arg in parsedOptions.arguments(for: .D).map(\.argument.asSingle) { if arg.contains("=") { - diagnosticEngine.emit(Error.cannotAssignToConditionalCompilationFlag(arg)) + diagnosticEngine.emit(.warning_cannot_assign_to_compilation_condition(name: arg)) } else if arg.hasPrefix("-D") { diagnosticEngine.emit(Error.conditionalCompilationFlagHasRedundantPrefix(arg)) } else if !arg.sd_isSwiftIdentifier { @@ -1624,7 +1624,7 @@ extension Driver { diagnosticEngine: DiagnosticsEngine) { for arg in parsedOptions.arguments(for: .F, .Fsystem).map(\.argument.asSingle) { if arg.hasSuffix(".framework") || arg.hasSuffix(".framework/") { - diagnosticEngine.emit(Error.frameworkSearchPathIncludesExtension(arg)) + diagnosticEngine.emit(.warning_framework_search_path_includes_extension(path: arg)) } } } diff --git a/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift b/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift index 029637e6b..897ace7c8 100644 --- a/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift +++ b/Tests/SwiftDriverTests/Helpers/AssertDiagnostics.swift @@ -13,7 +13,6 @@ import XCTest import SwiftDriver import TSCBasic -import TSCUtility func assertDriverDiagnostics( args: [String], diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index 4641fd7c3..db7602027 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1690,7 +1690,7 @@ final class SwiftDriverTests: XCTestCase { func testConditionalCompilationArgValidation() throws { try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-DFOO=BAR"]) { - $1.expect(.error(Driver.Error.cannotAssignToConditionalCompilationFlag("FOO=BAR"))) + $1.expect(.warning("conditional compilation flags do not have values in Swift; they are either present or absent (rather than 'FOO=BAR')")) } try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-D-DFOO"]) { @@ -1706,15 +1706,15 @@ final class SwiftDriverTests: XCTestCase { func testFrameworkSearchPathArgValidation() throws { try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework"]) { - $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) + $1.expect(.warning("framework search path ends in \".framework\"; add directory containing framework instead: /some/dir/xyz.framework")) } try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-F/some/dir/xyz.framework/"]) { - $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework/"))) + $1.expect(.warning("framework search path ends in \".framework\"; add directory containing framework instead: /some/dir/xyz.framework/")) } try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-Fsystem", "/some/dir/xyz.framework"]) { - $1.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) + $1.expect(.warning("framework search path ends in \".framework\"; add directory containing framework instead: /some/dir/xyz.framework")) } try assertNoDriverDiagnostics(args: "swiftc", "foo.swift", "-Fsystem", "/some/dir/") @@ -1723,7 +1723,7 @@ final class SwiftDriverTests: XCTestCase { func testMultipleValidationFailures() throws { try assertDiagnostics { engine, verifier in verifier.expect(.error(Driver.Error.conditionalCompilationFlagIsNotValidIdentifier("not-an-identifier"))) - verifier.expect(.error(Driver.Error.frameworkSearchPathIncludesExtension("/some/dir/xyz.framework"))) + verifier.expect(.warning("framework search path ends in \".framework\"; add directory containing framework instead: /some/dir/xyz.framework")) _ = try Driver(args: ["swiftc", "foo.swift", "-Dnot-an-identifier", "-F/some/dir/xyz.framework"], diagnosticsEngine: engine) } }