Merge pull request #836 from ahoppen/ahoppen/build-system-actor

Migrate `BuildSystemManager` and all the build systems to be actors

Author: ahoppen

Sources/LSPTestSupport/Assertions.swift

@@ -22,6 +22,27 @@ public func assertNoThrow<T>(
   XCTAssertNoThrow(try expression(), message(), file: file, line: line)
 }
 
+/// Same as `XCTAssertThrows` but executes the trailing closure.
+public func assertThrowsError<T>(
+  _ expression: @autoclosure () async throws -> T,
+  _ message: @autoclosure () -> String = "",
+  file: StaticString = #filePath,
+  line: UInt = #line,
+  _ errorHandler: (_ error: Error) -> Void = { _ in }
+) async {
+  let didThrow: Bool
+  do {
+    _ = try await expression()
+    didThrow = false
+  } catch {
+    errorHandler(error)
+    didThrow = true
+  }
+  if !didThrow {
+    XCTFail("Expression was expected to throw but did not throw", file: file, line: line)
+  }
+}
+
 /// Same as `XCTAssertEqual` but doesn't take autoclosures and thus `expression1`
 /// and `expression2` can contain `await`.
 public func assertEqual<T: Equatable>(
@@ -34,6 +55,28 @@ public func assertEqual<T: Equatable>(
   XCTAssertEqual(expression1, expression2, message(), file: file, line: line)
 }
 
+/// Same as `XCTAssertNil` but doesn't take autoclosures and thus `expression`
+/// can contain `await`.
+public func assertNil<T: Equatable>(
+  _ expression: T?,
+  _ message: @autoclosure () -> String = "",
+  file: StaticString = #filePath,
+  line: UInt = #line
+) {
+  XCTAssertNil(expression, message(), file: file, line: line)
+}
+
+/// Same as `XCTAssertNotNil` but doesn't take autoclosures and thus `expression`
+/// can contain `await`.
+public func assertNotNil<T: Equatable>(
+  _ expression: T?,
+  _ message: @autoclosure () -> String = "",
+  file: StaticString = #filePath,
+  line: UInt = #line
+) {
+  XCTAssertNotNil(expression, message(), file: file, line: line)
+}
+
 extension XCTestCase {
   private struct ExpectationNotFulfilledError: Error, CustomStringConvertible {
     var expecatations: [XCTestExpectation]

Sources/SKCore/BuildServerBuildSystem.swift

@@ -43,10 +43,7 @@ func executable(_ name: String) -> String {
 ///
 /// Provides build settings from a build server launched based on a
 /// `buildServer.json` configuration file provided in the repo root.
-public final class BuildServerBuildSystem: MessageHandler {
-  /// The handler's request queue. `delegate` will always be called on this queue.
-  public let queue: DispatchQueue = DispatchQueue(label: "language-server-queue", qos: .userInitiated)
-
+public actor BuildServerBuildSystem: MessageHandler {
   let projectRoot: AbsolutePath
   let buildFolder: AbsolutePath?
   let serverConfig: BuildServerConfig
@@ -65,10 +62,15 @@ public final class BuildServerBuildSystem: MessageHandler {
   /// Delegate to handle any build system events.
   public weak var delegate: BuildSystemDelegate?
 
+  /// - Note: Needed to set the delegate from a different actor isolation context
+  public func setDelegate(_ delegate: BuildSystemDelegate?) async {
+    self.delegate = delegate
+  }
+
   /// The build settings that have been received from the build server.
   private var buildSettings: [DocumentURI: FileBuildSettings] = [:]
 
-  public init(projectRoot: AbsolutePath, buildFolder: AbsolutePath?, fileSystem: FileSystem = localFileSystem) throws {
+  public init(projectRoot: AbsolutePath, buildFolder: AbsolutePath?, fileSystem: FileSystem = localFileSystem) async throws {
     let configPath = projectRoot.appending(component: "buildServer.json")
     let config = try loadBuildServerConfig(path: configPath, fileSystem: fileSystem)
 #if os(Windows)
@@ -90,12 +92,11 @@ public final class BuildServerBuildSystem: MessageHandler {
   /// Creates a build system using the Build Server Protocol config.
   ///
   /// - Returns: nil if `projectRoot` has no config or there is an error parsing it.
-  public convenience init?(projectRoot: AbsolutePath?, buildSetup: BuildSetup)
-  {
+  public init?(projectRoot: AbsolutePath?, buildSetup: BuildSetup) async {
     if projectRoot == nil { return nil }
 
     do {
-      try self.init(projectRoot: projectRoot!, buildFolder: buildSetup.path)
+      try await self.init(projectRoot: projectRoot!, buildFolder: buildSetup.path)
     } catch _ as FileSystemError {
       // config file was missing, no build server for this workspace
       return nil
@@ -166,13 +167,11 @@ public final class BuildServerBuildSystem: MessageHandler {
   /// the build server has sent us a notification.
   ///
   /// We need to notify the delegate about any updated build settings.
-  public func handle(_ params: some NotificationType, from clientID: ObjectIdentifier) {
-    queue.async {
-      if let params = params as? BuildTargetsChangedNotification {
-        self.handleBuildTargetsChanged(Notification(params, clientID: clientID))
-      } else if let params = params as? FileOptionsChangedNotification {
-        self.handleFileOptionsChanged(Notification(params, clientID: clientID))
-      }
+  public func handle(_ params: some NotificationType, from clientID: ObjectIdentifier) async {
+    if let params = params as? BuildTargetsChangedNotification {
+      await self.handleBuildTargetsChanged(Notification(params, clientID: clientID))
+    } else if let params = params as? FileOptionsChangedNotification {
+      await self.handleFileOptionsChanged(Notification(params, clientID: clientID))
     }
   }
 
@@ -185,30 +184,28 @@ public final class BuildServerBuildSystem: MessageHandler {
     from clientID: ObjectIdentifier,
     reply: @escaping (LSPResult<R.Response>) -> Void
   ) {
-    queue.async {
-      reply(.failure(ResponseError.methodNotFound(R.method)))
-    }
+    reply(.failure(ResponseError.methodNotFound(R.method)))
   }
 
-  func handleBuildTargetsChanged(_ notification: LanguageServerProtocol.Notification<BuildTargetsChangedNotification>) {
-    self.delegate?.buildTargetsChanged(notification.params.changes)
+  func handleBuildTargetsChanged(_ notification: LanguageServerProtocol.Notification<BuildTargetsChangedNotification>) async {
+    await self.delegate?.buildTargetsChanged(notification.params.changes)
   }
 
-  func handleFileOptionsChanged(_ notification: LanguageServerProtocol.Notification<FileOptionsChangedNotification>) {
+  func handleFileOptionsChanged(_ notification: LanguageServerProtocol.Notification<FileOptionsChangedNotification>) async {
     let result = notification.params.updatedOptions
     let settings = FileBuildSettings(
         compilerArguments: result.options, workingDirectory: result.workingDirectory)
-    self.buildSettingsChanged(for: notification.params.uri, settings: settings)
+    await self.buildSettingsChanged(for: notification.params.uri, settings: settings)
   }
 
   /// Record the new build settings for the given document and inform the delegate
   /// about the changed build settings.
-  private func buildSettingsChanged(for document: DocumentURI, settings: FileBuildSettings?) {
+  private func buildSettingsChanged(for document: DocumentURI, settings: FileBuildSettings?) async {
     buildSettings[document] = settings
     if let settings {
-      self.delegate?.fileBuildSettingsChanged([document: .modified(settings)])
+      await self.delegate?.fileBuildSettingsChanged([document: .modified(settings)])
     } else {
-      self.delegate?.fileBuildSettingsChanged([document: .removedOrUnavailable])
+      await self.delegate?.fileBuildSettingsChanged([document: .removedOrUnavailable])
     }
   }
 }
@@ -234,12 +231,14 @@ extension BuildServerBuildSystem: BuildSystem {
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     let request = RegisterForChanges(uri: uri, action: .register)
     _ = self.buildServer?.send(request, queue: requestQueue, reply: { result in
-      if let error = result.failure {
-        log("error registering \(uri): \(error)", level: .error)
-
-        // BuildServer registration failed, so tell our delegate that no build
-        // settings are available.
-        self.buildSettingsChanged(for: uri, settings: nil)
+      Task {
+        if let error = result.failure {
+          log("error registering \(uri): \(error)", level: .error)
+          
+          // BuildServer registration failed, so tell our delegate that no build
+          // settings are available.
+          await self.buildSettingsChanged(for: uri, settings: nil)
+        }
       }
     })
   }

Sources/SKCore/BuildSystem.swift

@@ -39,17 +39,23 @@ public enum FileHandlingCapability: Comparable {
 public protocol BuildSystem: AnyObject {
 
   /// The path to the raw index store data, if any.
-  var indexStorePath: AbsolutePath? { get }
+  var indexStorePath: AbsolutePath? { get async }
 
   /// The path to put the index database, if any.
-  var indexDatabasePath: AbsolutePath? { get }
+  var indexDatabasePath: AbsolutePath? { get async }
 
   /// Path remappings for remapping index data for local use.
-  var indexPrefixMappings: [PathPrefixMapping] { get }
+  var indexPrefixMappings: [PathPrefixMapping] { get async }
 
   /// Delegate to handle any build system events such as file build settings
   /// initial reports as well as changes.
-  var delegate: BuildSystemDelegate? { get set }
+  var delegate: BuildSystemDelegate? { get async }
+
+  /// Set the build system's delegate.
+  ///
+  /// - Note: Needed so we can set the delegate from a different actor isolation
+  ///   context.
+  func setDelegate(_ delegate: BuildSystemDelegate?) async
 
   /// Retrieve build settings for the given document with the given source
   /// language.
@@ -64,16 +70,16 @@ public protocol BuildSystem: AnyObject {
   /// IMPORTANT: When first receiving a register request, the `BuildSystem` MUST asynchronously
   /// inform its delegate of any initial settings for the given file via the
   /// `fileBuildSettingsChanged` method, even if unavailable.
-  func registerForChangeNotifications(for: DocumentURI, language: Language)
+  func registerForChangeNotifications(for: DocumentURI, language: Language) async
 
   /// Unregister the given file for build-system level change notifications,
   /// such as command line flag changes, dependency changes, etc.
-  func unregisterForChangeNotifications(for: DocumentURI)
+  func unregisterForChangeNotifications(for: DocumentURI) async
 
   /// Called when files in the project change.
-  func filesDidChange(_ events: [FileEvent])
+  func filesDidChange(_ events: [FileEvent]) async
 
-  func fileHandlingCapability(for uri: DocumentURI) -> FileHandlingCapability
+  func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability
 }
 
 public let buildTargetsNotSupported =

Sources/SKCore/BuildSystemDelegate.swift

@@ -18,25 +18,25 @@ public protocol BuildSystemDelegate: AnyObject {
   ///
   /// The callee should request new sources and outputs for the build targets of
   /// interest.
-  func buildTargetsChanged(_ changes: [BuildTargetEvent])
+  func buildTargetsChanged(_ changes: [BuildTargetEvent]) async
 
   /// Notify the delegate that the given files' build settings have changed.
   ///
   /// The delegate should cache the new build settings for any of the given
   /// files that they are interested in.
   func fileBuildSettingsChanged(
-    _ changedFiles: [DocumentURI: FileBuildSettingsChange])
+    _ changedFiles: [DocumentURI: FileBuildSettingsChange]) async
 
   /// Notify the delegate that the dependencies of the given files have changed
   /// and that ASTs may need to be refreshed. If the given set is empty, assume
   /// that all watched files are affected.
   ///
   /// The callee should refresh ASTs unless it is able to determine that a
   /// refresh is not necessary.
-  func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>)
+  func filesDependenciesUpdated(_ changedFiles: Set<DocumentURI>) async
 
   /// Notify the delegate that the file handling capability of this build system
   /// for some file has changed. The delegate should discard any cached file
   /// handling capability.
-  func fileHandlingCapabilityChanged()
+  func fileHandlingCapabilityChanged() async
 }