Move logic to select the toolchain for a document from SourceKitLSPServer to BuildSystemManager

This allows us to determine the toolchain to use during background indexing. It also moves toolchain selection closer to the build system, which is good because when we support multiple toolchains for a single workspace, the build system is what decides which toolchain to use for which document.

Author: ahoppen

Sources/SKCore/BuildSystemManager.swift

@@ -50,6 +50,11 @@ public actor BuildSystemManager {
   /// Build system delegate that will receive notifications about setting changes, etc.
   var delegate: BuildSystemDelegate?
 
+  /// The list of toolchains that are available.
+  ///
+  /// Used to determine which toolchain to use for a given document.
+  private let toolchainRegistry: ToolchainRegistry
+
   /// The root of the project that this build system manages. For example, for SwiftPM packages, this is the folder
   /// containing Package.swift. For compilation databases it is the root folder based on which the compilation database
   /// was found.
@@ -65,13 +70,15 @@ public actor BuildSystemManager {
     buildSystem: BuildSystem?,
     fallbackBuildSystem: FallbackBuildSystem?,
     mainFilesProvider: MainFilesProvider?,
+    toolchainRegistry: ToolchainRegistry,
     fallbackSettingsTimeout: DispatchTimeInterval = .seconds(3)
   ) async {
     let buildSystemHasDelegate = await buildSystem?.delegate != nil
     precondition(!buildSystemHasDelegate)
     self.buildSystem = buildSystem
     self.fallbackBuildSystem = fallbackBuildSystem
     self.mainFilesProvider = mainFilesProvider
+    self.toolchainRegistry = toolchainRegistry
     self.fallbackSettingsTimeout = fallbackSettingsTimeout
     await self.buildSystem?.setDelegate(self)
   }
@@ -87,6 +94,13 @@ extension BuildSystemManager {
     self.delegate = delegate
   }
 
+  /// Returns the toolchain that should be used to process the given document.
+  public func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
+    // To support multiple toolchains within a single workspace, we need to ask the build system which toolchain to use
+    // for this document.
+    return await toolchainRegistry.defaultToolchain(for: language)
+  }
+
   /// - Note: Needed so we can set the delegate from a different isolation context.
   public func setMainFilesProvider(_ mainFilesProvider: MainFilesProvider?) {
     self.mainFilesProvider = mainFilesProvider

Sources/SKCore/ToolchainRegistry.swift

@@ -12,6 +12,7 @@
 
 import Dispatch
 import Foundation
+import LanguageServerProtocol
 import SKSupport
 
 import struct TSCBasic.AbsolutePath
@@ -244,6 +245,33 @@ public final actor ToolchainRegistry {
   public var darwinToolchainIdentifier: String {
     return darwinToolchainOverride ?? ToolchainRegistry.darwinDefaultToolchainIdentifier
   }
+
+  /// The toolchain to use for a document in the given language if the build system doesn't override it.
+  func defaultToolchain(for language: Language) -> Toolchain? {
+    let supportsLang = { (toolchain: Toolchain) -> Bool in
+      // FIXME: the fact that we're looking at clangd/sourcekitd instead of the compiler indicates this method needs a parameter stating what kind of tool we're looking for.
+      switch language {
+      case .swift:
+        return toolchain.sourcekitd != nil
+      case .c, .cpp, .objective_c, .objective_cpp:
+        return toolchain.clangd != nil
+      default:
+        return false
+      }
+    }
+
+    if let toolchain = self.default, supportsLang(toolchain) {
+      return toolchain
+    }
+
+    for toolchain in toolchains {
+      if supportsLang(toolchain) {
+        return toolchain
+      }
+    }
+
+    return nil
+  }
 }
 
 /// Inspecting internal state for testing purposes.

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -653,32 +653,6 @@ public actor SourceKitLSPServer {
     return try await client.send(request)
   }
 
-  func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
-    let supportsLang = { (toolchain: Toolchain) -> Bool in
-      // FIXME: the fact that we're looking at clangd/sourcekitd instead of the compiler indicates this method needs a parameter stating what kind of tool we're looking for.
-      switch language {
-      case .swift:
-        return toolchain.sourcekitd != nil
-      case .c, .cpp, .objective_c, .objective_cpp:
-        return toolchain.clangd != nil
-      default:
-        return false
-      }
-    }
-
-    if let toolchain = await toolchainRegistry.default, supportsLang(toolchain) {
-      return toolchain
-    }
-
-    for toolchain in await toolchainRegistry.toolchains {
-      if supportsLang(toolchain) {
-        return toolchain
-      }
-    }
-
-    return nil
-  }
-
   /// After the language service has crashed, send `DidOpenTextDocumentNotification`s to a newly instantiated language service for previously open documents.
   func reopenDocuments(for languageService: LanguageService) async {
     for documentUri in self.documentManager.openDocuments {
@@ -815,7 +789,7 @@ public actor SourceKitLSPServer {
       return service
     }
 
-    guard let toolchain = await toolchain(for: uri, language),
+    guard let toolchain = await workspace.buildSystemManager.toolchain(for: uri, language),
       let service = await languageService(for: toolchain, language, in: workspace)
     else {
       return nil

Sources/SourceKitLSP/Workspace.swift

@@ -91,7 +91,8 @@ public final class Workspace {
     self.buildSystemManager = await BuildSystemManager(
       buildSystem: underlyingBuildSystem,
       fallbackBuildSystem: FallbackBuildSystem(buildSetup: buildSetup),
-      mainFilesProvider: uncheckedIndex
+      mainFilesProvider: uncheckedIndex,
+      toolchainRegistry: toolchainRegistry
     )
     await indexDelegate?.addMainFileChangedCallback { [weak self] in
       await self?.buildSystemManager.mainFilesChanged()

Tests/SKCoreTests/BuildSystemManagerTests.swift

@@ -13,7 +13,7 @@
 import BuildServerProtocol
 import LSPTestSupport
 import LanguageServerProtocol
-import SKCore
+@_spi(Testing) import SKCore
 import TSCBasic
 import XCTest
 
@@ -37,7 +37,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: nil,
       fallbackBuildSystem: FallbackBuildSystem(buildSetup: .default),
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
 
@@ -88,13 +89,14 @@ final class BuildSystemManagerTests: XCTestCase {
   }
 
   func testSettingsMainFile() async throws {
-    let a = try try DocumentURI(string: "bsm:a.swift")
+    let a = try DocumentURI(string: "bsm:a.swift")
     let mainFiles = ManualMainFilesProvider([a: [a]])
     let bs = ManualBuildSystem()
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -117,7 +119,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -139,7 +142,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: fallback,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -168,7 +172,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -208,7 +213,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -246,7 +252,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -300,7 +307,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -341,7 +349,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)
@@ -383,7 +392,8 @@ final class BuildSystemManagerTests: XCTestCase {
     let bsm = await BuildSystemManager(
       buildSystem: bs,
       fallbackBuildSystem: nil,
-      mainFilesProvider: mainFiles
+      mainFilesProvider: mainFiles,
+      toolchainRegistry: ToolchainRegistry.forTesting
     )
     defer { withExtendedLifetime(bsm) {} }  // Keep BSM alive for callbacks.
     let del = await BSMDelegate(bsm)