Allow overriding the workspace buildsystem per workspace

Merge options prior to determining the buildsystem to use for a
workspace, rather than after.

Do not search up the directory tree when determining a build system for
the given workspaces. Implicit workspaces may search up to the workspace
roots, but no further.

Author: bnbarham

Documentation/Configuration File.md

@@ -43,7 +43,7 @@ The structure of the file is currently not guaranteed to be stable. Options may
   - `logLevel: "debug"|"info"|"default"|"error"|"fault"`: The level from which one onwards log messages should be written.
   - `privacyLevel: "public"|"private"|"sensitive"`: Whether potentially sensitive information should be redacted. Default is `public`, which redacts potentially sensitive information.
   - `inputMirrorDirectory: string`: Write all input received by SourceKit-LSP on stdin to a file in this directory. Useful to record and replay an entire SourceKit-LSP session.
-- `defaultWorkspaceType: "buildserver"|"compdb"|"swiftpm"`: Overrides workspace type selection logic.
+- `defaultWorkspaceType: "swiftPM"|"compilationDatabase"|"buildServer"`: Overrides workspace type selection logic.
 - `generatedFilesPath: string`: Directory in which generated interfaces and macro expansions should be stored.
 - `backgroundIndexing: bool`: Explicitly enable or disable background indexing.
 - `backgroundPreparationMode: "build"|"noLazy"|"enabled"`: Determines how background indexing should prepare a target. Possible values are:

Sources/BuildSystemIntegration/DetermineBuildSystem.swift

@@ -31,11 +31,13 @@ import struct TSCBasic.AbsolutePath
 #endif
 
 /// Determine which build system should be started to handle the given workspace folder and at which folder that build
-/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`).
+/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`). `onlyConsiderRoot` controls whether
+/// paths outside the root should be considered (eg. configuration files in the user's home directory).
 ///
 /// Returns `nil` if no build system can handle this workspace folder.
 package func determineBuildSystem(
   forWorkspaceFolder workspaceFolder: DocumentURI,
+  onlyConsiderRoot: Bool,
   options: SourceKitLSPOptions
 ) -> BuildSystemSpec? {
   var buildSystemPreference: [WorkspaceType] = [

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -230,17 +230,12 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     guard var path = orLog("Getting realpath for project root", { try path.realpath }) else {
       return nil
     }
-    while true {
-      let packagePath = path.appendingPathComponent("Package.swift")
-      if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
-        return path
-      }
 
-      if (try? AbsolutePath(validating: path.filePath))?.isRoot ?? true {
-        break
-      }
-      path.deleteLastPathComponent()
+    let packagePath = path.appendingPathComponent("Package.swift")
+    if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
+      return path
     }
+
     return nil
   }
 

Sources/SKOptions/SourceKitLSPOptions.swift

@@ -280,7 +280,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     set { logging = newValue }
   }
 
-  /// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic.
+  /// Default workspace type (swiftPM|compilationDatabase|buildServer). Overrides workspace type selection logic.
   public var defaultWorkspaceType: WorkspaceType?
   public var generatedFilesPath: String?
 
@@ -444,6 +444,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     )
   }
 
