Make passing --experimental-prepare-for-indexing to swift build an experimental feature

I’d like to qualify `--experimental-prepare-for-indexing` independently of background indexing using `swift build`. Because of this, I think there is value in using SourceKit-LSP using background indexing but without `--experimental-prepare-for-indexing`. It could also be useful to determine if bugs we may find are due to `--experimental-prepare-for-indexing` or also occur when running plain `swift build` commands.

Author: ahoppen

Sources/Diagnose/IndexCommand.swift

@@ -74,7 +74,7 @@ public struct IndexCommand: AsyncParsableCommand {
 
   public func run() async throws {
     var serverOptions = SourceKitLSPServer.Options()
-    serverOptions.experimentalFeatures.append(.backgroundIndexing)
+    serverOptions.experimentalFeatures.insert(.backgroundIndexing)
 
     let installPath =
       if let toolchainOverride, let toolchain = Toolchain(try AbsolutePath(validating: toolchainOverride)) {

Sources/SKCore/CMakeLists.txt

@@ -8,6 +8,7 @@ add_library(SKCore STATIC
   CompilationDatabase.swift
   CompilationDatabaseBuildSystem.swift
   Debouncer.swift
+  ExperimentalFeatures.swift
   FallbackBuildSystem.swift
   FileBuildSettings.swift
   IndexProcessResult.swift

Sources/SourceKitLSP/ExperimentalFeatures.swift renamed to Sources/SKCore/ExperimentalFeatures.swift

@@ -15,4 +15,7 @@
 public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable {
   /// Enable background indexing.
   case backgroundIndexing = "background-indexing"
+
+  /// Add `--experimental-prepare-for-indexing` to the `swift build` command run to prepare a target for indexing.
+  case swiftpmPrepareForIndexing = "swiftpm-prepare-for-indexing"
 }

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -95,71 +95,36 @@ public actor SwiftPMBuildSystem {
   }
 
   /// Callbacks that should be called if the list of possible test files has changed.
-  public var testFilesDidChangeCallbacks: [() async -> Void] = []
+  private var testFilesDidChangeCallbacks: [() async -> Void] = []
 
-  let workspacePath: TSCAbsolutePath
+  private let workspacePath: TSCAbsolutePath
   /// The directory containing `Package.swift`.
+  @_spi(Testing)
   public var projectRoot: TSCAbsolutePath
-  var modulesGraph: ModulesGraph
-  let workspace: Workspace
-  public let toolsBuildParameters: BuildParameters
-  public let destinationBuildParameters: BuildParameters
-  let fileSystem: FileSystem
+  private var modulesGraph: ModulesGraph
+  private let workspace: Workspace
+  @_spi(Testing) public let toolsBuildParameters: BuildParameters
+  @_spi(Testing) public let destinationBuildParameters: BuildParameters
+  private let fileSystem: FileSystem
   private let toolchainRegistry: ToolchainRegistry
 
   private let swiftBuildSupportsPrepareForIndexingTask = SKSupport.ThreadSafeBox<Task<Bool, Never>?>(initialValue: nil)
 
-  #if compiler(>=6.1)
-  #warning(
-    "Remove swiftBuildSupportsPrepareForIndexing when we no longer need to support SwiftPM versions that don't have support for `--experimental-prepare-for-indexing`"
-  )
-  #endif
-  /// Whether `swift build` supports the `--experimental-prepare-for-indexing` flag.
-  private var swiftBuildSupportsPrepareForIndexing: Bool {
-    get async {
-      let task = swiftBuildSupportsPrepareForIndexingTask.withLock { task in
-        if let task {
-          return task
-        }
-        let newTask = Task { () -> Bool in
-          guard let swift = await toolchainRegistry.default?.swift else {
-            return false
-          }
-
-          do {
-            let process = Process(args: swift.pathString, "build", "--help-hidden")
-            try process.launch()
-            let result = try await process.waitUntilExit()
-            guard let output = String(bytes: try result.output.get(), encoding: .utf8) else {
-              return false
-            }
-            return output.contains("--experimental-prepare-for-indexing")
-          } catch {
-            return false
-          }
-        }
-        task = newTask
-        return newTask
-      }
 
-      return await task.value
-    }
-  }
-
-  var fileToTarget: [DocumentURI: SwiftBuildTarget] = [:]
-  var sourceDirToTarget: [DocumentURI: SwiftBuildTarget] = [:]
+  private var fileToTarget: [DocumentURI: SwiftBuildTarget] = [:]
+  private var sourceDirToTarget: [DocumentURI: SwiftBuildTarget] = [:]
 
   /// Maps configured targets ids to their SwiftPM build target as well as an index in their topological sorting.
   ///
   /// Targets with lower index are more low level, ie. targets with higher indices depend on targets with lower indices.
-  var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:]
+  private var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:]
 
   /// The URIs for which the delegate has registered for change notifications,
   /// mapped to the language the delegate specified when registering for change notifications.
-  var watchedFiles: Set<DocumentURI> = []
+  private var watchedFiles: Set<DocumentURI> = []
 
   /// This callback is informed when `reloadPackage` starts and ends executing.
-  var reloadPackageStatusCallback: (ReloadPackageStatus) async -> Void
+  private var reloadPackageStatusCallback: (ReloadPackageStatus) async -> Void
 
   /// Debounces calls to `delegate.filesDependenciesUpdated`.
   ///
@@ -169,16 +134,19 @@ public actor SwiftPMBuildSystem {
   /// `fileDependenciesUpdated` call once for every updated file within the target.
   ///
   /// Force-unwrapped optional because initializing it requires access to `self`.
-  var fileDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil
+  private var fileDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil
 
   /// A `ObservabilitySystem` from `SwiftPM` that logs.
   private let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
     logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)")
   })
 
