Use the new SwiftPM API to load the build plan

bnbarham · bnbarham · commit fda2bbe9e184 · 2025-02-10T18:07:37.000-08:00
We previously skipped building/running tool plugins here, which meant
that the compiler arguments for a target also missed any generated
sources. Use the new `BuildDescription.load` API from SwiftPM to address
this.

Resolves rdar://102242345.
diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
@@ -33,15 +33,15 @@ package import BuildServerProtocol
 package import Foundation
 package import LanguageServerProtocol
 package import SKOptions
-package import SourceKitLSPAPI
+@preconcurrency package import SourceKitLSPAPI
 package import ToolchainRegistry
 package import class ToolchainRegistry.Toolchain
 #else
 import BuildServerProtocol
 import Foundation
 import LanguageServerProtocol
 import SKOptions
-import SourceKitLSPAPI
+@preconcurrency import SourceKitLSPAPI
 import ToolchainRegistry
 import class ToolchainRegistry.Toolchain
 #endif
@@ -131,6 +131,8 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   private let toolchain: Toolchain
   private let swiftPMWorkspace: Workspace
 
+  private let pluginConfiguration: PluginConfiguration
+
   /// A `ObservabilitySystem` from `SwiftPM` that logs.
   private let observabilitySystem: ObservabilitySystem
 
@@ -170,13 +172,10 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   ) async throws {
     self.projectRoot = projectRoot
     self.options = options
-    self.fileWatchers =
-      try ["Package.swift", "Package@swift*.swift", "Package.resolved"].map {
-        FileSystemWatcher(globPattern: try projectRoot.appendingPathComponent($0).filePath, kind: [.change])
-      }
-      + FileRuleDescription.builtinRules.flatMap({ $0.fileTypes }).map { fileExtension in
-        FileSystemWatcher(globPattern: "**/*.\(fileExtension)", kind: [.create, .change, .delete])
-      }
+    // We could theoretically dynamically register all known files when we get back the build graph, but that seems
+    // more errorprone than just watching everything and then filtering when we need to (eg. in
+    // `SemanticIndexManager.filesDidChange`).
+    self.fileWatchers = [FileSystemWatcher(globPattern: "**/*", kind: [.create, .change, .delete])]
     let toolchain = await toolchainRegistry.preferredToolchain(containing: [
       \.clang, \.clangd, \.sourcekitd, \.swift, \.swiftc,
     ])
@@ -291,6 +290,19 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       prepareForIndexing: options.backgroundPreparationModeOrDefault.toSwiftPMPreparation
     )
 
