Create BuildSystem in BuiltInBuildSystemAdapter

This finalizes the move of `BuiltInBuildSystem` creation into `BuiltInBuildSystemAdapter` and means that we can set the message handler of the `BuiltInBuildSystem` during initialization instead of using a setter method.

Author: ahoppen

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -77,15 +77,12 @@ package actor BuildServerBuildSystem: MessageHandler {
 
   package weak var messageHandler: BuiltInBuildSystemMessageHandler?
 
-  package func setMessageHandler(_ messageHandler: any BuiltInBuildSystemMessageHandler) {
-    self.messageHandler = messageHandler
-  }
-
   /// The build settings that have been received from the build server.
   private var buildSettings: [DocumentURI: FileBuildSettings] = [:]
 
   package init(
     projectRoot: AbsolutePath,
+    messageHandler: BuiltInBuildSystemMessageHandler?,
     fileSystem: FileSystem = localFileSystem
   ) async throws {
     let configPath = projectRoot.appending(component: "buildServer.json")
@@ -105,17 +102,18 @@ package actor BuildServerBuildSystem: MessageHandler {
     #endif
     self.projectRoot = projectRoot
     self.serverConfig = config
+    self.messageHandler = messageHandler
     try await self.initializeBuildServer()
   }
 
   /// Creates a build system using the Build Server Protocol config.
   ///
   /// - Returns: nil if `projectRoot` has no config or there is an error parsing it.
-  package init?(projectRoot: AbsolutePath?) async {
+  package init?(projectRoot: AbsolutePath?, messageHandler: BuiltInBuildSystemMessageHandler?) async {
     guard let projectRoot else { return nil }
 
     do {
-      try await self.init(projectRoot: projectRoot)
+      try await self.init(projectRoot: projectRoot, messageHandler: messageHandler)
     } catch is FileSystemError {
       // config file was missing, no build server for this workspace
       return nil

Sources/BuildSystemIntegration/BuildSystem.swift

@@ -17,6 +17,7 @@ import SKOptions
 import ToolchainRegistry
 
 import struct TSCBasic.AbsolutePath
+import struct TSCBasic.RelativePath
 
 /// Defines how well a `BuildSystem` can handle a file with a given URI.
 package enum FileHandlingCapability: Comparable, Sendable {
@@ -98,11 +99,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
   ///   context.
   func setDelegate(_ delegate: BuildSystemDelegate?) async
 
-  /// Set the message handler that is used to send messages from the build system to SourceKit-LSP.
-  // FIXME: (BSP Migration) This should be set in the initializer but can't right now because BuiltInBuildSystemAdapter is not
-  // responsible for creating the build system.
-  func setMessageHandler(_ messageHandler: BuiltInBuildSystemMessageHandler) async
-
   /// Whether the build system is capable of preparing a target for indexing, ie. if the `prepare` methods has been
   /// implemented.
   var supportsPreparation: Bool { get }

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -19,7 +19,6 @@ import SwiftExtensions
 import ToolchainRegistry
 
 import struct TSCBasic.AbsolutePath
-import struct TSCBasic.RelativePath
 
 #if canImport(os)
 import os
@@ -58,35 +57,6 @@ fileprivate class RequestCache<Request: RequestType & Hashable> {
   }
 }
 
-/// Create a build system of the given type.
-private func createBuildSystem(
-  ofType buildSystemType: WorkspaceType,
-  projectRoot: AbsolutePath,
-  options: SourceKitLSPOptions,
-  swiftpmTestHooks: SwiftPMTestHooks,
-  toolchainRegistry: ToolchainRegistry,
-  reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
-) async -> BuiltInBuildSystem? {
-  switch buildSystemType {
-  case .buildServer:
-    return await BuildServerBuildSystem(projectRoot: projectRoot)
-  case .compilationDatabase:
-    return CompilationDatabaseBuildSystem(
-      projectRoot: projectRoot,
-      searchPaths: (options.compilationDatabaseOrDefault.searchPaths ?? [])
-        .compactMap { try? RelativePath(validating: $0) }
-    )
-  case .swiftPM:
-    return await SwiftPMBuildSystem(
-      projectRoot: projectRoot,
-      toolchainRegistry: toolchainRegistry,
-      options: options,
-      reloadPackageStatusCallback: reloadPackageStatusCallback,
-      testHooks: swiftpmTestHooks
-    )
-  }
-}
-
 /// `BuildSystem` that integrates client-side information such as main-file lookup as well as providing
 ///  common functionality such as caching.
 ///
@@ -136,7 +106,9 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   }
 
   package var supportsPreparation: Bool {
-    return buildSystem?.underlyingBuildSystem.supportsPreparation ?? false
+    get async {
+      return await buildSystem?.underlyingBuildSystem.supportsPreparation ?? false
+    }
   }
 
   package init(
@@ -146,29 +118,16 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     swiftpmTestHooks: SwiftPMTestHooks,
     reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
   ) async {
-    let buildSystem: BuiltInBuildSystem?
-    if let (buildSystemType, projectRoot) = buildSystemKind {
-      buildSystem = await createBuildSystem(
-        ofType: buildSystemType,
-        projectRoot: projectRoot,
-        options: options,
-        swiftpmTestHooks: swiftpmTestHooks,
-        toolchainRegistry: toolchainRegistry,
-        reloadPackageStatusCallback: reloadPackageStatusCallback
-      )
-    } else {
-      buildSystem = nil
-    }
-    let buildSystemHasDelegate = await buildSystem?.delegate != nil
-    precondition(!buildSystemHasDelegate)
     self.fallbackBuildSystem = FallbackBuildSystem(options: options.fallbackBuildSystemOrDefault)
     self.toolchainRegistry = toolchainRegistry
-    self.buildSystem =
-      if let buildSystem {
-        await BuiltInBuildSystemAdapter(buildSystem: buildSystem, messageHandler: self)
-      } else {
-        nil
-      }
+    self.buildSystem = await BuiltInBuildSystemAdapter(
+      buildSystemKind: buildSystemKind,
+      toolchainRegistry: toolchainRegistry,
+      options: options,
+      swiftpmTestHooks: swiftpmTestHooks,
+      reloadPackageStatusCallback: reloadPackageStatusCallback,
+      messageHandler: self
+    )
     await self.buildSystem?.underlyingBuildSystem.setDelegate(self)
   }
 
@@ -189,7 +148,7 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     self.toolchainRegistry = toolchainRegistry
     self.buildSystem =
       if let testBuildSystem {
-        await BuiltInBuildSystemAdapter(buildSystem: testBuildSystem, messageHandler: self)
+        await BuiltInBuildSystemAdapter(testBuildSystem: testBuildSystem, messageHandler: self)
       } else {
         nil
       }

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

@@ -13,7 +13,12 @@
 import BuildServerProtocol
 import LanguageServerProtocol
 import SKLogging
+import SKOptions
 import SKSupport
+import ToolchainRegistry
+
+import struct TSCBasic.AbsolutePath
+import struct TSCBasic.RelativePath
 
 // FIXME: (BSP Migration) This should be a MessageHandler once we have migrated all build system queries to BSP and can use
 // LocalConnection for the communication.
@@ -29,22 +34,84 @@ package protocol BuiltInBuildSystemMessageHandler: AnyObject, Sendable {
   func sendRequestToSourceKitLSP<R: RequestType>(_ request: R) async throws -> R.Response
 }
 
+/// Create a build system of the given type.
+private func createBuildSystem(
+  ofType buildSystemType: WorkspaceType,
+  projectRoot: AbsolutePath,
+  options: SourceKitLSPOptions,
+  swiftpmTestHooks: SwiftPMTestHooks,
+  toolchainRegistry: ToolchainRegistry,
+  messageHandler: BuiltInBuildSystemMessageHandler,
+  reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
+) async -> BuiltInBuildSystem? {
+  switch buildSystemType {
+  case .buildServer:
+    return await BuildServerBuildSystem(projectRoot: projectRoot, messageHandler: messageHandler)
+  case .compilationDatabase:
+    return CompilationDatabaseBuildSystem(
+      projectRoot: projectRoot,
+      searchPaths: (options.compilationDatabaseOrDefault.searchPaths ?? []).compactMap {
+        try? RelativePath(validating: $0)
+      },
+      messageHandler: messageHandler
+    )
+  case .swiftPM:
+    return await SwiftPMBuildSystem(
+      projectRoot: projectRoot,
+      toolchainRegistry: toolchainRegistry,
+      options: options,
+      messageHandler: messageHandler,
+      reloadPackageStatusCallback: reloadPackageStatusCallback,
+      testHooks: swiftpmTestHooks
+    )
+  }
+}
+
 /// A type that outwardly acts as a build server conforming to the Build System Integration Protocol and internally uses
 /// a `BuiltInBuildSystem` to satisfy the requests.
 package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler {
   /// The underlying build system
   // FIXME: (BSP Migration) This should be private, all messages should go through BSP. Only accessible from the outside for transition
   // purposes.
-  package let underlyingBuildSystem: BuiltInBuildSystem
+  private(set) package var underlyingBuildSystem: BuiltInBuildSystem!
   private let messageHandler: any BuiltInBuildSystemAdapterDelegate
 
-  init(
-    buildSystem: BuiltInBuildSystem,
+  init?(
+    buildSystemKind: (WorkspaceType, projectRoot: AbsolutePath)?,
+    toolchainRegistry: ToolchainRegistry,
+    options: SourceKitLSPOptions,
+    swiftpmTestHooks: SwiftPMTestHooks,
+    reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void,
     messageHandler: any BuiltInBuildSystemAdapterDelegate
   ) async {
+    guard let (buildSystemType, projectRoot) = buildSystemKind else {
+      return nil
+    }
+    self.messageHandler = messageHandler
+
+    let buildSystem = await createBuildSystem(
+      ofType: buildSystemType,
+      projectRoot: projectRoot,
+      options: options,
+      swiftpmTestHooks: swiftpmTestHooks,
+      toolchainRegistry: toolchainRegistry,
+      messageHandler: self,
+      reloadPackageStatusCallback: reloadPackageStatusCallback
+    )
+    guard let buildSystem else {
+      return nil
+    }
+
     self.underlyingBuildSystem = buildSystem
+  }
+
+  /// - Important: For testing purposes only
+  init(
+    testBuildSystem: BuiltInBuildSystem,
+    messageHandler: any BuiltInBuildSystemAdapterDelegate
+  ) async {
+    self.underlyingBuildSystem = testBuildSystem
     self.messageHandler = messageHandler
-    await buildSystem.setMessageHandler(self)
   }
 
   package func send<R: RequestType>(_ request: R) async throws -> R.Response {

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -50,10 +50,6 @@ package actor CompilationDatabaseBuildSystem {
 
   package weak var messageHandler: BuiltInBuildSystemMessageHandler?
 
-  package func setMessageHandler(_ messageHandler: any BuiltInBuildSystemMessageHandler) {
-    self.messageHandler = messageHandler
-  }
-
   package let projectRoot: AbsolutePath
 
   let searchPaths: [RelativePath]
@@ -87,11 +83,13 @@ package actor CompilationDatabaseBuildSystem {
   package init?(
     projectRoot: AbsolutePath,
     searchPaths: [RelativePath],
+    messageHandler: (any BuiltInBuildSystemMessageHandler)?,
     fileSystem: FileSystem = localFileSystem
   ) {
     self.fileSystem = fileSystem
     self.projectRoot = projectRoot
     self.searchPaths = searchPaths
+    self.messageHandler = messageHandler
     if let compdb = tryLoadCompilationDatabase(directory: projectRoot, additionalSearchPaths: searchPaths, fileSystem) {
       self.compdb = compdb
     } else {

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -182,10 +182,6 @@ package actor SwiftPMBuildSystem {
 
   package weak var messageHandler: BuiltInBuildSystemMessageHandler?
 
-  package func setMessageHandler(_ messageHandler: any BuiltInBuildSystemMessageHandler) {
-    self.messageHandler = messageHandler
-  }
-
   /// This callback is informed when `reloadPackage` starts and ends executing.
   private var reloadPackageStatusCallback: (ReloadPackageStatus) async -> Void
 
@@ -277,6 +273,7 @@ package actor SwiftPMBuildSystem {
     toolchainRegistry: ToolchainRegistry,
     fileSystem: FileSystem = localFileSystem,
     options: SourceKitLSPOptions,
+    messageHandler: (any BuiltInBuildSystemMessageHandler)?,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in },
     testHooks: SwiftPMTestHooks
   ) async throws {
@@ -292,6 +289,7 @@ package actor SwiftPMBuildSystem {
 
     self.toolchain = toolchain
     self.testHooks = testHooks
+    self.messageHandler = messageHandler
 
     guard let destinationToolchainBinDir = toolchain.swiftc?.parentDirectory else {
       throw Error.cannotDetermineHostToolchain
@@ -408,6 +406,7 @@ package actor SwiftPMBuildSystem {
     projectRoot: TSCBasic.AbsolutePath,
     toolchainRegistry: ToolchainRegistry,
     options: SourceKitLSPOptions,
+    messageHandler: any BuiltInBuildSystemMessageHandler,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void,
     testHooks: SwiftPMTestHooks
   ) async {
@@ -417,6 +416,7 @@ package actor SwiftPMBuildSystem {
         toolchainRegistry: toolchainRegistry,
         fileSystem: localFileSystem,
         options: options,
+        messageHandler: messageHandler,
         reloadPackageStatusCallback: reloadPackageStatusCallback,
         testHooks: testHooks
       )

Tests/BuildSystemIntegrationTests/BuildServerBuildSystemTests.swift

@@ -53,7 +53,7 @@ final class BuildServerBuildSystemTests: XCTestCase {
   let buildFolder = try! AbsolutePath(validating: NSTemporaryDirectory())
 
   func testServerInitialize() async throws {
-    let buildSystem = try await BuildServerBuildSystem(projectRoot: root)
+    let buildSystem = try await BuildServerBuildSystem(projectRoot: root, messageHandler: nil)
 
     assertEqual(
       await buildSystem.indexDatabasePath,
@@ -66,7 +66,6 @@ final class BuildServerBuildSystemTests: XCTestCase {
   }
 
   func testFileRegistration() async throws {
-    let buildSystem = try await BuildServerBuildSystem(projectRoot: root)
 
     let fileUrl = URL(fileURLWithPath: "/some/file/path")
     let expectation = XCTestExpectation(description: "\(fileUrl) settings updated")
@@ -75,14 +74,14 @@ final class BuildServerBuildSystemTests: XCTestCase {
       // BuildSystemManager has a weak reference to delegate. Keep it alive.
       _fixLifetime(buildSystemDelegate)
     }
+    let buildSystem = try await BuildServerBuildSystem(projectRoot: root, messageHandler: buildSystemDelegate)
     await buildSystem.setDelegate(buildSystemDelegate)
     await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl))
 
     XCTAssertEqual(XCTWaiter.wait(for: [expectation], timeout: defaultTimeout), .completed)
   }
 
   func testBuildTargetsChanged() async throws {
-    let buildSystem = try await BuildServerBuildSystem(projectRoot: root)
 
     let fileUrl = URL(fileURLWithPath: "/some/file/path")
     let expectation = XCTestExpectation(description: "target changed")
@@ -102,7 +101,7 @@ final class BuildServerBuildSystemTests: XCTestCase {
       // BuildSystemManager has a weak reference to delegate. Keep it alive.
       _fixLifetime(buildSystemDelegate)
     }
-    await buildSystem.setMessageHandler(buildSystemDelegate)
+    let buildSystem = try await BuildServerBuildSystem(projectRoot: root, messageHandler: buildSystemDelegate)
     await buildSystem.registerForChangeNotifications(for: DocumentURI(fileUrl))
 
     try await fulfillmentOfOrThrow([expectation])

Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift

@@ -464,12 +464,6 @@ class ManualBuildSystem: BuiltInBuildSystem {
     self.delegate = delegate
   }
 
-  weak var messageHandler: BuiltInBuildSystemMessageHandler?
-
-  func setMessageHandler(_ messageHandler: any BuiltInBuildSystemMessageHandler) {
-    self.messageHandler = messageHandler
-  }
-
   package nonisolated var supportsPreparation: Bool { false }
 
   func buildSettings(

Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift

@@ -421,6 +421,7 @@ private func checkCompilationDatabaseBuildSystem(
   let buildSystem = CompilationDatabaseBuildSystem(
     projectRoot: try AbsolutePath(validating: "/a"),
     searchPaths: try [RelativePath(validating: ".")],
+    messageHandler: nil,
     fileSystem: fs
   )
   try await block(XCTUnwrap(buildSystem))