Verify that only targets that are manifest dependencies can be import…

…ed. (#3562)

* 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

* Skip target-import dependency verification procedure and test, if the host compiler does not support it.

* Skip targets that contain or depend-on generated code, because generated source-code may not be available at the time of scan.

* Add an opt-out from explicit target dependency import checking.
`--disable-explicit-target-dependency-import-checking` prevents the verification code from being run as a just-in-case option to opt-out from this warning

* Flip the `--disable-explicit-target-dependency-import-checking` option to an opt-in `enable` option.
While we work on optimizing incremental build performance of this check, it would be nice to have it as an opt-in option.

* Allow configuring the explicit import checking with either 'error', 'warning', or 'none' modes, with 'none' being the default.

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: []),
+    ]
+)

Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/A/A.swift

@@ -0,0 +1,4 @@
+import B
+public func bar(x: Int) -> Int {
+    return 11 + foo(x: x)
+}

Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/B.swift

@@ -0,0 +1,3 @@
+public func foo(x: Int) -> Int {
+    return 11 + x
+}

Fixtures/Miscellaneous/ImportOfMissingDependency/Sources/B/main.swift

@@ -0,0 +1 @@
+print(baz(x: 11))

Sources/Build/BuildOperation.swift

@@ -25,6 +25,8 @@ import class TSCUtility.MultiLineNinjaProgressAnimation
 import class TSCUtility.NinjaProgressAnimation
 import protocol TSCUtility.ProgressAnimationProtocol
 
