Merge pull request #840 from ahoppen/ahoppen/re-apply-pr

Re-apply #834

Author: ahoppen

Sources/LSPLogging/Logging.swift

@@ -62,6 +62,22 @@ 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) -> Void
+public typealias CodeActionProvider = (CodeActionRequest, @escaping CodeActionProviderCompletion) async -> Void
 
 /// Request for returning all possible code actions for a given text document and range.
 ///

Sources/SKCore/BuildServerBuildSystem.swift

@@ -65,6 +65,9 @@ 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)
@@ -195,7 +198,18 @@ public final class BuildServerBuildSystem: MessageHandler {
     let result = notification.params.updatedOptions
     let settings = FileBuildSettings(
         compilerArguments: result.options, workingDirectory: result.workingDirectory)
-    self.delegate?.fileBuildSettingsChanged([notification.params.uri: .modified(settings)])
+    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])
+    }
   }
 }
 
@@ -208,19 +222,14 @@ private func readReponseDataKey(data: LSPAny?, key: String) -> String? {
   return nil
 }
 
-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 {
+  /// 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]
+  }
 
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
     let request = RegisterForChanges(uri: uri, action: .register)
@@ -230,7 +239,7 @@ extension BuildServerBuildSystem: BuildSystem {
 
         // BuildServer registration failed, so tell our delegate that no build
         // settings are available.
-        self.delegate?.fileBuildSettingsChanged([uri: .removedOrUnavailable])
+        self.buildSettingsChanged(for: uri, settings: nil)
       }
     })
   }

Sources/SKCore/BuildSystem.swift

@@ -51,6 +51,13 @@ 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,6 +146,41 @@ 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))")
@@ -222,7 +257,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.settings(for: mainFile, language) {
+      if let settings = fallback.buildSettings(for: mainFile, language: language) {
         newStatus = .primary(settings)
       } else {
         newStatus = .unsupported
@@ -235,7 +270,7 @@ extension BuildSystemManager {
   }
 
   /// *Must be called on queue*. Update and notify our delegate for the given
-  /// main file changes if they are convertable into `FileBuildSettingsChange`.
+  /// main file changes if they are convertible into `FileBuildSettingsChange`.
   func updateAndNotifyStatuses(changes: [DocumentURI: MainFileStatus]) {
     var changedWatchedFiles = [DocumentURI: FileBuildSettingsChange]()
     for (mainFile, status) in changes {
@@ -279,11 +314,11 @@ extension BuildSystemManager {
     _ fallback: FallbackBuildSystem
   ) {
     // There won't be a current status if it's unreferenced by any watched file.
-    // Simiarly, if the status isn't `waiting` then there's nothing to do.
+    // Similarly, 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.settings(for: mainFile, language) {
+    if let settings = fallback.buildSettings(for: mainFile, language: language) {
       self.updateAndNotifyStatuses(changes: [mainFile: .waitingUsingFallback(settings)])
     } else {
       // Keep the status as waiting.
@@ -336,9 +371,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 alteratively cache the fallback settings/language earlier?
+          // itself to pass it in here. Or alternatively cache the fallback settings/language earlier?
           let language = firstWatch.value.language
-          if let settings = fallback.settings(for: mainFile, language) {
+          if let settings = fallback.buildSettings(for: mainFile, language: language) {
             newStatus = .fallback(settings)
           } else {
             newStatus = .unsupported

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -93,6 +93,15 @@ 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 settings(for uri: DocumentURI, _ language: Language) -> FileBuildSettings? {
+  public func buildSettings(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.settings(for: uri, language)
+    let settings = self.buildSettings(for: uri, language: language)
     DispatchQueue.global().async {
       delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
     }

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

@@ -290,6 +290,18 @@ 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,