Merge pull request swiftlang#1533 from ahoppen/background-indexing-configuration-option

Make background indexing a proper option in `SourceKitLSPOptions`

Author: ahoppen

Sources/Diagnose/IndexCommand.swift

@@ -82,7 +82,10 @@ public struct IndexCommand: AsyncParsableCommand {
   public init() {}
 
   public func run() async throws {
-    let options = SourceKitLSPOptions(experimentalFeatures: Set(experimentalFeatures).union([.backgroundIndexing]))
+    let options = SourceKitLSPOptions(
+      backgroundIndexing: true,
+      experimentalFeatures: Set(experimentalFeatures)
+    )
 
     let installPath =
       if let toolchainOverride, let toolchain = Toolchain(try AbsolutePath(validating: toolchainOverride)) {

Sources/SKCore/SourceKitLSPOptions.swift

@@ -140,6 +140,18 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     public var maxCoresPercentageToUseForBackgroundIndexing: Double?
     public var updateIndexStoreTimeout: Int?
 
+    public var maxCoresPercentageToUseForBackgroundIndexingOrDefault: Double {
+      return maxCoresPercentageToUseForBackgroundIndexing ?? 1
+    }
+
+    public var updateIndexStoreTimeoutOrDefault: Duration {
+      if let updateIndexStoreTimeout {
+        .seconds(updateIndexStoreTimeout)
+      } else {
+        .seconds(120)
+      }
+    }
+
     public init(
       indexStorePath: String? = nil,
       indexDatabasePath: String? = nil,
@@ -166,16 +178,19 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     }
   }
 
-  public var swiftPM: SwiftPMOptions?
-  public var compilationDatabase: CompilationDatabaseOptions?
-  public var fallbackBuildSystem: FallbackBuildSystemOptions?
+  public var swiftPM: SwiftPMOptions
+  public var compilationDatabase: CompilationDatabaseOptions
+  public var fallbackBuildSystem: FallbackBuildSystemOptions
   public var clangdOptions: [String]?
-  public var index: IndexOptions?
+  public var index: IndexOptions
 
   /// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic.
   public var defaultWorkspaceType: WorkspaceType?
   public var generatedFilesPath: String?
 
+  /// Whether background indexing is enabled.
+  public var backgroundIndexing: Bool?
+
   /// Experimental features that are enabled.
   public var experimentalFeatures: Set<ExperimentalFeature>? = nil
 
@@ -184,25 +199,29 @@ public struct SourceKitLSPOptions: Sendable, Codable {
   ///
   /// This is mostly intended for testing purposes so we don't need to wait the debouncing time to get a diagnostics
   /// notification when running unit tests.
-  public var swiftPublishDiagnosticsDebounce: Double? = nil
+  public var swiftPublishDiagnosticsDebounceDuration: Double? = nil
 
-  public var swiftPublishDiagnosticsDebounceDuration: TimeInterval {
-    if let workDoneProgressDebounce {
-      return workDoneProgressDebounce
+  public var swiftPublishDiagnosticsDebounceDurationOrDefault: Duration {
+    if let swiftPublishDiagnosticsDebounceDuration {
+      return .seconds(swiftPublishDiagnosticsDebounceDuration)
     }
-    return 2 /* seconds */
+    return .seconds(2)
   }
 
   /// When a task is started that should be displayed to the client as a work done progress, how many milliseconds to
   /// wait before actually starting the work done progress. This prevents flickering of the work done progress in the
   /// client for short-lived index tasks which end within this duration.
-  public var workDoneProgressDebounce: Double? = nil
+  public var workDoneProgressDebounceDuration: Double? = nil
 
-  public var workDoneProgressDebounceDuration: Duration {
-    if let workDoneProgressDebounce {
-      return .seconds(workDoneProgressDebounce)
+  public var workDoneProgressDebounceDurationOrDefault: Duration {
+    if let workDoneProgressDebounceDuration {
+      return .seconds(workDoneProgressDebounceDuration)
     }
-    return .seconds(0)
+    return .seconds(1)
+  }
+
+  public var backgroundIndexingOrDefault: Bool {
+    return backgroundIndexing ?? false
   }
 
   public init(
@@ -213,9 +232,10 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     index: IndexOptions = .init(),
     defaultWorkspaceType: WorkspaceType? = nil,
     generatedFilesPath: String? = nil,
+    backgroundIndexing: Bool? = nil,
     experimentalFeatures: Set<ExperimentalFeature>? = nil,
-    swiftPublishDiagnosticsDebounce: Double? = nil,
-    workDoneProgressDebounce: Double? = nil
+    swiftPublishDiagnosticsDebounceDuration: Double? = nil,
+    workDoneProgressDebounceDuration: Double? = nil
   ) {
     self.swiftPM = swiftPM
     self.fallbackBuildSystem = fallbackBuildSystem
@@ -224,9 +244,10 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     self.index = index
     self.generatedFilesPath = generatedFilesPath
     self.defaultWorkspaceType = defaultWorkspaceType
+    self.backgroundIndexing = backgroundIndexing
     self.experimentalFeatures = experimentalFeatures
-    self.swiftPublishDiagnosticsDebounce = swiftPublishDiagnosticsDebounce
-    self.workDoneProgressDebounce = workDoneProgressDebounce
+    self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
+    self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration
   }
 
   public init?(path: URL?) {
@@ -236,7 +257,7 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     guard
       let decoded = orLog(
         "Parsing config.json",
-        { try JSONDecoder().decode(SourceKitLSPOptions.self, from: contents) }
+        { try JSONDecoder().decode(Self.self, from: contents) }
       )
     else {
       return nil
@@ -246,23 +267,25 @@ public struct SourceKitLSPOptions: Sendable, Codable {
 
   public static func merging(base: SourceKitLSPOptions, override: SourceKitLSPOptions?) -> SourceKitLSPOptions {
     return SourceKitLSPOptions(
-      swiftPM: SwiftPMOptions.merging(base: base.swiftPM ?? .init(), override: override?.swiftPM),
+      swiftPM: SwiftPMOptions.merging(base: base.swiftPM, override: override?.swiftPM),
       fallbackBuildSystem: FallbackBuildSystemOptions.merging(
-        base: base.fallbackBuildSystem ?? .init(),
+        base: base.fallbackBuildSystem,
         override: override?.fallbackBuildSystem
       ),
       compilationDatabase: CompilationDatabaseOptions.merging(
-        base: base.compilationDatabase ?? .init(),
+        base: base.compilationDatabase,
         override: override?.compilationDatabase
       ),
       clangdOptions: override?.clangdOptions ?? base.clangdOptions,
-      index: IndexOptions.merging(base: base.index ?? .init(), override: override?.index),
+      index: IndexOptions.merging(base: base.index, override: override?.index),
       defaultWorkspaceType: override?.defaultWorkspaceType ?? base.defaultWorkspaceType,
       generatedFilesPath: override?.generatedFilesPath ?? base.generatedFilesPath,
+      backgroundIndexing: override?.backgroundIndexing ?? base.backgroundIndexing,
       experimentalFeatures: override?.experimentalFeatures ?? base.experimentalFeatures,
-      swiftPublishDiagnosticsDebounce: override?.swiftPublishDiagnosticsDebounce
-        ?? base.swiftPublishDiagnosticsDebounce,
-      workDoneProgressDebounce: override?.workDoneProgressDebounce ?? base.workDoneProgressDebounce
+      swiftPublishDiagnosticsDebounceDuration: override?.swiftPublishDiagnosticsDebounceDuration
+        ?? base.swiftPublishDiagnosticsDebounceDuration,
+      workDoneProgressDebounceDuration: override?.workDoneProgressDebounceDuration
+        ?? base.workDoneProgressDebounceDuration
     )
   }
 
@@ -279,4 +302,34 @@ public struct SourceKitLSPOptions: Sendable, Codable {
     }
     return experimentalFeatures.contains(feature)
   }
+
+  public init(from decoder: any Decoder) throws {
+    let container = try decoder.container(keyedBy: CodingKeys.self)
+
+    self.swiftPM = try container.decodeIfPresent(SwiftPMOptions.self, forKey: CodingKeys.swiftPM) ?? .init()
+    self.compilationDatabase =
+      try container.decodeIfPresent(CompilationDatabaseOptions.self, forKey: CodingKeys.compilationDatabase) ?? .init()
+    self.fallbackBuildSystem =
+      try container.decodeIfPresent(FallbackBuildSystemOptions.self, forKey: CodingKeys.fallbackBuildSystem) ?? .init()
+    self.clangdOptions = try container.decodeIfPresent([String].self, forKey: CodingKeys.clangdOptions)
+    self.index = try container.decodeIfPresent(IndexOptions.self, forKey: CodingKeys.index) ?? .init()
+    self.defaultWorkspaceType = try container.decodeIfPresent(
+      WorkspaceType.self,
+      forKey: CodingKeys.defaultWorkspaceType
+    )
+    self.generatedFilesPath = try container.decodeIfPresent(String.self, forKey: CodingKeys.generatedFilesPath)
+    self.backgroundIndexing = try container.decodeIfPresent(Bool.self, forKey: CodingKeys.backgroundIndexing)
+    self.experimentalFeatures = try container.decodeIfPresent(
+      Set<ExperimentalFeature>.self,
+      forKey: CodingKeys.experimentalFeatures
+    )
+    self.swiftPublishDiagnosticsDebounceDuration = try container.decodeIfPresent(
+      Double.self,
+      forKey: CodingKeys.swiftPublishDiagnosticsDebounceDuration
+    )
+    self.workDoneProgressDebounceDuration = try container.decodeIfPresent(
+      Double.self,
+      forKey: CodingKeys.workDoneProgressDebounceDuration
+    )
+  }
 }

Sources/SKSupport/ExperimentalFeatures.swift

@@ -13,9 +13,6 @@
 /// An experimental feature that can be enabled by passing `--experimental-feature` to `sourcekit-lsp` on the command
 /// line. The raw value of this feature is how it is named on the command line.
 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

@@ -131,7 +131,7 @@ public actor SwiftPMBuildSystem {
   private let workspacePath: TSCAbsolutePath
 
   /// Options that allow the user to pass extra compiler flags.
-  private let options: SourceKitLSPOptions.SwiftPMOptions
+  private let options: SourceKitLSPOptions
 
   /// The directory containing `Package.swift`.
   @_spi(Testing)
@@ -175,12 +175,9 @@ public actor SwiftPMBuildSystem {
     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 var isForIndexBuild: Bool { experimentalFeatures.contains(.backgroundIndexing) }
+  private var isForIndexBuild: Bool { options.backgroundIndexingOrDefault }
 
   private let testHooks: SwiftPMTestHooks
 
@@ -196,8 +193,7 @@ public actor SwiftPMBuildSystem {
     workspacePath: TSCAbsolutePath,
     toolchainRegistry: ToolchainRegistry,
     fileSystem: FileSystem = localFileSystem,
-    options: SourceKitLSPOptions.SwiftPMOptions,
-    experimentalFeatures: Set<ExperimentalFeature>,
+    options: SourceKitLSPOptions,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in },
     testHooks: SwiftPMTestHooks
   ) async throws {
@@ -209,7 +205,6 @@ public actor SwiftPMBuildSystem {
     }
 
     self.toolchain = toolchain
-    self.experimentalFeatures = experimentalFeatures
     self.testHooks = testHooks
 
     guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
@@ -250,9 +245,9 @@ public actor SwiftPMBuildSystem {
       forRootPackage: AbsolutePath(packageRoot),
       fileSystem: fileSystem
     )
-    if experimentalFeatures.contains(.backgroundIndexing) {
+    if options.backgroundIndexingOrDefault {
       location.scratchDirectory = AbsolutePath(packageRoot.appending(component: ".index-build"))
-    } else if let scratchDirectory = options.scratchPath,
+    } else if let scratchDirectory = options.swiftPM.scratchPath,
       let scratchDirectoryPath = try? AbsolutePath(validating: scratchDirectory)
     {
       location.scratchDirectory = scratchDirectoryPath
@@ -269,18 +264,18 @@ public actor SwiftPMBuildSystem {
     )
 
     let buildConfiguration: PackageModel.BuildConfiguration
-    switch options.configuration {
+    switch options.swiftPM.configuration {
     case .debug, nil:
       buildConfiguration = .debug
     case .release:
       buildConfiguration = .release
     }
 
     let buildFlags = BuildFlags(
-      cCompilerFlags: options.cCompilerFlags ?? [],
-      cxxCompilerFlags: options.cxxCompilerFlags ?? [],
-      swiftCompilerFlags: options.swiftCompilerFlags ?? [],
-      linkerFlags: options.linkerFlags ?? []
+      cCompilerFlags: options.swiftPM.cCompilerFlags ?? [],
+      cxxCompilerFlags: options.swiftPM.cxxCompilerFlags ?? [],
+      swiftCompilerFlags: options.swiftPM.swiftCompilerFlags ?? [],
+      linkerFlags: options.swiftPM.linkerFlags ?? []
     )
 
     self.toolsBuildParameters = try BuildParameters(
@@ -334,8 +329,7 @@ public actor SwiftPMBuildSystem {
   public init?(
     uri: DocumentURI,
     toolchainRegistry: ToolchainRegistry,
-    options: SourceKitLSPOptions.SwiftPMOptions,
-    experimentalFeatures: Set<ExperimentalFeature>,
+    options: SourceKitLSPOptions,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void,
     testHooks: SwiftPMTestHooks
   ) async {
@@ -348,7 +342,6 @@ public actor SwiftPMBuildSystem {
         toolchainRegistry: toolchainRegistry,
         fileSystem: localFileSystem,
         options: options,
-        experimentalFeatures: experimentalFeatures,
         reloadPackageStatusCallback: reloadPackageStatusCallback,
         testHooks: testHooks
       )
@@ -635,14 +628,14 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       "--disable-index-store",
       "--target", target.targetID,
     ]
-    if let configuration = options.configuration {
+    if let configuration = options.swiftPM.configuration {
       arguments += ["-c", configuration.rawValue]
     }
-    arguments += options.cCompilerFlags?.flatMap { ["-Xcc", $0] } ?? []
-    arguments += options.cxxCompilerFlags?.flatMap { ["-Xcxx", $0] } ?? []
-    arguments += options.swiftCompilerFlags?.flatMap { ["-Xswiftc", $0] } ?? []
-    arguments += options.linkerFlags?.flatMap { ["-Xlinker", $0] } ?? []
-    if experimentalFeatures.contains(.swiftpmPrepareForIndexing) {
+    arguments += options.swiftPM.cCompilerFlags?.flatMap { ["-Xcc", $0] } ?? []
+    arguments += options.swiftPM.cxxCompilerFlags?.flatMap { ["-Xcxx", $0] } ?? []
+    arguments += options.swiftPM.swiftCompilerFlags?.flatMap { ["-Xswiftc", $0] } ?? []
+    arguments += options.swiftPM.linkerFlags?.flatMap { ["-Xlinker", $0] } ?? []
+    if options.hasExperimentalFeature(.swiftpmPrepareForIndexing) {
       arguments.append("--experimental-prepare-for-indexing")
     }
     if Task.isCancelled {

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -24,7 +24,7 @@ import XCTest
 
 extension SourceKitLSPOptions {
   public static func testDefault(experimentalFeatures: Set<ExperimentalFeature>? = nil) -> SourceKitLSPOptions {
-    return SourceKitLSPOptions(experimentalFeatures: experimentalFeatures, swiftPublishDiagnosticsDebounce: 0)
+    return SourceKitLSPOptions(experimentalFeatures: experimentalFeatures, swiftPublishDiagnosticsDebounceDuration: 0)
   }
 }
 
@@ -108,13 +108,10 @@ public final class TestSourceKitLSPClient: MessageHandler, Sendable {
   ) async throws {
     var options = options
     if let globalModuleCache {
-      options.swiftPM = options.swiftPM ?? SourceKitLSPOptions.SwiftPMOptions()
-      options.swiftPM!.swiftCompilerFlags =
-        (options.swiftPM!.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
-    }
-    if enableBackgroundIndexing {
-      options.experimentalFeatures = (options.experimentalFeatures ?? []).union([.backgroundIndexing])
+      options.swiftPM.swiftCompilerFlags =
+        (options.swiftPM.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
     }
+    options.backgroundIndexing = enableBackgroundIndexing
 
     var notificationYielder: AsyncStream<any NotificationType>.Continuation!
     self.notifications = AsyncStream { continuation in

Sources/SourceKitLSP/CreateBuildSystem.swift

@@ -38,8 +38,7 @@ func createBuildSystem(
     return await SwiftPMBuildSystem(
       uri: rootUri,
       toolchainRegistry: toolchainRegistry,
-      options: options.swiftPM ?? .init(),
-      experimentalFeatures: options.experimentalFeatures ?? [],
+      options: options,
       reloadPackageStatusCallback: reloadPackageStatusCallback,
       testHooks: testHooks.swiftpmTestHooks
     )
@@ -48,7 +47,7 @@ func createBuildSystem(
   func createCompilationDatabaseBuildSystem(rootPath: AbsolutePath) -> CompilationDatabaseBuildSystem? {
     return CompilationDatabaseBuildSystem(
       projectRoot: rootPath,
-      searchPaths: (options.compilationDatabase?.searchPaths ?? []).compactMap { try? RelativePath(validating: $0) }
+      searchPaths: (options.compilationDatabase.searchPaths ?? []).compactMap { try? RelativePath(validating: $0) }
     )
   }
 

Sources/SourceKitLSP/IndexProgressManager.swift

@@ -128,7 +128,7 @@ actor IndexProgressManager {
       workDoneProgress = await WorkDoneProgressManager(
         server: sourceKitLSPServer,
         tokenPrefix: "indexing",
-        initialDebounce: sourceKitLSPServer.options.workDoneProgressDebounceDuration,
+        initialDebounce: sourceKitLSPServer.options.workDoneProgressDebounceDurationOrDefault,
         title: "Indexing",
         message: message,
         percentage: percentage

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -214,7 +214,7 @@ public actor SourceKitLSPServer {
 
     self.client = client
     let processorCount = ProcessInfo.processInfo.processorCount
-    let lowPriorityCores = (options.index?.maxCoresPercentageToUseForBackgroundIndexing ?? 1) * Double(processorCount)
+    let lowPriorityCores = options.index.maxCoresPercentageToUseForBackgroundIndexingOrDefault * Double(processorCount)
     self.indexTaskScheduler = TaskScheduler(maxConcurrentTasksByPriority: [
       (TaskPriority.medium, processorCount),
       (TaskPriority.low, max(Int(lowPriorityCores), 1)),
@@ -940,7 +940,7 @@ extension SourceKitLSPServer {
         self?.indexProgressManager.indexProgressStatusDidChange()
       }
     )
-    if let workspace, options.hasExperimentalFeature(.backgroundIndexing), workspace.semanticIndexManager == nil,
+    if let workspace, options.backgroundIndexingOrDefault, workspace.semanticIndexManager == nil,
       !self.didSendBackgroundIndexingNotSupportedNotification
     {
       self.sendNotificationToClient(