Skip to content

Commit

Permalink
Verify that only targets that are manifest dependencies can be imported.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
artemcm committed Jun 25, 2021
1 parent e3ea6ed commit c8ba669
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 10 deletions.
20 changes: 20 additions & 0 deletions 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: []),
]
)
@@ -0,0 +1,4 @@
import B
public func bar(x: Int) -> Int {
return 11 + foo(x: x)
}
@@ -0,0 +1,3 @@
public func foo(x: Int) -> Int {
return 11 + x
}
@@ -0,0 +1 @@
print(baz(x: 11))
43 changes: 42 additions & 1 deletion Sources/Build/BuildOperation.swift
Expand Up @@ -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.
Expand Down Expand Up @@ -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.swiftTargetScanArgs {
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.
Expand Down
21 changes: 21 additions & 0 deletions Sources/Build/BuildOperationBuildSystemDelegateHandler.swift
Expand Up @@ -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]
Expand All @@ -205,6 +206,12 @@ 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 dependency-scan a given Swift target
let swiftTargetScanArgs: [TargetName: [CommandLineFlag]]

/// The built test products.
public let builtTestProducts: [BuiltTestProduct]

Expand All @@ -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(scanInvocation: true) + ["-driver-use-frontend-path",
plan.buildParameters.toolchain.swiftCompiler.pathString]
}
self.swiftTargetScanArgs = targetCommandLines

self.builtTestProducts = plan.buildProducts.filter{ $0.product.type == .test }.map { desc in
return BuiltTestProduct(
Expand Down
22 changes: 13 additions & 9 deletions Sources/Build/BuildPlan.swift
Expand Up @@ -796,23 +796,27 @@ public final class SwiftTargetBuildDescription {
return args
}

public func emitCommandLine() throws -> [String] {
/// When `scanInvocation` argument is set to `true`, omit the side-effect producing arguments
/// such as emitting a module or supplementary outputs.
public func emitCommandLine(scanInvocation: Bool = false) throws -> [String] {
var result: [String] = []
result.append(buildParameters.toolchain.swiftCompiler.pathString)

result.append("-module-name")
result.append(target.c99name)

result.append("-emit-dependencies")
if !scanInvocation {
result.append("-emit-dependencies")

// FIXME: Do we always have a module?
result.append("-emit-module")
result.append("-emit-module-path")
result.append(moduleOutputPath.pathString)
// FIXME: Do we always have a module?
result.append("-emit-module")
result.append("-emit-module-path")
result.append(moduleOutputPath.pathString)

result.append("-output-file-map")
// FIXME: Eliminate side effect.
result.append(try writeOutputFileMap().pathString)
result.append("-output-file-map")
// FIXME: Eliminate side effect.
result.append(try writeOutputFileMap().pathString)
}

if buildParameters.useWholeModuleOptimization {
result.append("-whole-module-optimization")
Expand Down
13 changes: 13 additions & 0 deletions Tests/CommandsTests/BuildToolTests.swift
Expand Up @@ -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)
Expand Down

0 comments on commit c8ba669

Please sign in to comment.