+@_implementationOnly import SwiftDriver
+
 public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildSystem, BuildErrorAdviceProvider {
 
     /// The delegate used by the build system.
@@ -156,12 +158,80 @@ 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 {
+        let checkingMode = description.explicitTargetDependencyImportCheckingMode
+        guard checkingMode != .none else {
+            return
+        }
+        // Ensure the compiler supports the import-scan operation
+        guard SwiftTargetBuildDescription.checkSupportedFrontendFlags(flags: ["import-prescan"], fileSystem: localFileSystem) else {
+            return
+        }
+
+        for (target, commandLine) in description.swiftTargetScanArgs {
+            do {
+                guard let dependencies = description.targetDependencyMap[target] else {
+                    // Skip target if no dependency information is present
+                    continue
+                }
+                let targetDependenciesSet = Set(dependencies)
+                guard !description.generatedSourceTargetSet.contains(target),
+                      targetDependenciesSet.intersection(description.generatedSourceTargetSet).isEmpty else {
+                    // Skip targets which contain, or depend-on-targets, with generated source-code.
+                    // Such as test discovery targets and targets with plugins.
+                    continue
+                }
+                let resolver = try ArgsResolver(fileSystem: localFileSystem)
+                let executor = SPMSwiftDriverExecutor(resolver: resolver,
+                                                      fileSystem: localFileSystem,
+                                                      env: ProcessEnv.vars)
+
+                let consumeDiagnostics: DiagnosticsEngine = DiagnosticsEngine(handlers: [])
+                var driver = try Driver(args: commandLine,
+                                        diagnosticsEngine: consumeDiagnostics,
+                                        fileSystem: localFileSystem,
+                                        executor: executor)
+                guard !consumeDiagnostics.hasErrors else {
+                  // If we could not init the driver with this command, something went wrong,
+                  // proceed without checking this target.
+                  continue
+                }
+                let imports = try driver.performImportPrescan().imports
+                let nonDependencyTargetsSet =
+                    Set(description.targetDependencyMap.keys.filter { !targetDependenciesSet.contains($0) })
+                let importedTargetsMissingDependency = Set(imports).intersection(nonDependencyTargetsSet)
+                if let missedDependency = importedTargetsMissingDependency.first {
+                    switch checkingMode {
+                        case .error:
+                            self.observabilityScope.emit(error: "Target \(target) imports another target (\(missedDependency)) in the package without declaring it a dependency.")
+                        case .warn:
+                            self.observabilityScope.emit(warning: "Target \(target) imports another target (\(missedDependency)) in the package without declaring it a dependency.")
+                        case .none:
+                            fatalError("Explicit import checking is disabled.")
+                    }
+                }
+            } 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 {
+
         let buildStartTime = DispatchTime.now()
 
         // Get the build description (either a cached one or newly created).
-        let buildDescription = try self.getBuildDescription()
+
+        // 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 self.createBuildSystem(buildDescription: buildDescription)

Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

@@ -98,7 +98,6 @@ final class TestDiscoveryCommand: CustomLLBuildCommand {
             let className = iterator.key
             stream <<< indent(8) <<< "testCase(\(className).__allTests__\(className)),\n"
         }
-
         stream <<< """
             ]
         }
@@ -213,6 +212,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]
@@ -226,6 +226,19 @@ public struct BuildDescription: Codable {
     /// The map of copy commands.
     let copyCommands: [BuildManifest.CmdName: LLBuildManifest.CopyTool]
 
+    /// A flag that inidcates this build should perform a check for whether targets only import
+    /// their explicitly-declared dependencies
+    let explicitTargetDependencyImportCheckingMode: BuildParameters.TargetDependencyImportCheckingMode
+
+    /// 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]]
+
+    /// A set of all targets with generated source
+    let generatedSourceTargetSet: Set<TargetName>
+
     /// The built test products.
     public let builtTestProducts: [BuiltTestProduct]
 
@@ -244,6 +257,28 @@ public struct BuildDescription: Codable {
         self.swiftFrontendCommands = swiftFrontendCommands
         self.testDiscoveryCommands = testDiscoveryCommands
         self.copyCommands = copyCommands
+        self.explicitTargetDependencyImportCheckingMode = plan.buildParameters.explicitTargetDependencyImportCheckingMode
+        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]] = [:]
+        var generatedSourceTargets: [TargetName] = []
+        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.swiftCompilerPath.pathString]
+            if desc.isTestDiscoveryTarget {
+                generatedSourceTargets.append(target.c99name)
+            }
+        }
+        generatedSourceTargets.append(contentsOf: plan.graph.allTargets.filter {$0.type == .plugin}
+                                                                       .map { $0.c99name })
+        self.swiftTargetScanArgs = targetCommandLines
+        self.generatedSourceTargetSet = Set(generatedSourceTargets)
         self.builtTestProducts = plan.buildProducts.filter{ $0.product.type == .test }.map { desc in
             return BuiltTestProduct(
                 productName: desc.product.name,

Sources/Build/BuildPlan.swift

@@ -889,23 +889,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.swiftCompilerPath.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")

Sources/Commands/Options.swift

@@ -335,6 +335,11 @@ struct BuildOptions: ParsableArguments {
     @Flag()
     var useIntegratedSwiftDriver: Bool = false
 
+    /// A flag that inidcates this build should check whether targets only import
+    /// their explicitly-declared dependencies
+    @Option()
+    var explicitTargetDependencyImportCheck: TargetDependencyImportCheckingMode = .none
+
     /// Whether to use the explicit module build flow (with the integrated driver)
     @Flag(name: .customLong("experimental-explicit-module-build"))
     var useExplicitModuleBuild: Bool = false
@@ -370,6 +375,12 @@ struct BuildOptions: ParsableArguments {
         case native
         case xcode
     }
+
+    enum TargetDependencyImportCheckingMode : String, Codable, ExpressibleByArgument {
+        case none
+        case warn
+        case error
+    }
 }
 
 struct LinkerOptions: ParsableArguments {

Sources/Commands/SwiftTool.swift

@@ -795,6 +795,7 @@ public class SwiftTool {
                 useExplicitModuleBuild: options.build.useExplicitModuleBuild,
                 isXcodeBuildSystemEnabled: options.build.buildSystem == .xcode,
                 forceTestDiscovery: options.build.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery
+                explicitTargetDependencyImportCheckingMode: options.build.explicitTargetDependencyImportCheck.modeParameter,
                 linkerDeadStrip: options.linker.linkerDeadStrip,
                 verboseOutput: self.logLevel <= .info
             )
@@ -1187,6 +1188,19 @@ extension BuildOptions.StoreMode {
     }
 }
 
+extension BuildOptions.TargetDependencyImportCheckingMode {
+    fileprivate var modeParameter: BuildParameters.TargetDependencyImportCheckingMode {
+        switch self {
+        case .none:
+            return .none
+        case .warn:
+            return .warn
+        case .error:
+            return .error
+        }
+    }
+}
+
 extension Basics.Diagnostic.Severity {
     fileprivate var isVerbose: Bool {
         return self <= .info

Sources/SPMBuildCore/BuildParameters.swift

@@ -72,6 +72,13 @@ public struct BuildParameters: Encodable {
         }
     }
 
+    /// A mode for explicit import checking
+    public enum TargetDependencyImportCheckingMode : Codable {
+        case none
+        case warn
+        case error
+    }
+
     /// The path to the data directory.
     public var dataPath: AbsolutePath
 
@@ -128,6 +135,10 @@ public struct BuildParameters: Encodable {
     /// Whether to use the explicit module build flow (with the integrated driver)
     public var useExplicitModuleBuild: Bool
 
+    /// A flag that inidcates this build should check whether targets only import
+    /// their explicitly-declared dependencies
+    public var explicitTargetDependencyImportCheckingMode: TargetDependencyImportCheckingMode
+
     /// Whether to create dylibs for dynamic library products.
     public var shouldCreateDylibForDynamicProducts: Bool
 
@@ -199,6 +210,7 @@ public struct BuildParameters: Encodable {
         isXcodeBuildSystemEnabled: Bool = false,
         enableTestability: Bool? = nil,
         forceTestDiscovery: Bool = false,
+        explicitTargetDependencyImportCheckingMode: TargetDependencyImportCheckingMode = .none,
         linkerDeadStrip: Bool = true,
         colorizedOutput: Bool = false,
         verboseOutput: Bool = false
@@ -236,6 +248,7 @@ public struct BuildParameters: Encodable {
         self.enableTestability = enableTestability ?? (.debug == configuration)
         // decide if to enable the use of test manifests based on platform. this is likely to change in the future
         self.testDiscoveryStrategy = triple.isDarwin() ? .objectiveC : .manifest(generate: forceTestDiscovery)
+        self.explicitTargetDependencyImportCheckingMode = explicitTargetDependencyImportCheckingMode
         self.linkerDeadStrip = linkerDeadStrip
         self.colorizedOutput = colorizedOutput
         self.verboseOutput = verboseOutput

Tests/CommandsTests/BuildToolTests.swift

@@ -75,6 +75,49 @@ final class BuildToolTests: CommandsTestCase {
         }
     }
 
+    func testImportOfMissedDepWarning() throws {
+        #if swift(<5.5)
+        try XCTSkipIf(true, "skipping because host compiler doesn't support '-import-prescan'")
+        #endif
+        // Verify the warning flow
+        try fixture(name: "Miscellaneous/ImportOfMissingDependency") { path in
+            let fullPath = resolveSymlinks(path)
+            XCTAssertThrowsError(try build(["--explicit-target-dependency-import-check=warn"], 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."))
+            }
+        }
+
+        // Verify the error flow
+        try fixture(name: "Miscellaneous/ImportOfMissingDependency") { path in
+            let fullPath = resolveSymlinks(path)
+            XCTAssertThrowsError(try build(["--explicit-target-dependency-import-check=error"], packagePath: fullPath)) { error in
+                guard case SwiftPMProductError.executionFailure(_, _, let stderr) = error else {
+                    XCTFail()
+                    return
+                }
+
+                XCTAssertTrue(stderr.contains("error: Target A imports another target (B) in the package without declaring it a dependency."))
+            }
+        }
+
+        // Verify that the default does not run the check
+        try 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
+                }
+                XCTAssertFalse(stderr.contains("warning: Target A imports another target (B) in the package without declaring it a dependency."))
+            }
+        }
+    }
+
     func testBinPathAndSymlink() throws {
         try fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in
             let fullPath = resolveSymlinks(fixturePath)