+  /// Whether to pass `--experimental-prepare-for-indexing` to `swift build` as part of preparation.
+  private let experimentalFeatures: Set<ExperimentalFeature>
+
   /// Whether the `SwiftPMBuildSystem` is pointed at a `.index-build` directory that's independent of the
   /// user's build.
-  private let isForIndexBuild: Bool
+  private var isForIndexBuild: Bool { experimentalFeatures.contains(.backgroundIndexing) }
 
   /// Creates a build system using the Swift Package Manager, if this workspace is a package.
   ///
@@ -193,13 +161,13 @@ public actor SwiftPMBuildSystem {
     toolchainRegistry: ToolchainRegistry,
     fileSystem: FileSystem = localFileSystem,
     buildSetup: BuildSetup,
-    isForIndexBuild: Bool,
+    experimentalFeatures: Set<ExperimentalFeature>,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in }
   ) async throws {
     self.workspacePath = workspacePath
     self.fileSystem = fileSystem
     self.toolchainRegistry = toolchainRegistry
-    self.isForIndexBuild = isForIndexBuild
+    self.experimentalFeatures = experimentalFeatures
 
     guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
       throw Error.noManifest(workspacePath: workspacePath)
@@ -218,7 +186,7 @@ public actor SwiftPMBuildSystem {
       forRootPackage: AbsolutePath(packageRoot),
       fileSystem: fileSystem
     )
-    if isForIndexBuild {
+    if experimentalFeatures.contains(.backgroundIndexing) {
       location.scratchDirectory = AbsolutePath(packageRoot.appending(component: ".index-build"))
     } else if let scratchDirectory = buildSetup.path {
       location.scratchDirectory = AbsolutePath(scratchDirectory)
@@ -289,7 +257,7 @@ public actor SwiftPMBuildSystem {
     uri: DocumentURI,
     toolchainRegistry: ToolchainRegistry,
     buildSetup: BuildSetup,
-    isForIndexBuild: Bool,
+    experimentalFeatures: Set<ExperimentalFeature>,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void
   ) async {
     guard let fileURL = uri.fileURL else {
@@ -301,7 +269,7 @@ public actor SwiftPMBuildSystem {
         toolchainRegistry: toolchainRegistry,
         fileSystem: localFileSystem,
         buildSetup: buildSetup,
-        isForIndexBuild: isForIndexBuild,
+        experimentalFeatures: experimentalFeatures,
         reloadPackageStatusCallback: reloadPackageStatusCallback
       )
     } catch Error.noManifest {
@@ -612,7 +580,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
     arguments += self.destinationBuildParameters.flags.swiftCompilerFlags.flatMap { ["-Xswiftc", $0] }
     arguments += self.destinationBuildParameters.flags.linkerFlags.flatMap { ["-Xlinker", $0] }
     arguments += self.destinationBuildParameters.flags.xcbuildFlags?.flatMap { ["-Xxcbuild", $0] } ?? []
-    if await swiftBuildSupportsPrepareForIndexing {
+    if experimentalFeatures.contains(.swiftpmPrepareForIndexing) {
       arguments.append("--experimental-prepare-for-indexing")
     }
     if Task.isCancelled {

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -110,7 +110,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
       serverOptions.buildSetup.flags.swiftCompilerFlags += ["-module-cache-path", globalModuleCache.path]
     }
     if enableBackgroundIndexing {
-      serverOptions.experimentalFeatures.append(.backgroundIndexing)
+      serverOptions.experimentalFeatures.insert(.backgroundIndexing)
     }
 
     var notificationYielder: AsyncStream<any NotificationType>.Continuation!

Sources/SourceKitLSP/CMakeLists.txt

@@ -4,7 +4,6 @@ add_library(SourceKitLSP STATIC
   CreateBuildSystem.swift
   DocumentManager.swift
   DocumentSnapshot+FromFileContents.swift
-  ExperimentalFeatures.swift
   IndexProgressManager.swift
   IndexStoreDB+MainFilesProvider.swift
   LanguageServerType.swift

Sources/SourceKitLSP/CreateBuildSystem.swift

@@ -38,7 +38,7 @@ func createBuildSystem(
       uri: rootUri,
       toolchainRegistry: toolchainRegistry,
       buildSetup: options.buildSetup,
-      isForIndexBuild: options.experimentalFeatures.contains(.backgroundIndexing),
+      experimentalFeatures: options.experimentalFeatures,
       reloadPackageStatusCallback: reloadPackageStatusCallback
     )
   }

Sources/SourceKitLSP/SourceKitLSPServer+Options.swift

@@ -50,7 +50,7 @@ extension SourceKitLSPServer {
     public var swiftPublishDiagnosticsDebounceDuration: TimeInterval
 
     /// Experimental features that are enabled.
-    public var experimentalFeatures: [ExperimentalFeature]
+    public var experimentalFeatures: Set<ExperimentalFeature>
 
     public var indexTestHooks: IndexTestHooks
 
@@ -62,7 +62,7 @@ extension SourceKitLSPServer {
       completionOptions: SKCompletionOptions = .init(),
       generatedInterfacesPath: AbsolutePath = defaultDirectoryForGeneratedInterfaces,
       swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
-      experimentalFeatures: [ExperimentalFeature] = [],
+      experimentalFeatures: Set<ExperimentalFeature> = [],
       indexTestHooks: IndexTestHooks = IndexTestHooks()
     ) {
       self.buildSetup = buildSetup

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -224,7 +224,7 @@ struct SourceKitLSP: AsyncParsableCommand {
     serverOptions.indexOptions.indexPrefixMappings = indexPrefixMappings
     serverOptions.completionOptions.maxResults = completionMaxResults
     serverOptions.generatedInterfacesPath = generatedInterfacesPath
-    serverOptions.experimentalFeatures = experimentalFeatures
+    serverOptions.experimentalFeatures = Set(experimentalFeatures)
 
     return serverOptions
   }

Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift

@@ -15,7 +15,7 @@ import LSPTestSupport
 import LanguageServerProtocol
 import PackageModel
 @_spi(Testing) import SKCore
-import SKSwiftPMWorkspace
+@_spi(Testing) import SKSwiftPMWorkspace
 import SKTestSupport
 import SourceKitLSP
 import TSCBasic
@@ -54,7 +54,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
           toolchainRegistry: tr,
           fileSystem: fs,
           buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-          isForIndexBuild: false
+          experimentalFeatures: []
         )
       )
     }
@@ -81,7 +81,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       await assertThrowsError(try await buildSystem.generateBuildGraph(allowFileSystemWrites: false))
     }
@@ -111,7 +111,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
           toolchainRegistry: ToolchainRegistry(toolchains: []),
           fileSystem: fs,
           buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-          isForIndexBuild: false
+          experimentalFeatures: []
         )
       )
     }
@@ -142,7 +142,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -270,7 +270,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: config,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -312,7 +312,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -349,7 +349,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -398,7 +398,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -463,7 +463,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -507,7 +507,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -588,7 +588,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: ToolchainRegistry.forTesting,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -640,7 +640,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -708,7 +708,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: ToolchainRegistry.forTesting,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -748,7 +748,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
 
@@ -789,7 +789,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
 
       assertEqual(await swiftpmBuildSystem.projectRoot, try resolveSymlinks(tempDir.appending(component: "pkg")))
@@ -824,7 +824,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         toolchainRegistry: tr,
         fileSystem: fs,
         buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
-        isForIndexBuild: false
+        experimentalFeatures: []
       )
       try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)