Remove custom decoding function from SourceKitLSPOptions

We forgot to decode the following keys in the custom decode function, which meant that you couldn’t set them using SourceKit-LSP’s `config.json` file.
- `backgroundPreparationMode`
- `sourcekitdRequestTimeout`
- `cancelTextDocumentRequestsOnEditAndClose`

We had the custom decoder function so that the keys weren’t required in the JSON but we could access eg. `SwiftPMOptions` without needing to deal with optionals in the codebase.

Make the accesses to these nested options structs a little more verbose but eliminate the source of the above bug, which seems like a good tradeoff.

Author: ahoppen

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -229,11 +229,11 @@ package actor SwiftPMBuildSystem {
     let destinationSDK = try SwiftSDK.deriveTargetSwiftSDK(
       hostSwiftSDK: hostSDK,
       hostTriple: hostSwiftPMToolchain.targetTriple,
-      customCompileTriple: options.swiftPM.triple.map { try Triple($0) },
-      swiftSDKSelector: options.swiftPM.swiftSDK,
+      customCompileTriple: options.swiftPMOrDefault.triple.map { try Triple($0) },
+      swiftSDKSelector: options.swiftPMOrDefault.swiftSDK,
       store: SwiftSDKBundleStore(
         swiftSDKsDirectory: fileSystem.getSharedSwiftSDKsDirectory(
-          explicitDirectory: options.swiftPM.swiftSDKsDirectory.map { try AbsolutePath(validating: $0) }
+          explicitDirectory: options.swiftPMOrDefault.swiftSDKsDirectory.map { try AbsolutePath(validating: $0) }
         ),
         fileSystem: fileSystem,
         observabilityScope: observabilitySystem.topScope,
@@ -251,7 +251,7 @@ package actor SwiftPMBuildSystem {
     )
     if options.backgroundIndexingOrDefault {
       location.scratchDirectory = AbsolutePath(packageRoot.appending(component: ".index-build"))
-    } else if let scratchDirectory = options.swiftPM.scratchPath,
+    } else if let scratchDirectory = options.swiftPMOrDefault.scratchPath,
       let scratchDirectoryPath = try? AbsolutePath(validating: scratchDirectory)
     {
       location.scratchDirectory = scratchDirectoryPath
@@ -267,25 +267,25 @@ package actor SwiftPMBuildSystem {
       customHostToolchain: hostSwiftPMToolchain,
       customManifestLoader: ManifestLoader(
         toolchain: hostSwiftPMToolchain,
-        isManifestSandboxEnabled: !(options.swiftPM.disableSandbox ?? false),
+        isManifestSandboxEnabled: !(options.swiftPMOrDefault.disableSandbox ?? false),
         cacheDir: location.sharedManifestsCacheDirectory,
         importRestrictions: configuration.manifestImportRestrictions
       )
     )
 
     let buildConfiguration: PackageModel.BuildConfiguration
-    switch options.swiftPM.configuration {
+    switch options.swiftPMOrDefault.configuration {
     case .debug, nil:
       buildConfiguration = .debug
     case .release:
       buildConfiguration = .release
     }
 
     let buildFlags = BuildFlags(
-      cCompilerFlags: options.swiftPM.cCompilerFlags ?? [],
-      cxxCompilerFlags: options.swiftPM.cxxCompilerFlags ?? [],
-      swiftCompilerFlags: options.swiftPM.swiftCompilerFlags ?? [],
-      linkerFlags: options.swiftPM.linkerFlags ?? []
+      cCompilerFlags: options.swiftPMOrDefault.cCompilerFlags ?? [],
+      cxxCompilerFlags: options.swiftPMOrDefault.cxxCompilerFlags ?? [],
+      swiftCompilerFlags: options.swiftPMOrDefault.swiftCompilerFlags ?? [],
+      linkerFlags: options.swiftPMOrDefault.linkerFlags ?? []
     )
 
     self.toolsBuildParameters = try BuildParameters(
@@ -381,7 +381,7 @@ extension SwiftPMBuildSystem {
       destinationBuildParameters: destinationBuildParameters,
       toolsBuildParameters: toolsBuildParameters,
       graph: modulesGraph,
-      disableSandbox: options.swiftPM.disableSandbox ?? false,
+      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
       fileSystem: fileSystem,
       observabilityScope: observabilitySystem.topScope
     )
@@ -607,16 +607,16 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
       "--disable-index-store",
       "--target", target.targetID,
     ]
-    if options.swiftPM.disableSandbox ?? false {
+    if options.swiftPMOrDefault.disableSandbox ?? false {
       arguments += ["--disable-sandbox"]
     }
-    if let configuration = options.swiftPM.configuration {
+    if let configuration = options.swiftPMOrDefault.configuration {
       arguments += ["-c", configuration.rawValue]
     }
-    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] } ?? []
+    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] } ?? []
     switch options.backgroundPreparationModeOrDefault {
     case .build: break
     case .noLazy: arguments += ["--experimental-prepare-for-indexing", "--experimental-prepare-for-indexing-no-lazy"]

Sources/SKOptions/SourceKitLSPOptions.swift

@@ -20,8 +20,8 @@ import struct TSCBasic.AbsolutePath
 /// Options that can be used to modify SourceKit-LSP's behavior.
 ///
 /// See `ConfigurationFile.md` for a description of the configuration file's behavior.
-public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible {
-  public struct SwiftPMOptions: Sendable, Codable, CustomLogStringConvertible {
+public struct SourceKitLSPOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
+  public struct SwiftPMOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
     /// Build configuration (debug|release).
     ///
     /// Equivalent to SwiftPM's `--configuration` option.
@@ -104,7 +104,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     }
   }
 
-  public struct CompilationDatabaseOptions: Sendable, Codable, CustomLogStringConvertible {
+  public struct CompilationDatabaseOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
     /// Additional paths to search for a compilation database, relative to a workspace root.
     public var searchPaths: [String]?
 
@@ -128,7 +128,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     }
   }
 
-  public struct FallbackBuildSystemOptions: Sendable, Codable, CustomLogStringConvertible {
+  public struct FallbackBuildSystemOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
     public var cCompilerFlags: [String]?
     public var cxxCompilerFlags: [String]?
     public var swiftCompilerFlags: [String]?
@@ -163,7 +163,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     }
   }
 
-  public struct IndexOptions: Sendable, Codable, CustomLogStringConvertible {
+  public struct IndexOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
     public var indexStorePath: String?
     public var indexDatabasePath: String?
     public var indexPrefixMap: [String: String]?
@@ -216,7 +216,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     }
   }
 