+  public static func merging(base: SourceKitLSPOptions, workspaceFolder: DocumentURI) -> SourceKitLSPOptions {
+    return SourceKitLSPOptions.merging(
+      base: base,
+      override: SourceKitLSPOptions(
+        path: workspaceFolder.fileURL?
+          .appendingPathComponent(".sourcekit-lsp")
+          .appendingPathComponent("config.json")
+      )
+    )
+  }
+
   public var generatedFilesAbsolutePath: URL {
     if let generatedFilesPath {
       return URL(fileURLWithPath: generatedFilesPath)

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -209,29 +209,40 @@ package actor SourceKitLSPServer {
     guard var url = uri.fileURL?.deletingLastPathComponent() else {
       return nil
     }
-    let projectRoots = await self.workspacesAndIsImplicit.filter { !$0.isImplicit }.asyncCompactMap {
+
+    // Roots of opened workspaces - only consider explicit here (all implicit must necessarily be subdirectories of
+    // the explicit workspace roots)
+    let workspaceRoots = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL }
+
+    // We want to skip creating another workspace if the project roots are the same. This could happen if an existing
+    // workspace hasn't reloaded after a new file was added to it (and thus that build system needs to be reloaded).
+    let projectRoots = await workspacesAndIsImplicit.asyncCompactMap {
       await $0.workspace.buildSystemManager.projectRoot
     }
-    let rootURLs = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL }
-    while url.pathComponents.count > 1 && rootURLs.contains(where: { $0.isPrefix(of: url) }) {
+
+    while url.pathComponents.count > 1 && workspaceRoots.contains(where: { $0.isPrefix(of: url) }) {
       defer {
         url.deleteLastPathComponent()
       }
-      // Ignore workspaces that have the same project root as an existing workspace.
-      // This might happen if there is an existing SwiftPM workspace that hasn't been reloaded after a new file
-      // was added to it and thus currently doesn't know that it can handle that file. In that case, we shouldn't open
-      // a new workspace for the same root. Instead, the existing workspace's build system needs to be reloaded.
+
       let uri = DocumentURI(url)
-      guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, options: self.options) else {
+      let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: uri)
+
+      // Some build systems consider paths outside of the folder (eg. BSP has settings in the home directory). If we
+      // allowed those paths, then the very first folder that the file is in would always be its own build system - so
+      // skip them in that case.
+      guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, onlyConsiderRoot: true, options: options) else {
         continue
       }
       guard !projectRoots.contains(buildSystemSpec.projectRoot) else {
         continue
       }
+
+      // No existing workspace matches this root - create one.
       guard
         let workspace = await orLog(
-          "Creating workspace",
-          { try await createWorkspace(workspaceFolder: uri, buildSystemSpec: buildSystemSpec) }
+          "Creating implicit workspace",
+          { try await createWorkspace(workspaceFolder: uri, options: options, buildSystemSpec: buildSystemSpec) }
         )
       else {
         continue
@@ -818,26 +829,20 @@ extension SourceKitLSPServer {
 
   /// Creates a workspace at the given `uri`.
   ///
-  /// If the build system that was determined for the workspace does not satisfy `condition`, `nil` is returned.
+  /// A workspace does not necessarily have any buildsystem attached to it, in which case `buildSystemSpec` may be
+  /// `nil` - consider eg. a top level workspace folder with multiple SwiftPM projects inside it.
   private func createWorkspace(
     workspaceFolder: DocumentURI,
+    options: SourceKitLSPOptions,
     buildSystemSpec: BuildSystemSpec?
   ) async throws -> Workspace {
     guard let capabilityRegistry = capabilityRegistry else {
       struct NoCapabilityRegistryError: Error {}
       logger.log("Cannot open workspace before server is initialized")
       throw NoCapabilityRegistryError()
     }
-    let testHooks = self.testHooks
-    let options = SourceKitLSPOptions.merging(
-      base: self.options,
-      override: SourceKitLSPOptions(
-        path: workspaceFolder.fileURL?
-          .appendingPathComponent(".sourcekit-lsp")
-          .appendingPathComponent("config.json")
-      )
-    )
-    logger.log("Creating workspace at \(workspaceFolder.forLogging) with options: \(options.forLogging)")
+
+    logger.log("Creating workspace at \(workspaceFolder.forLogging)")
     logger.logFullObjectInMultipleLogMessages(header: "Options for workspace", options.loggingProxy)
 
     let workspace = await Workspace(
@@ -848,7 +853,7 @@ extension SourceKitLSPServer {
       buildSystemSpec: buildSystemSpec,
       toolchainRegistry: self.toolchainRegistry,
       options: options,
-      testHooks: testHooks,
+      testHooks: self.testHooks,
       indexTaskScheduler: indexTaskScheduler
     )
     return workspace
@@ -857,9 +862,12 @@ extension SourceKitLSPServer {
   /// Determines the build system for the given workspace folder and creates a `Workspace` that uses this inferred build
   /// system.
   private func createWorkspaceWithInferredBuildSystem(workspaceFolder: DocumentURI) async throws -> Workspace {
+    let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: workspaceFolder)
+    let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: workspaceFolder, onlyConsiderRoot: false, options: options)
     return try await self.createWorkspace(
       workspaceFolder: workspaceFolder,
-      buildSystemSpec: determineBuildSystem(forWorkspaceFolder: workspaceFolder, options: self.options)
+      options: options,
+      buildSystemSpec: buildSystemSpec
     )
   }
 

Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift

@@ -976,9 +976,9 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         .appendingPathComponent("pkg")
         .appendingPathComponent("Sources")
         .appendingPathComponent("lib")
-      let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault())
 
-      assertEqual(projectRoot, tempDir.appendingPathComponent("pkg", isDirectory: true))
+      let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault())
+      assertNil(projectRoot)
     }
   }
 

Tests/SourceKitLSPTests/WorkspaceTests.swift

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import BuildSystemIntegration
 import Foundation
 import LanguageServerProtocol
 import SKLogging
@@ -967,4 +968,116 @@ final class WorkspaceTests: XCTestCase {
       ["Cannot convert value of type 'Int' to specified type 'String'"]
     )
   }
