Merge pull request #838 from bnbarham/revert-actors

Revert ahoppen/toolchain-language-servers-actor

Author: DougGregor

Sources/LSPLogging/Logging.swift

@@ -62,22 +62,6 @@ public func orLog<R>(
   }
 }
 
-/// Like `try?`, but logs the error on failure.
-public func orLog<R>(
-  _ prefix: String = "",
-  level: LogLevel = .default,
-  logger: Logger = Logger.shared,
-  _ block: () async throws -> R?) async -> R?
-{
-  do {
-    return try await block()
-  } catch {
-    logger.log("\(prefix)\(prefix.isEmpty ? "" : " ")\(error)", level: level)
-    return nil
-  }
-}
-
-
 /// Logs the time that the given block takes to execute in milliseconds.
 public func logExecutionTime<R>(
   _ prefix: String = #function,

Sources/LanguageServerProtocol/Requests/CodeActionRequest.swift

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 public typealias CodeActionProviderCompletion = (LSPResult<[CodeAction]>) -> Void
-public typealias CodeActionProvider = (CodeActionRequest, @escaping CodeActionProviderCompletion) async -> Void
+public typealias CodeActionProvider = (CodeActionRequest, @escaping CodeActionProviderCompletion) -> Void
 
 /// Request for returning all possible code actions for a given text document and range.
 ///

Sources/LanguageServerProtocolJSONRPC/JSONRPCConnection.swift

@@ -286,22 +286,14 @@ public final class JSONRPCConnection {
         await notification._handle(self.receiveHandler!, connection: self)
       }
     case .request(let request, id: let id):
+      let semaphore: DispatchSemaphore? = syncRequests ? .init(value: 0) : nil
       messageHandlingQueue.async {
-        if self.syncRequests {
-          await withCheckedContinuation { continuation in
-            Task {
-              await request._handle(self.receiveHandler!, id: id, connection: self) { (response, id) in
-                self.sendReply(response, id: id)
-                continuation.resume()
-              }
-            }
-          }
-        } else {
-          await request._handle(self.receiveHandler!, id: id, connection: self) { (response, id) in
-            self.sendReply(response, id: id)
-          }
+        await request._handle(self.receiveHandler!, id: id, connection: self) { (response, id) in
+          self.sendReply(response, id: id)
         }
+        semaphore?.signal()
       }
+      semaphore?.wait()
 
     case .response(let response, id: let id):
       guard let outstanding = outstandingRequests.removeValue(forKey: id) else {

Sources/SKCore/BuildServerBuildSystem.swift

@@ -65,9 +65,6 @@ public final class BuildServerBuildSystem: MessageHandler {
   /// Delegate to handle any build system events.
   public weak var delegate: BuildSystemDelegate?
 
-  /// 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 {
     let configPath = projectRoot.appending(component: "buildServer.json")
     let config = try loadBuildServerConfig(path: configPath, fileSystem: fileSystem)
@@ -198,18 +195,7 @@ public final class BuildServerBuildSystem: MessageHandler {
     let result = notification.params.updatedOptions
     let settings = FileBuildSettings(
         compilerArguments: result.options, workingDirectory: result.workingDirectory)
-    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?) {
-    buildSettings[document] = settings
-    if let settings {
-      self.delegate?.fileBuildSettingsChanged([document: .modified(settings)])
-    } else {
-      self.delegate?.fileBuildSettingsChanged([document: .removedOrUnavailable])
-    }
+    self.delegate?.fileBuildSettingsChanged([notification.params.uri: .modified(settings)])
   }
 }
 
@@ -222,14 +208,19 @@ private func readReponseDataKey(data: LSPAny?, key: String) -> String? {
   return nil
 }
 
-extension BuildServerBuildSystem: BuildSystem {
-  /// The build settings for the given file.
-  ///
-  /// Returns `nil` if no build settings have been received from the build
-  /// server yet or if no build settings are available for this file.
-  public func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? {
-    return buildSettings[document]
+extension BuildServerBuildSystem {
+  /// Exposed for *testing*.
+  public func _settings(for uri: DocumentURI) -> FileBuildSettings? {
+    if let response = try? self.buildServer?.sendSync(SourceKitOptions(uri: uri)) {
+      return FileBuildSettings(
+        compilerArguments: response.options,
+        workingDirectory: response.workingDirectory)
+    }
+    return nil
   }
+}
+
+extension BuildServerBuildSystem: BuildSystem {
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     let request = RegisterForChanges(uri: uri, action: .register)
@@ -239,7 +230,7 @@ extension BuildServerBuildSystem: BuildSystem {
 
         // BuildServer registration failed, so tell our delegate that no build
         // settings are available.
-        self.buildSettingsChanged(for: uri, settings: nil)
+        self.delegate?.fileBuildSettingsChanged([uri: .removedOrUnavailable])
       }
     })
   }

Sources/SKCore/BuildSystem.swift

@@ -51,13 +51,6 @@ public protocol BuildSystem: AnyObject {
   /// initial reports as well as changes.
   var delegate: BuildSystemDelegate? { get set }
 
-  /// Retrieve build settings for the given document with the given source
-  /// language.
-  ///
-  /// Returns `nil` if the build system can't provide build settings for this
-  /// file or if it hasn't computed build settings for the file yet.
-  func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings?
-
   /// Register the given file for build-system level change notifications, such
   /// as command line flag changes, dependency changes, etc.
   ///

Sources/SKCore/BuildSystemManager.swift

@@ -146,41 +146,6 @@ extension BuildSystemManager {
     set { queue.sync { _mainFilesProvider = newValue } }
   }
 
-  /// Get the build settings for the given document, assuming it has the given
-  /// language.
-  ///
-  /// Returns `nil` if no build settings are available in the build system and
-  /// no fallback build settings can be computed.
-  ///
-  /// `isFallback` is `true` if the build settings couldn't be computed and
-  /// fallback settings are used. These fallback settings are most likely not
-  /// correct and provide limited semantic functionality.
-  public func buildSettings(
-    for document: DocumentURI,
-    language: Language
-  ) async -> (buildSettings: FileBuildSettings, isFallback: Bool)? {
-    do {
-      // FIXME: (async) We should only wait `fallbackSettingsTimeout` for build
-      // settings and return fallback afterwards. I am not sure yet, how best to
-      // implement that with Swift concurrency.
-      // For now, this should be fine because all build systems return
-      // very quickly from `settings(for:language:)`.
-      if let settings = try await buildSystem?.buildSettings(for: document, language: language) {
-        return (buildSettings: settings, isFallback: false)
-      }
-    } catch {
-      log("Getting build settings failed: \(error)")
-    }
-    if let settings = fallbackBuildSystem?.buildSettings(for: document, language: language) {
-      // If there is no build system and we only have the fallback build system,
-      // we will never get real build settings. Consider the build settings
-      // non-fallback.
-      return (buildSettings: settings, isFallback: buildSystem != nil)
-    } else {
-      return nil
-    }
-  }
-
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     return queue.async {
       log("registerForChangeNotifications(\(uri.pseudoPath))")
@@ -257,7 +222,7 @@ extension BuildSystemManager {
     } else if let fallback = self.fallbackBuildSystem {
       // Only have a fallback build system. We consider it be a primary build
       // system that functions synchronously.
-      if let settings = fallback.buildSettings(for: mainFile, language: language) {
+      if let settings = fallback.settings(for: mainFile, language) {
         newStatus = .primary(settings)
       } else {
         newStatus = .unsupported
@@ -270,7 +235,7 @@ extension BuildSystemManager {
   }
 
   /// *Must be called on queue*. Update and notify our delegate for the given
-  /// main file changes if they are convertible into `FileBuildSettingsChange`.
+  /// main file changes if they are convertable into `FileBuildSettingsChange`.
   func updateAndNotifyStatuses(changes: [DocumentURI: MainFileStatus]) {
     var changedWatchedFiles = [DocumentURI: FileBuildSettingsChange]()
     for (mainFile, status) in changes {
@@ -314,11 +279,11 @@ extension BuildSystemManager {
     _ fallback: FallbackBuildSystem
   ) {
     // There won't be a current status if it's unreferenced by any watched file.
-    // Similarly, if the status isn't `waiting` then there's nothing to do.
+    // Simiarly, if the status isn't `waiting` then there's nothing to do.
     guard let status = self.mainFileStatuses[mainFile], status == .waiting else {
       return
     }
-    if let settings = fallback.buildSettings(for: mainFile, language: language) {
+    if let settings = fallback.settings(for: mainFile, language) {
       self.updateAndNotifyStatuses(changes: [mainFile: .waitingUsingFallback(settings)])
     } else {
       // Keep the status as waiting.
@@ -371,9 +336,9 @@ extension BuildSystemManager: BuildSystemDelegate {
           newStatus = settingsChange.isFallback ? .fallback(newSettings) : .primary(newSettings)
         } else if let fallback = self.fallbackBuildSystem {
           // FIXME: we need to stop threading the language everywhere, or we need the build system
-          // itself to pass it in here. Or alternatively cache the fallback settings/language earlier?
+          // itself to pass it in here. Or alteratively cache the fallback settings/language earlier?
           let language = firstWatch.value.language
-          if let settings = fallback.buildSettings(for: mainFile, language: language) {
+          if let settings = fallback.settings(for: mainFile, language) {
             newStatus = .fallback(settings)
           } else {
             newStatus = .unsupported

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -93,15 +93,6 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
 
   public var indexPrefixMappings: [PathPrefixMapping] { return [] }
 
-  public func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? {
-    // FIXME: (async) Convert this to an async function once `CompilationDatabaseBuildSystem` is an actor.
-    return await withCheckedContinuation { continuation in
-      self.queue.async {
-        continuation.resume(returning: self.settings(for: document))
-      }
-    }
-  }
-
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     queue.async {
       self.watchedFiles[uri] = language

Sources/SKCore/FallbackBuildSystem.swift

@@ -44,7 +44,7 @@ public final class FallbackBuildSystem: BuildSystem {
 
   public var indexPrefixMappings: [PathPrefixMapping] { return [] }
 
-  public func buildSettings(for uri: DocumentURI, language: Language) -> FileBuildSettings? {
+  public func settings(for uri: DocumentURI, _ language: Language) -> FileBuildSettings? {
     switch language {
     case .swift:
       return settingsSwift(uri.pseudoPath)
@@ -58,7 +58,7 @@ public final class FallbackBuildSystem: BuildSystem {
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     guard let delegate = self.delegate else { return }
 
-    let settings = self.buildSettings(for: uri, language: language)
+    let settings = self.settings(for: uri, language)
     DispatchQueue.global().async {
       delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
     }

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -290,18 +290,6 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
     }
   }
 
-  public func buildSettings(for document: DocumentURI, language: Language) async throws -> FileBuildSettings? {
-    return try await withCheckedThrowingContinuation { continuation in
-      queue.async {
-        do {
-          continuation.resume(returning: try self.settings(for: document, language))
-        } catch {
-          continuation.resume(throwing: error)
-        }
-      }
-    }
-  }
-
   /// Must only be called on `queue`.
   private func settings(
     for uri: DocumentURI,