-  public struct LoggingOptions: Sendable, Codable, CustomLogStringConvertible {
+  public struct LoggingOptions: Sendable, Codable, Equatable, CustomLogStringConvertible {
     /// The level from which one onwards log messages should be written.
     public var level: String?
     /// Whether potentially sensitive information should be redacted.
@@ -259,12 +259,37 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     case enabled
   }
 
-  public var swiftPM: SwiftPMOptions
-  public var compilationDatabase: CompilationDatabaseOptions
-  public var fallbackBuildSystem: FallbackBuildSystemOptions
+  private var swiftPM: SwiftPMOptions?
+  public var swiftPMOrDefault: SwiftPMOptions {
+    get { swiftPM ?? .init() }
+    set { swiftPM = newValue }
+  }
+
+  private var compilationDatabase: CompilationDatabaseOptions?
+  public var compilationDatabaseOrDefault: CompilationDatabaseOptions {
+    get { compilationDatabase ?? .init() }
+    set { compilationDatabase = newValue }
+  }
+
+  private var fallbackBuildSystem: FallbackBuildSystemOptions?
+  public var fallbackBuildSystemOrDefault: FallbackBuildSystemOptions {
+    get { fallbackBuildSystem ?? .init() }
+    set { fallbackBuildSystem = newValue }
+  }
+
   public var clangdOptions: [String]?
-  public var index: IndexOptions
-  public var logging: LoggingOptions
+
+  private var index: IndexOptions?
+  public var indexOrDefault: IndexOptions {
+    get { index ?? .init() }
+    set { index = newValue }
+  }
+
+  private var logging: LoggingOptions?
+  public var loggingOrDefault: LoggingOptions {
+    get { logging ?? .init() }
+    set { logging = newValue }
+  }
 
   /// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic.
   public var defaultWorkspaceType: WorkspaceType?
@@ -406,18 +431,18 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
 
   public static func merging(base: SourceKitLSPOptions, override: SourceKitLSPOptions?) -> SourceKitLSPOptions {
     return SourceKitLSPOptions(
-      swiftPM: SwiftPMOptions.merging(base: base.swiftPM, override: override?.swiftPM),
+      swiftPM: SwiftPMOptions.merging(base: base.swiftPMOrDefault, override: override?.swiftPM),
       fallbackBuildSystem: FallbackBuildSystemOptions.merging(
-        base: base.fallbackBuildSystem,
+        base: base.fallbackBuildSystemOrDefault,
         override: override?.fallbackBuildSystem
       ),
       compilationDatabase: CompilationDatabaseOptions.merging(
-        base: base.compilationDatabase,
+        base: base.compilationDatabaseOrDefault,
         override: override?.compilationDatabase
       ),
       clangdOptions: override?.clangdOptions ?? base.clangdOptions,
-      index: IndexOptions.merging(base: base.index, override: override?.index),
-      logging: LoggingOptions.merging(base: base.logging, override: override?.logging),
+      index: IndexOptions.merging(base: base.indexOrDefault, override: override?.index),
+      logging: LoggingOptions.merging(base: base.loggingOrDefault, override: override?.logging),
       defaultWorkspaceType: override?.defaultWorkspaceType ?? base.defaultWorkspaceType,
       generatedFilesPath: override?.generatedFilesPath ?? base.generatedFilesPath,
       backgroundIndexing: override?.backgroundIndexing ?? base.backgroundIndexing,
@@ -447,37 +472,6 @@ public struct SourceKitLSPOptions: Sendable, Codable, CustomLogStringConvertible
     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.logging = try container.decodeIfPresent(LoggingOptions.self, forKey: .logging) ?? .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
-    )
-  }
-
   public var description: String {
     recursiveDescription(of: self)
   }

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -121,7 +121,10 @@ package struct IndexedSingleSwiftFileTestProject {
 
     // Create the test client
     var options = SourceKitLSPOptions.testDefault()
-    options.index = SourceKitLSPOptions.IndexOptions(indexStorePath: indexURL.path, indexDatabasePath: indexDBURL.path)
+    options.indexOrDefault = SourceKitLSPOptions.IndexOptions(
+      indexStorePath: indexURL.path,
+      indexDatabasePath: indexDBURL.path
+    )
     self.testClient = try await TestSourceKitLSPClient(
       options: options,
       workspaceFolders: [

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -112,8 +112,8 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
   ) async throws {
     var options = options
     if let globalModuleCache {
-      options.swiftPM.swiftCompilerFlags =
-        (options.swiftPM.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
+      options.swiftPMOrDefault.swiftCompilerFlags =
+        (options.swiftPMOrDefault.swiftCompilerFlags ?? []) + ["-module-cache-path", globalModuleCache.path]
     }
     options.backgroundIndexing = enableBackgroundIndexing
 

Sources/SourceKitLSP/CreateBuildSystem.swift

@@ -48,7 +48,9 @@ func createBuildSystem(
   func createCompilationDatabaseBuildSystem(rootPath: AbsolutePath) -> CompilationDatabaseBuildSystem? {
     return CompilationDatabaseBuildSystem(
       projectRoot: rootPath,
-      searchPaths: (options.compilationDatabase.searchPaths ?? []).compactMap { try? RelativePath(validating: $0) }
+      searchPaths: (options.compilationDatabaseOrDefault.searchPaths ?? []).compactMap {
+        try? RelativePath(validating: $0)
+      }
     )
   }
 

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -225,7 +225,8 @@ package actor SourceKitLSPServer {
 
     self.client = client
     let processorCount = ProcessInfo.processInfo.processorCount
-    let lowPriorityCores = options.index.maxCoresPercentageToUseForBackgroundIndexingOrDefault * Double(processorCount)
+    let lowPriorityCores =
+      options.indexOrDefault.maxCoresPercentageToUseForBackgroundIndexingOrDefault * Double(processorCount)
     self.indexTaskScheduler = TaskScheduler(maxConcurrentTasksByPriority: [
       (TaskPriority.medium, processorCount),
       (TaskPriority.low, max(Int(lowPriorityCores), 1)),

Sources/SourceKitLSP/Workspace.swift

@@ -107,15 +107,15 @@ package final class Workspace: Sendable {
     self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex)
     self.buildSystemManager = await BuildSystemManager(
       buildSystem: underlyingBuildSystem,
-      fallbackBuildSystem: FallbackBuildSystem(options: options.fallbackBuildSystem),
+      fallbackBuildSystem: FallbackBuildSystem(options: options.fallbackBuildSystemOrDefault),
       mainFilesProvider: uncheckedIndex,
       toolchainRegistry: toolchainRegistry
     )
     if options.backgroundIndexingOrDefault, let uncheckedIndex, await buildSystemManager.supportsPreparation {
       self.semanticIndexManager = SemanticIndexManager(
         index: uncheckedIndex,
         buildSystemManager: buildSystemManager,
-        updateIndexStoreTimeout: options.index.updateIndexStoreTimeoutOrDefault,
+        updateIndexStoreTimeout: options.indexOrDefault.updateIndexStoreTimeoutOrDefault,
         testHooks: testHooks.indexTestHooks,
         indexTaskScheduler: indexTaskScheduler,
         logMessageToIndexLog: logMessageToIndexLog,
@@ -163,7 +163,7 @@ package final class Workspace: Sendable {
     var index: IndexStoreDB? = nil
     var indexDelegate: SourceKitIndexDelegate? = nil
 
-    let indexOptions = options.index
+    let indexOptions = options.indexOrDefault
     if let storePath = await firstNonNil(
       AbsolutePath(validatingOrNil: indexOptions.indexStorePath),
       await buildSystem?.indexStorePath

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -277,10 +277,12 @@ struct SourceKitLSP: AsyncParsableCommand {
     }
 
     let globalConfigurationOptions = globalConfigurationOptions
-    if let logLevelStr = globalConfigurationOptions.logging.level, let logLevel = NonDarwinLogLevel(logLevelStr) {
+    if let logLevelStr = globalConfigurationOptions.loggingOrDefault.level,
+      let logLevel = NonDarwinLogLevel(logLevelStr)
+    {
       LogConfig.logLevel.value = logLevel
     }
-    if let privacyLevelStr = globalConfigurationOptions.logging.privacyLevel,
+    if let privacyLevelStr = globalConfigurationOptions.loggingOrDefault.privacyLevel,
       let privacyLevel = NonDarwinLogPrivacy(privacyLevelStr)
     {
       LogConfig.privacyLevel.value = privacyLevel

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -965,7 +965,7 @@ final class BackgroundIndexingTests: XCTestCase {
 
   func testUseBuildFlagsDuringPreparation() async throws {
     var options = SourceKitLSPOptions.testDefault()
-    options.swiftPM.swiftCompilerFlags = ["-D", "MY_FLAG"]
+    options.swiftPMOrDefault.swiftCompilerFlags = ["-D", "MY_FLAG"]
     let project = try await SwiftPMTestProject(
       files: [
         "Lib/Lib.swift": """
@@ -1390,7 +1390,7 @@ final class BackgroundIndexingTests: XCTestCase {
 
     var options = SourceKitLSPOptions.testDefault()
     options.backgroundPreparationMode = SourceKitLSPOptions.BackgroundPreparationMode.enabled.rawValue
-    options.index.updateIndexStoreTimeout = 1 /* second */
+    options.indexOrDefault.updateIndexStoreTimeout = 1 /* second */
 
     let dateStarted = Date()
     _ = try await SwiftPMTestProject(

Tests/SourceKitLSPTests/LifecycleTests.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import LanguageServerProtocol
+import SKOptions
 import SKTestSupport
 import XCTest
 
@@ -39,6 +40,13 @@ final class LifecycleTests: XCTestCase {
     XCTAssertNotNil(initResult.capabilities.completionProvider)
   }
 
+  func testEmptySourceKitLSPOptionsCanBeDecoded() {
+    // Check that none of the keys in `SourceKitLSPOptions` are required.
+    assertNoThrow {
+      try JSONDecoder().decode(SourceKitLSPOptions.self, from: "{}") == SourceKitLSPOptions()
+    }
+  }
+
   func testCancellation() async throws {
     let testClient = try await TestSourceKitLSPClient()
     let uri = DocumentURI(for: .swift)