+
+  func testWorkspaceOptionsOverrideBuildSystem() async throws {
+    let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc)
+    let sdkArgs =
+      if let defaultSDKPath {
+        """
+        -sdk '\(defaultSDKPath)'
+        """
+      } else {
+        ""
+      }
+
+    let project = try await MultiFileTestProject(files: [
+      ".sourcekit-lsp/config.json": """
+      {
+          "defaultWorkspaceType": "compilationDatabase"
+      }
+      """,
+      "src/Foo.swift": """
+      #if HAVE_SETTINGS
+      #error("Have settings")
+      #endif
+      """,
+      "Sources/MyLib/Bar.swift": "",
+      "build/compile_commands.json": """
+      [
+      {
+        "directory": "$TEST_DIR",
+        "command": "\(swiftc.filePath) $TEST_DIR/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)",
+        "file": "src/Foo.swift",
+        "output": "$TEST_DIR/build/Foo.swift.o"
+      },
+      ]
+      """,
+      "Package.swift": """
+      // swift-tools-version: 5.7
+
+      import PackageDescription
+
+      let package = Package(
+        name: "MyLib",
+        targets: [
+          .target(name: "MyLib"),
+        ]
+      )
+      """,
+    ])
+    let (uri, _) = try project.openDocument("Foo.swift")
+    let diagnostics = try await project.testClient.send(
+      DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
+    )
+    XCTAssertEqual(
+      diagnostics.fullReport?.items.map(\.message),
+      ["Have settings"]
+    )
+  }
+
+  func testImplicitWorkspaceOptionsOverrideBuildSystem() async throws {
+    let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc)
+    let sdkArgs =
+      if let defaultSDKPath {
+        """
+        -sdk '\(defaultSDKPath)'
+        """
+      } else {
+        ""
+      }
+
+    let project = try await MultiFileTestProject(files: [
+      "projA/.sourcekit-lsp/config.json": """
+      {
+          "defaultWorkspaceType": "compilationDatabase"
+      }
+      """,
+      "projA/src/Foo.swift": """
+      #if HAVE_SETTINGS
+      #error("Have settings")
+      #endif
+      """,
+      "projA/Sources/MyLib/Bar.swift": "",
+      "projA/build/compile_commands.json": """
+      [
+      {
+        "directory": "$TEST_DIR/projA",
+        "command": "\(swiftc.filePath) $TEST_DIR/projA/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)",
+        "file": "src/Foo.swift",
+        "output": "$TEST_DIR/projA/build/Foo.swift.o"
+      },
+      ]
+      """,
+      "projA/Package.swift": """
+      // swift-tools-version: 5.7
+
+      import PackageDescription
+
+      let package = Package(
+        name: "MyLib",
+        targets: [
+          .target(name: "MyLib"),
+        ]
+      )
+      """,
+    ])
+    let (uri, _) = try project.openDocument("Foo.swift")
+    let diagnostics = try await project.testClient.send(
+      DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
+    )
+    XCTAssertEqual(
+      diagnostics.fullReport?.items.map(\.message),
+      ["Have settings"]
+    )
+  }
 }