Merge pull request #1973 from bnbarham/generated-source-should-work

Use the new SwiftPM API to load the build plan

Author: bnbarham

Documentation/Configuration File.md

@@ -20,10 +20,12 @@ The structure of the file is currently not guaranteed to be stable. Options may
   - `swiftSDKsDirectory: string`: Equivalent to SwiftPM's `--swift-sdks-path` option.
   - `swiftSDK: string`: Equivalent to SwiftPM's `--swift-sdk` option.
   - `triple: string`: Equivalent to SwiftPM's `--triple` option.
+  - `traits: string[]`: Traits to enable for the package. Equivalent to SwiftPM's `--traits` option.
   - `cCompilerFlags: string[]`: Extra arguments passed to the compiler for C files. Equivalent to SwiftPM's `-Xcc` option.
   - `cxxCompilerFlags: string[]`: Extra arguments passed to the compiler for C++ files. Equivalent to SwiftPM's `-Xcxx` option.
   - `swiftCompilerFlags: string[]`: Extra arguments passed to the compiler for Swift files. Equivalent to SwiftPM's `-Xswiftc` option.
   - `linkerFlags: string[]`: Extra arguments passed to the linker. Equivalent to SwiftPM's `-Xlinker` option.
+  - `buildToolsSwiftCompilerFlags: string[]`: Extra arguments passed to the compiler for Swift files or plugins. Equivalent to SwiftPM's `-Xbuild-tools-swiftc` option.
   - `disableSandbox: boolean`: Disables running subprocesses from SwiftPM in a sandbox. Equivalent to SwiftPM's `--disable-sandbox` option. Useful when running `sourcekit-lsp` in a sandbox because nested sandboxes are not supported.
 - `compilationDatabase`: Dictionary with the following keys, defining options for workspaces with a compilation database.
   - `searchPaths: string[]`: Additional paths to search for a compilation database, relative to a workspace root.

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,9 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   private let toolchain: Toolchain
   private let swiftPMWorkspace: Workspace
 
+  private let pluginConfiguration: PluginConfiguration
+  private let traitConfiguration: TraitConfiguration
+
   /// A `ObservabilitySystem` from `SwiftPM` that logs.
   private let observabilitySystem: ObservabilitySystem
 
@@ -170,13 +173,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,
     ])
@@ -250,6 +250,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
         toolchain: hostSwiftPMToolchain,
         isManifestSandboxEnabled: !(options.swiftPMOrDefault.disableSandbox ?? false),
         cacheDir: location.sharedManifestsCacheDirectory,
+        extraManifestFlags: options.swiftPMOrDefault.buildToolsSwiftCompilerFlags,
         importRestrictions: configuration.manifestImportRestrictions
       )
     )
@@ -291,6 +292,23 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       prepareForIndexing: options.backgroundPreparationModeOrDefault.toSwiftPMPreparation
     )
 
