Make all methods on BuildSystem async

This is the prerequisite for making the build systems actors.

Author: ahoppen

Sources/SKCore/BuildServerBuildSystem.swift

@@ -65,6 +65,11 @@ 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] = [:]
 

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/BuildSystemManager.swift

@@ -111,17 +111,18 @@ public actor BuildSystemManager {
   /// Create a BuildSystemManager that wraps the given build system. The new
   /// manager will modify the delegate of the underlying build system.
   public init(buildSystem: BuildSystem?, fallbackBuildSystem: FallbackBuildSystem?,
-              mainFilesProvider: MainFilesProvider?, fallbackSettingsTimeout: DispatchTimeInterval = .seconds(3)) {
-    precondition(buildSystem?.delegate == nil)
+              mainFilesProvider: MainFilesProvider?, fallbackSettingsTimeout: DispatchTimeInterval = .seconds(3)) async {
+    let buildSystemHasDelegate = await buildSystem?.delegate != nil
+    precondition(!buildSystemHasDelegate)
     self.buildSystem = buildSystem
     self.fallbackBuildSystem = fallbackBuildSystem
     self._mainFilesProvider = mainFilesProvider
     self.fallbackSettingsTimeout = fallbackSettingsTimeout
-    self.buildSystem?.delegate = self
+    await self.buildSystem?.setDelegate(self)
   }
 
-  public func filesDidChange(_ events: [FileEvent]) {
-    self.buildSystem?.filesDidChange(events)
+  public func filesDidChange(_ events: [FileEvent]) async {
+    await self.buildSystem?.filesDidChange(events)
     self.fallbackBuildSystem?.filesDidChange(events)
   }
 }
@@ -182,7 +183,7 @@ extension BuildSystemManager {
     }
   }
 
-  public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
+  public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
     log("registerForChangeNotifications(\(uri.pseudoPath))")
     let mainFile: DocumentURI
 
@@ -194,7 +195,7 @@ extension BuildSystemManager {
       self.watchedFiles[uri] = (mainFile, language)
     }
 
