Introduce a BuiltInBuildSystemAdapter that can be used to transition BuildSystem to a type that implements BSP

Author: ahoppen

Package.swift

@@ -254,6 +254,7 @@ let package = Package(
       dependencies: [
         "CAtomics",
         "LanguageServerProtocol",
+        "LanguageServerProtocolJSONRPC",
         "SKLogging",
         "SwiftExtensions",
         .product(name: "SwiftToolsSupport-auto", package: "swift-tools-support-core"),

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -74,6 +74,12 @@ package actor BuildServerBuildSystem: MessageHandler {
     self.delegate = delegate
   }
 
+  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] = [:]
 

Sources/BuildSystemIntegration/BuildSystem.swift

@@ -119,6 +119,11 @@ 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

@@ -34,14 +34,16 @@ import os
 /// Since some `BuildSystem`s may require a bit of a time to compute their arguments asynchronously,
 /// this class has a configurable `buildSettings` timeout which denotes the amount of time to give
 /// the build system before applying the fallback arguments.
-package actor BuildSystemManager {
+package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   /// The files for which the delegate has requested change notifications, ie.
   /// the files for which the delegate wants to get `filesDependenciesUpdated`
   /// callbacks if the file's build settings.
   var watchedFiles: [DocumentURI: (mainFile: DocumentURI, language: Language)] = [:]
 
   /// The underlying primary build system.
-  let buildSystem: BuiltInBuildSystem?
+  ///
+  /// - Important: The only time this should be modified is in the initializer. Afterwards, it must be constant.
+  private(set) var buildSystem: BuiltInBuildSystemAdapter?
 
   /// Timeout before fallback build settings are used.
   let fallbackSettingsTimeout: DispatchTimeInterval
@@ -66,12 +68,12 @@ package actor BuildSystemManager {
   /// was found.
   package var projectRoot: AbsolutePath? {
     get async {
-      return await buildSystem?.projectRoot
+      return await buildSystem?.underlyingBuildSystem.projectRoot
     }
   }
 
   package var supportsPreparation: Bool {
-    return buildSystem?.supportsPreparation ?? false
+    return buildSystem?.underlyingBuildSystem.supportsPreparation ?? false
   }
 
   /// Create a BuildSystemManager that wraps the given build system. The new
@@ -85,28 +87,45 @@ package actor BuildSystemManager {
   ) 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)
+    self.buildSystem =
+      if let buildSystem {
+        await BuiltInBuildSystemAdapter(buildSystem: buildSystem, messageHandler: self)
+      } else {
+        nil
+      }
+    await self.buildSystem?.underlyingBuildSystem.setDelegate(self)
   }
 
   package func filesDidChange(_ events: [FileEvent]) async {
-    await self.buildSystem?.filesDidChange(events)
+    await self.buildSystem?.underlyingBuildSystem.filesDidChange(events)
+  }
+
+  /// Implementation of `MessageHandler`, handling notifications from the build system.
+  ///
+  /// - Important: Do not call directly.
+  package func handle(_ notification: some LanguageServerProtocol.NotificationType) {
+    logger.error("Ignoring unknown notification \(type(of: notification).method) from build system")
+  }
+
+  /// Implementation of `MessageHandler`, handling requests from the build system.
+  ///
+  /// - Important: Do not call directly.
+  package nonisolated func handle<R: RequestType>(_ request: R) async throws -> R.Response {
+    throw ResponseError.methodNotFound(R.method)
   }
-}
 
