From 11e0f93aed9dd971b3054ffb0aa5aea9d0ca5700 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Wed, 16 Jun 2021 14:53:13 -0700 Subject: [PATCH] Verify that only targets that are manifest dependencies can be imported. This change introduces a build verification step that attempts to detect scenarios where a target contains an `import` of another target in the package without declaring the imported target as a dependency in the manifest. This is done via SwiftDriver's import-prescan capability which relies on libSwiftScan to quickly parse a target's sources and identify all `import`ed modules. Related to rdar://79423257 --- .../ImportOfMissingDependency/Package.swift | 20 +++++++++ .../Sources/A/A.swift | 4 ++ .../Sources/B/B.swift | 3 ++ .../Sources/B/main.swift | 1 + Sources/Build/BuildOperation.swift | 43 ++++++++++++++++++- ...dOperationBuildSystemDelegateHandler.swift | 23 +++++++++- Tests/CommandsTests/BuildToolTests.swift | 13 ++++++ 7 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 Fixtures/Miscellaneous/ImportOfMissingDependency/Package.swift create mode 100644 Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/A/A.swift create mode 100644 Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/B.swift create mode 100644 Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/main.swift diff --git a/Fixtures/Miscellaneous/ImportOfMissingDependency/Package.swift b/Fixtures/Miscellaneous/ImportOfMissingDependency/Package.swift new file mode 100644 index 00000000000..fcf79d6a61c --- /dev/null +++ b/Fixtures/Miscellaneous/ImportOfMissingDependency/Package.swift @@ -0,0 +1,20 @@ +// swift-tools-version:5.1 +import PackageDescription + +let package = Package( + name: "VerificationTestPackage", + products: [ + .executable(name: "BExec", targets: ["B"]), + ], + dependencies: [ + + ], + targets: [ + .target( + name: "A", + dependencies: []), + .target( + name: "B", + dependencies: []), + ] +) diff --git a/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/A/A.swift b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/A/A.swift new file mode 100644 index 00000000000..992a085263c --- /dev/null +++ b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/A/A.swift @@ -0,0 +1,4 @@ +import B +public func bar(x: Int) -> Int { + return 11 + foo(x: x) +} diff --git a/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/B.swift b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/B.swift new file mode 100644 index 00000000000..4da1a2e51cd --- /dev/null +++ b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/B.swift @@ -0,0 +1,3 @@ +public func foo(x: Int) -> Int { + return 11 + x +} diff --git a/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/main.swift b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/main.swift new file mode 100644 index 00000000000..5364a821f33 --- /dev/null +++ b/Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/main.swift @@ -0,0 +1 @@ +print(baz(x: 11)) diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index c17262fd50f..aebbfd19381 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -17,6 +17,8 @@ import SPMLLBuild import TSCBasic import TSCUtility +@_implementationOnly import SwiftDriver + public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildSystem, BuildErrorAdviceProvider { /// The delegate used by the build system. @@ -118,10 +120,49 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS buildSystem?.cancel() } + // Emit a warning if a target imports another target in this build + // without specifying it as a dependency in the manifest + private func verifyTargetImports(in description: BuildDescription) throws { + for (target, commandLine) in description.swiftTargetCommandLineArgs { + do { + guard let dependencies = description.targetDependencyMap[target] else { + // Skip target if no dependency information is present + continue + } + let resolver = try ArgsResolver(fileSystem: localFileSystem) + let executor = SPMSwiftDriverExecutor(resolver: resolver, + fileSystem: localFileSystem, + env: ProcessEnv.vars) + var driver = try Driver(args: commandLine, + diagnosticsEngine: diagnostics, + fileSystem: localFileSystem, + executor: executor) + let imports = try driver.performImportPrescan().imports + let targetDependenciesSet = Set(dependencies) + let nonDependencyTargetsSet = + Set(description.targetDependencyMap.keys.filter { !targetDependenciesSet.contains($0) }) + let importedTargetsMissingDependency = Set(imports).intersection(nonDependencyTargetsSet) + if let missedDependency = importedTargetsMissingDependency.first { + diagnostics.emit(warning: "Target \(target) imports another target (\(missedDependency)) in the package without declaring it a dependency.", location: nil) + } + } catch { + // The above verification is a best-effort attempt to warn the user about a potential manifest + // error. If something went wrong during the import-prescan, proceed silently. + return + } + } + } + /// Perform a build using the given build description and subset. public func build(subset: BuildSubset) throws { + // Get the build description + let buildDescription = try getBuildDescription() + + // Verify dependency imports on the described targers + try verifyTargetImports(in: buildDescription) + // Create the build system. - let buildSystem = try createBuildSystem(with: getBuildDescription()) + let buildSystem = try createBuildSystem(with: buildDescription) self.buildSystem = buildSystem // Perform the build. diff --git a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift index ed801adf3d3..cf3e9019425 100644 --- a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift +++ b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift @@ -192,6 +192,7 @@ private final class InProcessTool: Tool { public struct BuildDescription: Codable { public typealias CommandName = String public typealias TargetName = String + public typealias CommandLineFlag = String /// The Swift compiler invocation targets. let swiftCommands: [BuildManifest.CmdName : SwiftCompilerTool] @@ -205,8 +206,14 @@ public struct BuildDescription: Codable { /// The map of copy commands. let copyCommands: [BuildManifest.CmdName: LLBuildManifest.CopyTool] + /// Every target's set of dependencies. + let targetDependencyMap: [TargetName: [TargetName]] + + /// A full swift driver command-line invocation used to build a given Swift target + let swiftTargetCommandLineArgs: [TargetName: [CommandLineFlag]] + /// The built test products. - public let builtTestProducts: [BuiltTestProduct] + let builtTestProducts: [BuiltTestProduct] public init( plan: BuildPlan, @@ -219,6 +226,20 @@ public struct BuildDescription: Codable { self.swiftFrontendCommands = swiftFrontendCommands self.testDiscoveryCommands = testDiscoveryCommands self.copyCommands = copyCommands + self.targetDependencyMap = try plan.targets.reduce(into: [TargetName: [TargetName]]()) { + let deps = try $1.target.recursiveTargetDependencies().map { $0.c99name } + $0[$1.target.c99name] = deps + } + var targetCommandLines: [TargetName: [CommandLineFlag]] = [:] + for (target, description) in plan.targetMap { + guard case .swift(let desc) = description else { + continue + } + targetCommandLines[target.c99name] = + try desc.emitCommandLine() + ["-driver-use-frontend-path", + plan.buildParameters.toolchain.swiftCompiler.pathString] + } + self.swiftTargetCommandLineArgs = targetCommandLines self.builtTestProducts = plan.buildProducts.filter{ $0.product.type == .test }.map { desc in return BuiltTestProduct( diff --git a/Tests/CommandsTests/BuildToolTests.swift b/Tests/CommandsTests/BuildToolTests.swift index b445c8796e8..9e818fcb23d 100644 --- a/Tests/CommandsTests/BuildToolTests.swift +++ b/Tests/CommandsTests/BuildToolTests.swift @@ -71,6 +71,19 @@ final class BuildToolTests: XCTestCase { } } + func testImportOfMissedDepWarning() throws { + fixture(name: "Miscellaneous/ImportOfMissingDependency") { path in + let fullPath = resolveSymlinks(path) + XCTAssertThrowsError(try build([], packagePath: fullPath)) { error in + guard case SwiftPMProductError.executionFailure(_, _, let stderr) = error else { + XCTFail() + return + } + XCTAssertTrue(stderr.contains("warning: Target A imports another target (B) in the package without declaring it a dependency.")) + } + } + } + func testBinPathAndSymlink() throws { fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { path in let fullPath = resolveSymlinks(path)