+    let pluginScriptRunner = DefaultPluginScriptRunner(
+      fileSystem: localFileSystem,
+      cacheDir: location.pluginWorkingDirectory.appending("cache"),
+      toolchain: hostSwiftPMToolchain,
+      extraPluginSwiftCFlags: [],
+      enableSandbox: !(options.swiftPMOrDefault.disableSandbox ?? false)
+    )
+    self.pluginConfiguration = PluginConfiguration(
+      scriptRunner: pluginScriptRunner,
+      workDirectory: location.pluginWorkingDirectory,
+      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false
+    )
+
     packageLoadingQueue.async {
       await orLog("Initial package loading") {
         // Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
@@ -334,24 +346,47 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
 
     signposter.emitEvent("Finished loading modules graph", id: signpostID)
 
-    let plan = try await BuildPlan(
-      destinationBuildParameters: destinationBuildParameters,
-      toolsBuildParameters: toolsBuildParameters,
-      graph: modulesGraph,
-      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
-      fileSystem: localFileSystem,
-      observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build plan")
-    )
+    // We have a whole separate arena if we're performing background indexing. This allows us to also build and run
+    // plugins, without having to worry about messing up any regular build state.
+    let buildDescription: SourceKitLSPAPI.BuildDescription
+    if isForIndexBuild {
+      let loaded = try await BuildDescription.load(
+        destinationBuildParameters: destinationBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        packageGraph: modulesGraph,
+        pluginConfiguration: pluginConfiguration,
+        disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
+        scratchDirectory: swiftPMWorkspace.location.scratchDirectory.asURL,
+        fileSystem: localFileSystem,
+        observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build description")
+      )
+      if !loaded.errors.isEmpty {
+        logger.error("Loading SwiftPM description had errors: \(loaded.errors)")
+      }
 
-    signposter.emitEvent("Finished generating build plan", id: signpostID)
+      signposter.emitEvent("Finished generating build description", id: signpostID)
 
-    let buildDescription = BuildDescription(buildPlan: plan)
-    self.buildDescription = buildDescription
+      buildDescription = loaded.description
+    } else {
+      let plan = try await BuildPlan(
+        destinationBuildParameters: destinationBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        graph: modulesGraph,
+        disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
+        fileSystem: localFileSystem,
+        observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build plan")
+      )
+
+      signposter.emitEvent("Finished generating build plan", id: signpostID)
+
+      buildDescription = BuildDescription(buildPlan: plan)
+    }
 
     /// Make sure to execute any throwing statements before setting any
     /// properties because otherwise we might end up in an inconsistent state
     /// with only some properties modified.
 
+    self.buildDescription = buildDescription
     self.swiftPMTargets = [:]
     self.targetDependencies = [:]
 
diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift
@@ -385,15 +385,36 @@ package final actor SemanticIndexManager {
     let changedFiles = events.map(\.uri)
     await indexStoreUpToDateTracker.markOutOfDate(changedFiles)
 
-    let targets = await changedFiles.asyncMap { await buildSystemManager.targets(for: $0) }.flatMap { $0 }
-    let dependentTargets = await buildSystemManager.targets(dependingOn: Set(targets))
-    logger.info(
-      """
-      Marking targets as out-of-date: \
-      \(String(dependentTargets.map(\.uri.stringValue).joined(separator: ", ")))
-      """
-    )
-    await preparationUpToDateTracker.markOutOfDate(dependentTargets)
+    // Preparation tracking should be per file. For now consider any non-known-language change as having to re-prepare
+    // the target itself so that we re-prepare potential input files to plugins.
+    // https://github.com/swiftlang/sourcekit-lsp/issues/1975
+    let targets = await changedFiles.asyncMap {
+      let targets = await buildSystemManager.targets(for: $0)
+      let outOfDate = Language.init(inferredFromFileExtension: $0) == nil
+      return (identifiers: targets, forceOutOfDate: outOfDate)
+    }
+
+    let outOfDateTargets = Set(targets.lazy.flatMap { $0.forceOutOfDate ? $0.identifiers : [] })
+    if !outOfDateTargets.isEmpty {
+      logger.info(
+        """
+        Marking targets as out-of-date: \
+        \(String(outOfDateTargets.map(\.uri.stringValue).joined(separator: ", ")))
+        """
+      )
+      await preparationUpToDateTracker.markOutOfDate(outOfDateTargets)
+    }
+
+    let dependentTargets = await buildSystemManager.targets(dependingOn: Set(targets.lazy.flatMap { $0.identifiers }))
+    if !dependentTargets.isEmpty {
+      logger.info(
+        """
+        Marking dependent targets as out-of-date: \
+        \(String(dependentTargets.map(\.uri.stringValue).joined(separator: ", ")))
+        """
+      )
+      await preparationUpToDateTracker.markOutOfDate(dependentTargets)
+    }
 
     await scheduleBuildGraphGenerationAndBackgroundIndexAllFiles(
       filesToIndex: changedFiles,
diff --git a/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift b/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift
@@ -359,4 +359,164 @@ final class SwiftPMIntegrationTests: XCTestCase {
       ["Cannot convert value of type 'Int' to specified type 'String'"]
     )
   }
+
+  func testToolPluginWithBuild() async throws {
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Plugins/plugin.swift": #"""
+        import PackagePlugin
+        @main struct CodeGeneratorPlugin: BuildToolPlugin {
+          func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
+            let genSourcesDir = context.pluginWorkDirectoryURL.appending(path: "GeneratedSources")
+            guard let target = target as? SourceModuleTarget else { return [] }
+            let codeGenerator = try context.tool(named: "CodeGenerator").url
+            let generatedFile = genSourcesDir.appending(path: "\(target.name)-generated.swift")
+            return [.buildCommand(
+              displayName: "Generating code for \(target.name)",
+              executable: codeGenerator,
+              arguments: [
+                generatedFile.path
+              ],
+              inputFiles: [],
+              outputFiles: [generatedFile]
+            )]
+          }
+        }
+        """#,
+
+        "Sources/CodeGenerator/CodeGenerator.swift": #"""
+        import Foundation
+        try "let generated = 1".write(to: URL(fileURLWithPath: CommandLine.arguments[1]), atomically: true, encoding: String.Encoding.utf8)
+        """#,
+
+        "Sources/TestLib/TestLib.swift": #"""
+        func useGenerated() {
+          _ = 2️⃣generated
+        }
+        """#,
+      ],
+      manifest: """
+        // swift-tools-version: 6.0
+        import PackageDescription
+        let package = Package(
+          name: "PluginTest",
+          targets: [
+            .executableTarget(name: "CodeGenerator"),
+            .target(
+              name: "TestLib",
+              plugins: [.plugin(name: "CodeGeneratorPlugin")]
+            ),
+            .plugin(
+              name: "CodeGeneratorPlugin",
+              capability: .buildTool(),
+              dependencies: ["CodeGenerator"]
+            ),
+          ]
+        )
+        """,
+      enableBackgroundIndexing: false
+    )
+
+    let (uri, positions) = try project.openDocument("TestLib.swift")
+    let result = try await project.testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    // We cannot run plugins when not using background indexing, so we expect no result here.
+    XCTAssertNil(result)
+  }
+
+  func testToolPluginWithBackgroundIndexing() async throws {
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Plugins/plugin.swift": #"""
+        import Foundation
+        import PackagePlugin
+        @main struct CodeGeneratorPlugin: BuildToolPlugin {
+          func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
+            let genSourcesDir = context.pluginWorkDirectoryURL.appending(path: "GeneratedSources")
+            guard let target = target as? SourceModuleTarget else { return [] }
+            let codeGenerator = try context.tool(named: "CodeGenerator").url
+            let generatedFile = genSourcesDir.appending(path: "\(target.name)-generated.swift")
+            return [.buildCommand(
+              displayName: "Generating code for \(target.name)",
+              executable: codeGenerator,
+              arguments: [
+                generatedFile.path
+              ],
+              inputFiles: [URL(fileURLWithPath: "$TEST_DIR/Sources/TestLib/dep.txt")],
+              outputFiles: [generatedFile]
+            )]
+          }
+        }
+        """#,
+
+        "Sources/CodeGenerator/CodeGenerator.swift": #"""
+        import Foundation
+        let generated = try String(contentsOf: URL(fileURLWithPath: "$TEST_DIR/Sources/TestLib/dep.txt"))
+        try generated.write(to: URL(fileURLWithPath: CommandLine.arguments[1]), atomically: true, encoding: String.Encoding.utf8)
+        """#,
+
+        "Sources/TestLib/TestLib.swift": #"""
+        func useGenerated() {
+          _ = 2️⃣generated
+        }
+        """#,
+
+        "Sources/TestLib/dep.txt": "let generated = 1",
+      ],
+      manifest: """
+        // swift-tools-version: 6.0
+        import PackageDescription
+        let package = Package(
+          name: "PluginTest",
+          targets: [
+            .executableTarget(name: "CodeGenerator"),
+            .target(
+              name: "TestLib",
+              plugins: [.plugin(name: "CodeGeneratorPlugin")]
+            ),
+            .plugin(
+              name: "CodeGeneratorPlugin",
+              capability: .buildTool(),
+              dependencies: ["CodeGenerator"]
+            ),
+          ]
+        )
+        """,
+      enableBackgroundIndexing: true
+    )
+
+    let (uri, positions) = try project.openDocument("TestLib.swift")
+
+    // We should have run plugins and thus created generated.swift
+    do {
+      let result = try await project.testClient.send(
+        DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+      )
+      let location = try XCTUnwrap(result?.locations?.only)
+      XCTAssertTrue(location.uri.pseudoPath.hasSuffix("generated.swift"))
+      XCTAssertEqual(location.range.lowerBound, Position(line: 0, utf16index: 4))
+      XCTAssertEqual(location.range.upperBound, Position(line: 0, utf16index: 4))
+    }
+
+    // Make a change to the input file of the plugin command
+    let depUri = try project.uri(for: "dep.txt")
+    let depUrl = try XCTUnwrap(depUri.fileURL)
+    try "// some change\nlet generated = 1".write(
+      to: depUrl,
+      atomically: true,
+      encoding: .utf8
+    )
+    project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: depUri, type: .changed)]))
+    try await project.testClient.send(PollIndexRequest())
+
+    // Expect that the position has been updated in the dependency
+    try await repeatUntilExpectedResult {
+      let result = try await project.testClient.send(
+        DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+      )
+      let location = try XCTUnwrap(result?.locations?.only)
+      return location.range.lowerBound == Position(line: 1, utf16index: 4)
+    }
+  }
 }