+    let pluginScriptRunner = DefaultPluginScriptRunner(
+      fileSystem: localFileSystem,
+      cacheDir: location.pluginWorkingDirectory.appending("cache"),
+      toolchain: hostSwiftPMToolchain,
+      extraPluginSwiftCFlags: options.swiftPMOrDefault.buildToolsSwiftCompilerFlags ?? [],
+      enableSandbox: !(options.swiftPMOrDefault.disableSandbox ?? false)
+    )
+    self.pluginConfiguration = PluginConfiguration(
+      scriptRunner: pluginScriptRunner,
+      workDirectory: location.pluginWorkingDirectory,
+      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false
+    )
+
+    self.traitConfiguration = TraitConfiguration(
+      enabledTraits: options.swiftPMOrDefault.traits.flatMap(Set.init)
+    )
+
     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 +352,48 @@ 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 && !(options.swiftPMOrDefault.skipPlugins ?? false) {
+      let loaded = try await BuildDescription.load(
+        destinationBuildParameters: destinationBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        packageGraph: modulesGraph,
+        pluginConfiguration: pluginConfiguration,
+        traitConfiguration: traitConfiguration,
+        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 = [:]
 
@@ -600,10 +642,14 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     if let swiftSDK = options.swiftPMOrDefault.swiftSDK {
       arguments += ["--swift-sdk", swiftSDK]
     }
+    if let traits = options.swiftPMOrDefault.traits {
+      arguments += ["--traits", traits.joined(separator: ",")]
+    }
     arguments += options.swiftPMOrDefault.cCompilerFlags?.flatMap { ["-Xcc", $0] } ?? []
     arguments += options.swiftPMOrDefault.cxxCompilerFlags?.flatMap { ["-Xcxx", $0] } ?? []
     arguments += options.swiftPMOrDefault.swiftCompilerFlags?.flatMap { ["-Xswiftc", $0] } ?? []
     arguments += options.swiftPMOrDefault.linkerFlags?.flatMap { ["-Xlinker", $0] } ?? []
+    arguments += options.swiftPMOrDefault.buildToolsSwiftCompilerFlags?.flatMap { ["-Xbuild-tools-swiftc", $0] } ?? []
     switch options.backgroundPreparationModeOrDefault {
     case .build: break
     case .noLazy: arguments += ["--experimental-prepare-for-indexing", "--experimental-prepare-for-indexing-no-lazy"]

Sources/SKOptions/SourceKitLSPOptions.swift

@@ -53,6 +53,9 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     /// Equivalent to SwiftPM's `--triple` option.
     public var triple: String?
 
+    /// Traits to enable for the package. Equivalent to SwiftPM's `--traits` option.
+    public var traits: [String]?
+
     /// Extra arguments passed to the compiler for C files. Equivalent to SwiftPM's `-Xcc` option.
     public var cCompilerFlags: [String]?
 
@@ -65,31 +68,46 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     /// Extra arguments passed to the linker. Equivalent to SwiftPM's `-Xlinker` option.
     public var linkerFlags: [String]?
 
+    /// Extra arguments passed to the compiler for Swift files or plugins. Equivalent to SwiftPM's
+    /// `-Xbuild-tools-swiftc` option.
+    public var buildToolsSwiftCompilerFlags: [String]?
+
     /// Disables running subprocesses from SwiftPM in a sandbox. Equivalent to SwiftPM's `--disable-sandbox` option.
     /// Useful when running `sourcekit-lsp` in a sandbox because nested sandboxes are not supported.
     public var disableSandbox: Bool?
 
+    /// Whether to skip building and running plugins when creating the in-memory build graph.
+    ///
+    /// - Note: Internal option, only exists as an escape hatch in case this causes unintentional interactions with
+    ///   background indexing.
+    public var skipPlugins: Bool?
+
     public init(
       configuration: BuildConfiguration? = nil,
       scratchPath: String? = nil,
       swiftSDKsDirectory: String? = nil,
       swiftSDK: String? = nil,
       triple: String? = nil,
+      traits: [String]? = nil,
       cCompilerFlags: [String]? = nil,
       cxxCompilerFlags: [String]? = nil,
       swiftCompilerFlags: [String]? = nil,
       linkerFlags: [String]? = nil,
-      disableSandbox: Bool? = nil
+      buildToolsSwiftCompilerFlags: [String]? = nil,
+      disableSandbox: Bool? = nil,
+      skipPlugins: Bool? = nil
     ) {
       self.configuration = configuration
       self.scratchPath = scratchPath
       self.swiftSDKsDirectory = swiftSDKsDirectory
       self.swiftSDK = swiftSDK
       self.triple = triple
+      self.traits = traits
       self.cCompilerFlags = cCompilerFlags
       self.cxxCompilerFlags = cxxCompilerFlags
       self.swiftCompilerFlags = swiftCompilerFlags
       self.linkerFlags = linkerFlags
+      self.buildToolsSwiftCompilerFlags = buildToolsSwiftCompilerFlags
       self.disableSandbox = disableSandbox
     }
 
@@ -100,11 +118,14 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
         swiftSDKsDirectory: override?.swiftSDKsDirectory ?? base.swiftSDKsDirectory,
         swiftSDK: override?.swiftSDK ?? base.swiftSDK,
         triple: override?.triple ?? base.triple,
+        traits: override?.traits ?? base.traits,
         cCompilerFlags: override?.cCompilerFlags ?? base.cCompilerFlags,
         cxxCompilerFlags: override?.cxxCompilerFlags ?? base.cxxCompilerFlags,
         swiftCompilerFlags: override?.swiftCompilerFlags ?? base.swiftCompilerFlags,
         linkerFlags: override?.linkerFlags ?? base.linkerFlags,
-        disableSandbox: override?.disableSandbox ?? base.disableSandbox
+        buildToolsSwiftCompilerFlags: override?.buildToolsSwiftCompilerFlags ?? base.buildToolsSwiftCompilerFlags,
+        disableSandbox: override?.disableSandbox ?? base.disableSandbox,
+        skipPlugins: override?.skipPlugins ?? base.skipPlugins
       )
     }
   }

Sources/SKTestSupport/SkipUnless.swift

@@ -270,6 +270,85 @@ package actor SkipUnless {
       }
     }
   }
+
+  package static func canLoadPluginsBuiltByToolchain(
+    file: StaticString = #filePath,
+    line: UInt = #line
+  ) async throws {
+    return try await shared.skipUnlessSupported(file: file, line: line) {
+      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: [],
+                outputFiles: [generatedFile]
+              )]
+            }
+          }
+          """#,
+
+          "Sources/CodeGenerator/CodeGenerator.swift": #"""
+          import Foundation
+          try "let foo = 1".write(
+            to: URL(fileURLWithPath: CommandLine.arguments[1]),
+            atomically: true,
+            encoding: String.Encoding.utf8
+          )
+          """#,
+
+          "Sources/TestLib/TestLib.swift": #"""
+          func useGenerated() {
+            _ = 1️⃣foo
+          }
+          """#,
+        ],
+        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")
+
+      let result = try await project.testClient.send(
+        DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+      )
+
+      if result?.locations?.only == nil {
+        return .featureUnsupported(skipMessage: "Skipping because plugin protocols do not match.")
+      }
+      return .featureSupported
+    }
+  }
 }
 
 // MARK: - Parsing Swift compiler version

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+import LanguageServerProtocolExtensions
+
 #if compiler(>=6)
 package import BuildServerProtocol
 package import BuildSystemIntegration
@@ -385,15 +387,28 @@ 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
+    var outOfDateTargets = Set<BuildTargetIdentifier>()
+    for file in changedFiles {
+      let changedTargets = await buildSystemManager.targets(for: file)
+      if Language(inferredFromFileExtension: file) == nil {
+        outOfDateTargets.formUnion(changedTargets)
+      }
+
+      let dependentTargets = await buildSystemManager.targets(dependingOn: changedTargets)
+      outOfDateTargets.formUnion(dependentTargets)
+    }
+    if !outOfDateTargets.isEmpty {
+      logger.info(
+        """
+        Marking dependent targets as out-of-date: \
+        \(String(outOfDateTargets.map(\.uri.stringValue).joined(separator: ", ")))
+        """
+      )
+      await preparationUpToDateTracker.markOutOfDate(outOfDateTargets)
+    }
 
     await scheduleBuildGraphGenerationAndBackgroundIndexAllFiles(
       filesToIndex: changedFiles,