-    let newStatus = self.cachedStatusOrRegisterForSettings(for: mainFile, language: language)
+    let newStatus = await self.cachedStatusOrRegisterForSettings(for: mainFile, language: language)
 
     if let mainChange = newStatus.buildSettingsChange,
        let delegate = self._delegate {
@@ -206,7 +207,7 @@ extension BuildSystemManager {
   /// Return settings for `file` based on  the `change` settings for `mainFile`.
   ///
   /// This is used when inferring arguments for header files (e.g. main file is a `.m`  while file is  a` .h`).
-  func convert(
+  nonisolated func convert(
     change: FileBuildSettingsChange,
     ofMainFile mainFile: DocumentURI,
     to file: DocumentURI
@@ -227,7 +228,7 @@ extension BuildSystemManager {
   func cachedStatusOrRegisterForSettings(
     for mainFile: DocumentURI,
     language: Language
-  ) -> MainFileStatus {
+  ) async -> MainFileStatus {
     // If we already have a status for the main file, use that.
     // Don't update any existing timeout.
     if let status = self.mainFileStatuses[mainFile] {
@@ -248,7 +249,7 @@ extension BuildSystemManager {
       // Intentionally register with the `BuildSystem` after setting the fallback to allow for
       // testing of the fallback system triggering before the `BuildSystem` can reply (e.g. if a
       // fallback time of 0 is specified).
-      buildSystem.registerForChangeNotifications(for: mainFile, language: language)
+      await buildSystem.registerForChangeNotifications(for: mainFile, language: language)
 
 
       newStatus = .waiting
@@ -263,6 +264,16 @@ extension BuildSystemManager {
     } else {  // Don't have any build systems.
       newStatus = .unsupported
     }
+
+    if let status = self.mainFileStatuses[mainFile] {
+      // Since we await above, another call to `cachedStatusOrRegisterForSettings`
+      // might have set the main file status of `mainFile`. If this race happened,
+      // return the value set by the concurrently executing function. This is safe
+      // since all calls from this function are either side-effect free or
+      // idempotent.
+      return status
+    }
+
     self.mainFileStatuses[mainFile] = newStatus
     return newStatus
   }
@@ -321,27 +332,30 @@ extension BuildSystemManager {
     }
   }
 
-  public func unregisterForChangeNotifications(for uri: DocumentURI) {
+  public func unregisterForChangeNotifications(for uri: DocumentURI) async {
     guard let mainFile = self.watchedFiles[uri]?.mainFile else {
       log("Unbalanced calls for registerForChangeNotifications and unregisterForChangeNotifications", level: .warning)
       return
     }
     self.watchedFiles[uri] = nil
-    self.checkUnreferencedMainFile(mainFile)
+    await self.checkUnreferencedMainFile(mainFile)
   }
 
   /// If the given main file is no longer referenced by any watched files,
   /// remove it and unregister it at the underlying build system.
-  func checkUnreferencedMainFile(_ mainFile: DocumentURI) {
+  func checkUnreferencedMainFile(_ mainFile: DocumentURI) async {
     if !self.watchedFiles.values.lazy.map({ $0.mainFile }).contains(mainFile) {
       // This was the last reference to the main file. Remove it.
-      self.buildSystem?.unregisterForChangeNotifications(for: mainFile)
+      await self.buildSystem?.unregisterForChangeNotifications(for: mainFile)
       self.mainFileStatuses[mainFile] = nil
     }
   }
 
-  public nonisolated func fileHandlingCapability(for uri: DocumentURI) -> FileHandlingCapability {
-    return max(buildSystem?.fileHandlingCapability(for: uri) ?? .unhandled, fallbackBuildSystem?.fileHandlingCapability(for: uri) ?? .unhandled)
+  public func fileHandlingCapability(for uri: DocumentURI) async -> FileHandlingCapability {
+    return max(
+      await buildSystem?.fileHandlingCapability(for: uri) ?? .unhandled,
+      fallbackBuildSystem?.fileHandlingCapability(for: uri) ?? .unhandled
+    )
   }
 }
 
@@ -444,7 +458,7 @@ extension BuildSystemManager: MainFilesDelegate {
   }
 
   // FIXME: Consider debouncing/limiting this, seems to trigger often during a build.
-  public func mainFilesChangedImpl() {
+  public func mainFilesChangedImpl() async {
     let origWatched = self.watchedFiles
     self.watchedFiles = [:]
     var buildSettingsChanges = [DocumentURI: FileBuildSettingsChange]()
@@ -458,9 +472,9 @@ extension BuildSystemManager: MainFilesDelegate {
 
       if state.mainFile != newMainFile {
         log("main file for '\(uri)' changed old: '\(state.mainFile)' -> new: '\(newMainFile)'", level: .info)
-        self.checkUnreferencedMainFile(state.mainFile)
+        await self.checkUnreferencedMainFile(state.mainFile)
 
-        let newStatus = self.cachedStatusOrRegisterForSettings(
+        let newStatus = await self.cachedStatusOrRegisterForSettings(
             for: newMainFile, language: language)
         if let change = newStatus.buildSettingsChange {
           let newChange = self.convert(change: change, ofMainFile: newMainFile, to: uri)

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -46,6 +46,10 @@ public final class CompilationDatabaseBuildSystem {
   /// Delegate to handle any build system events.
   public weak var delegate: BuildSystemDelegate? = nil
 
+  public func setDelegate(_ delegate: BuildSystemDelegate?) async {
+    self.delegate = delegate
+  }
+
   let projectRoot: AbsolutePath?
 
   let fileSystem: FileSystem

Sources/SKCore/FallbackBuildSystem.swift

@@ -38,6 +38,10 @@ public final class FallbackBuildSystem: BuildSystem {
   /// Delegate to handle any build system events.
   public weak var delegate: BuildSystemDelegate? = nil
 
+  public func setDelegate(_ delegate: BuildSystemDelegate?) async {
+    self.delegate = delegate
+  }
+
   public var indexStorePath: AbsolutePath? { return nil }
 
   public var indexDatabasePath: AbsolutePath? { return nil }

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -62,6 +62,10 @@ public final class SwiftPMWorkspace {
   /// Delegate to handle any build system events.
   public weak var delegate: SKCore.BuildSystemDelegate? = nil
 
+  public func setDelegate(_ delegate: SKCore.BuildSystemDelegate?) async {
+    self.delegate = delegate
+  }
+
   let workspacePath: TSCAbsolutePath
   let packageRoot: TSCAbsolutePath
   /// *Public for testing*

Sources/SKTestSupport/SKSwiftPMTestWorkspace.swift

@@ -104,7 +104,7 @@ public final class SKSwiftPMTestWorkspace {
       listenToUnitEvents: false)
 
     let server = self.testServer.server!
-    let workspace = Workspace(
+    let workspace = await Workspace(
       documentManager: DocumentManager(),
       rootUri: DocumentURI(sources.rootDirectory),
       capabilityRegistry: CapabilityRegistry(clientCapabilities: ClientCapabilities()),

Sources/SKTestSupport/SKTibsTestWorkspace.swift

@@ -87,7 +87,7 @@ public final class SKTibsTestWorkspace {
     let indexDelegate = SourceKitIndexDelegate()
     tibsWorkspace.delegate = indexDelegate
 
-    let workspace = Workspace(
+    let workspace = await Workspace(
       documentManager: DocumentManager(),
       rootUri: DocumentURI(sources.rootDirectory),
       capabilityRegistry: CapabilityRegistry(clientCapabilities: clientCapabilities),

Sources/SourceKitLSP/CMakeLists.txt

@@ -5,6 +5,7 @@ add_library(SourceKitLSP STATIC
   DocumentTokens.swift
   IndexStoreDB+MainFilesProvider.swift
   RangeAdjuster.swift
+  Sequence+AsyncMap.swift
   SourceKitIndexDelegate.swift
   SourceKitLSPCommandMetadata.swift
   SourceKitServer+Options.swift

Sources/SourceKitLSP/Sequence+AsyncMap.swift

@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+extension Sequence {
+  /// Just like `Sequence.map` but allows an `async` transform function.
+  func asyncMap<T>(
+    _ transform: (Element) async throws -> T
+  ) async rethrows -> [T] {
+    var result: [T] = []
+    result.reserveCapacity(self.underestimatedCount)
+
+    for element in self {
+      try await result.append(transform(element))
+    }
+
+    return result
+  }
+
+  /// Just like `Sequence.compactMap` but allows an `async` transform function.
+  func asyncCompactMap<T>(
+    _ transform: (Element) async throws -> T?
+  ) async rethrows -> [T] {
+    var result: [T] = []
+
+    for element in self {
+      if let transformed = try await transform(element) {
+        result.append(transformed)
+      }
+    }
+
+    return result
+  }
+}
+