-extension BuildSystemManager {
   /// - Note: Needed so we can set the delegate from a different isolation context.
   package func setDelegate(_ delegate: BuildSystemDelegate?) {
     self.delegate = delegate
   }
 
   /// Returns the toolchain that should be used to process the given document.
   package func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
-    if let toolchain = await buildSystem?.toolchain(for: uri, language) {
+    if let toolchain = await buildSystem?.underlyingBuildSystem.toolchain(for: uri, language) {
       return toolchain
     }
 
@@ -128,7 +147,7 @@ extension BuildSystemManager {
   /// Returns the language that a document should be interpreted in for background tasks where the editor doesn't
   /// specify the document's language.
   package func defaultLanguage(for document: DocumentURI) async -> Language? {
-    if let defaultLanguage = await buildSystem?.defaultLanguage(for: document) {
+    if let defaultLanguage = await buildSystem?.underlyingBuildSystem.defaultLanguage(for: document) {
       return defaultLanguage
     }
     switch document.fileURL?.pathExtension {
@@ -143,7 +162,7 @@ extension BuildSystemManager {
 
   /// Returns all the `ConfiguredTarget`s that the document is part of.
   package func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] {
-    return await buildSystem?.configuredTargets(for: document) ?? []
+    return await buildSystem?.underlyingBuildSystem.configuredTargets(for: document) ?? []
   }
 
   /// Returns the `ConfiguredTarget` that should be used for semantic functionality of the given document.
@@ -203,7 +222,7 @@ extension BuildSystemManager {
     // For now, this should be fine because all build systems return
     // very quickly from `settings(for:language:)`.
     // https://github.com/apple/sourcekit-lsp/issues/1181
-    return try await buildSystem.buildSettings(for: document, in: target, language: language)
+    return try await buildSystem.underlyingBuildSystem.buildSettings(for: document, in: target, language: language)
   }
 
   /// Returns the build settings for the given file in the given target.
@@ -266,26 +285,26 @@ extension BuildSystemManager {
   }
 
   package func generateBuildGraph() async throws {
-    try await self.buildSystem?.generateBuildGraph()
+    try await self.buildSystem?.underlyingBuildSystem.generateBuildGraph()
   }
 
   package func waitForUpToDateBuildGraph() async {
-    await self.buildSystem?.waitForUpToDateBuildGraph()
+    await self.buildSystem?.underlyingBuildSystem.waitForUpToDateBuildGraph()
   }
 
   package func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {
-    return await buildSystem?.topologicalSort(of: targets)
+    return await buildSystem?.underlyingBuildSystem.topologicalSort(of: targets)
   }
 
   package func targets(dependingOn targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
-    return await buildSystem?.targets(dependingOn: targets)
+    return await buildSystem?.underlyingBuildSystem.targets(dependingOn: targets)
   }
 
   package func prepare(
     targets: [ConfiguredTarget],
     logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void
   ) async throws {
-    try await buildSystem?.prepare(targets: targets, logMessageToIndexLog: logMessageToIndexLog)
+    try await buildSystem?.underlyingBuildSystem.prepare(targets: targets, logMessageToIndexLog: logMessageToIndexLog)
   }
 
   package func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
@@ -297,7 +316,7 @@ extension BuildSystemManager {
     // system. That way, iff the main file changes, we will also notify the
     // delegate about build setting changes of all header files that are based
     // on that main file.
-    await buildSystem?.registerForChangeNotifications(for: mainFile)
+    await buildSystem?.underlyingBuildSystem.registerForChangeNotifications(for: mainFile)
   }
 
   package func unregisterForChangeNotifications(for uri: DocumentURI) async {
@@ -310,19 +329,19 @@ extension BuildSystemManager {
     if watchedFilesReferencing(mainFiles: [mainFile]).isEmpty {
       // Nobody is interested in this main file anymore.
       // We are no longer interested in change notifications for it.
-      await self.buildSystem?.unregisterForChangeNotifications(for: mainFile)
+      await self.buildSystem?.underlyingBuildSystem.unregisterForChangeNotifications(for: mainFile)
     }
   }
 
   package func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability {
     return max(
-      await buildSystem?.fileHandlingCapability(for: uri) ?? .unhandled,
+      await buildSystem?.underlyingBuildSystem.fileHandlingCapability(for: uri) ?? .unhandled,
       fallbackBuildSystem != nil ? .fallback : .unhandled
     )
   }
 
   package func sourceFiles() async -> [SourceFileInfo] {
-    return await buildSystem?.sourceFiles() ?? []
+    return await buildSystem?.underlyingBuildSystem.sourceFiles() ?? []
   }
 
   package func testFiles() async -> [DocumentURI] {

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

@@ -0,0 +1,107 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import LanguageServerProtocol
+import SKLogging
+import SKSupport
+
+// 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.
+protocol BuiltInBuildSystemAdapterDelegate: Sendable {
+  func handle(_ notification: some NotificationType) async
+  func handle<R: RequestType>(_ request: R) async throws -> R.Response
+}
+
+// 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.
+package protocol BuiltInBuildSystemMessageHandler: AnyObject, Sendable {
+  func sendNotificationToSourceKitLSP(_ notification: some NotificationType) async
+  func sendRequestToSourceKitLSP<R: RequestType>(_ request: R) async throws -> R.Response
+}
+
+/// 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.
+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 let messageHandler: any BuiltInBuildSystemAdapterDelegate
+
+  init(
+    buildSystem: BuiltInBuildSystem,
+    messageHandler: any BuiltInBuildSystemAdapterDelegate
+  ) async {
+    self.underlyingBuildSystem = buildSystem
+    self.messageHandler = messageHandler
+    await buildSystem.setMessageHandler(self)
+  }
+
+  package func send<R: RequestType>(_ request: R) async throws -> R.Response {
+    logger.info(
+      """
+      Received request to build system
+      \(request.forLogging)
+      """
+    )
+    /// Executes `body` and casts the result type to `R.Response`, statically checking that the return type of `body` is
+    /// the response type of `request`.
+    func handle<HandledRequestType: RequestType>(
+      _ request: HandledRequestType,
+      _ body: (HandledRequestType) async throws -> HandledRequestType.Response
+    ) async throws -> R.Response {
+      return try await body(request) as! R.Response
+    }
+
+    switch request {
+    default:
+      throw ResponseError.methodNotFound(R.method)
+    }
+  }
+
+  package func send(_ notification: some NotificationType) async {
+    logger.info(
+      """
+      Sending notification to build system
+      \(notification.forLogging)
+      """
+    )
+    // FIXME: (BSP Migration) These messages should be handled using a LocalConnection, which also gives us logging for the messages
+    // sent. We can only do this once all requests to the build system have been migrated and we can implement proper
+    // dependency management between the BSP messages
+    switch notification {
+    default:
+      logger.error("Ignoring unknown notification \(type(of: notification).method) from SourceKit-LSP")
+    }
+  }
+
+  func sendNotificationToSourceKitLSP(_ notification: some LanguageServerProtocol.NotificationType) async {
+    logger.info(
+      """
+      Received notification from build system
+      \(notification.forLogging)
+      """
+    )
+    await messageHandler.handle(notification)
+  }
+
+  func sendRequestToSourceKitLSP<R: RequestType>(_ request: R) async throws -> R.Response {
+    logger.info(
+      """
+      Received request from build system
+      \(request.forLogging)
+      """
+    )
+    return try await messageHandler.handle(request)
+  }
+
+}

Sources/BuildSystemIntegration/CMakeLists.txt

@@ -4,6 +4,7 @@ add_library(BuildSystemIntegration STATIC
   BuildSystem.swift
   BuildSystemDelegate.swift
   BuildSystemManager.swift
+  BuiltInBuildSystemAdapter.swift
   CompilationDatabase.swift
   CompilationDatabaseBuildSystem.swift
   FallbackBuildSystem.swift

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -47,6 +47,12 @@ package actor CompilationDatabaseBuildSystem {
     self.delegate = delegate
   }
 
+  package weak var messageHandler: BuiltInBuildSystemMessageHandler?
+
+  package func setMessageHandler(_ messageHandler: any BuiltInBuildSystemMessageHandler) {
+    self.messageHandler = messageHandler
+  }
+
   package let projectRoot: AbsolutePath
 
   let searchPaths: [RelativePath]

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -130,6 +130,12 @@ package actor SwiftPMBuildSystem {
     self.delegate = delegate
   }
 
+  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
 

Sources/InProcessClient/CMakeLists.txt

@@ -1,6 +1,5 @@
 add_library(InProcessClient STATIC
-  InProcessSourceKitLSPClient.swift
-  LocalConnection.swift)
+  InProcessSourceKitLSPClient.swift)
 
 set_target_properties(InProcessClient PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})

Sources/LanguageServerProtocol/Connection.swift

@@ -45,3 +45,35 @@ public protocol MessageHandler: AnyObject, Sendable {
     reply: @Sendable @escaping (LSPResult<Request.Response>) -> Void
   )
 }
+
+// MARK: - WeakMessageHelper
+
+/// Wrapper around a weak `MessageHandler`.
+///
+/// This allows us to eg. set the ``TestSourceKitLSPClient`` as the message handler of
+/// `SourceKitLSPServer` without retaining it.
+public final class WeakMessageHandler: MessageHandler, Sendable {
+  // `nonisolated(unsafe)` is fine because `handler` is never modified, only if the weak reference is deallocated, which
+  // is atomic.
+  private nonisolated(unsafe) weak var handler: (any MessageHandler)?
+
+  public init(_ handler: any MessageHandler) {
+    self.handler = handler
+  }
+
+  public func handle(_ params: some LanguageServerProtocol.NotificationType) {
+    handler?.handle(params)
+  }
+
+  public func handle<Request: RequestType>(
+    _ params: Request,
+    id: LanguageServerProtocol.RequestID,
+    reply: @Sendable @escaping (LanguageServerProtocol.LSPResult<Request.Response>) -> Void
+  ) {
+    guard let handler = handler else {
+      reply(.failure(.unknown("Handler has been deallocated")))
+      return
+    }
+    handler.handle(params, id: id, reply: reply)
+  }
+}