Add ReportTask and refine the code

Author: joehsieh

Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift

@@ -16,6 +16,9 @@ import SourceKitD
 import SwiftParserDiagnostics
 
 actor DiagnosticReportManager {
+  /// A task that gets report from sourcekitd
+  private typealias ReportTask = Task<RelatedFullDocumentDiagnosticReport, Error>
+
   private let sourcekitd: SourceKitD
   private let syntaxTreeManager: SyntaxTreeManager
   private let documentManager: DocumentManager
@@ -24,19 +27,19 @@ actor DiagnosticReportManager {
   private nonisolated var keys: sourcekitd_keys { return sourcekitd.keys }
   private nonisolated var requests: sourcekitd_requests { return sourcekitd.requests }
 
-  /// The cache that stores reports for snapshot id and buildSettings
+  /// The cache that stores reportTasks for snapshot id and buildSettings
   ///
   /// Conceptually, this is a dictionary. To prevent excessive memory usage we
   /// only keep `cacheSize` entries within the array. Older entries are at the
   /// end of the list, newer entries at the front.
-  private var reportCache:
+  private var reportTaskCache:
     [(
       snapshotID: DocumentSnapshot.ID,
       buildSettings: SwiftCompileCommand?,
-      report: RelatedFullDocumentDiagnosticReport
+      reportTask: ReportTask
     )] = []
 
-  /// The number of reports to keep
+  /// The number of reportTasks to keep
   ///
   /// - Note: This has been chosen without scientific measurements.
   private let cacheSize = 5
@@ -53,46 +56,39 @@ actor DiagnosticReportManager {
     self.clientHasDiagnosticsCodeDescriptionSupport = clientHasDiagnosticsCodeDescriptionSupport
   }
 
-  func diagnosticReport(for snapshot: DocumentSnapshot, buildSettings: SwiftCompileCommand?, useCache: Bool = false)
-    async throws -> RelatedFullDocumentDiagnosticReport
-  {
-    if useCache, let report = report(for: snapshot.id, buildSettings: buildSettings) {
-      return report
-    }
-    guard let buildSettings = buildSettings else {
-      logger.log(
-        "Producing syntactic diagnostics from the built-in swift-syntax because we don't have buildSettings"
-      )
-      // If we don't have build settings,
-      // sourcekitd won't be able to give us accurate semantic diagnostics.
-      // Fall back to providing syntactic diagnostics from the built-in
-      // swift-syntax. That's the best we can do for now.
-      let report = try await requestFallbackReport(with: snapshot)
-      setReport(for: snapshot.id, buildSettings: nil, report: report)
+  func diagnosticReport(
+    for snapshot: DocumentSnapshot,
+    buildSettings: SwiftCompileCommand?,
+    useCache: Bool = false
+  ) async throws -> RelatedFullDocumentDiagnosticReport {
+    if useCache, let report = try await reportTask(for: snapshot.id, buildSettings: buildSettings)?.value {
       return report
     }
-    guard !buildSettings.isFallback else {
+    let reportTask: Task<RelatedFullDocumentDiagnosticReport, Error>
+    if let buildSettings, !buildSettings.isFallback {
+      reportTask = Task {
+        return try await requestReport(with: snapshot, compilerArgs: buildSettings.compilerArgs)
+      }
+    } else {
       logger.log(
-        "Producing syntactic diagnostics from the built-in swift-syntax because we have fallback arguments"
+        "Producing syntactic diagnostics from the built-in swift-syntax because we \(buildSettings != nil ? "have fallback build settings" : "don't have build settings")"
       )
-      // If we only have fallback build settings,
+      // If we don't have build settings or we only have fallback build settings,
       // sourcekitd won't be able to give us accurate semantic diagnostics.
       // Fall back to providing syntactic diagnostics from the built-in
       // swift-syntax. That's the best we can do for now.
-      let report = try await requestFallbackReport(with: snapshot)
-      setReport(for: snapshot.id, buildSettings: buildSettings, report: report)
-      return report
+      reportTask = Task {
+        return try await requestFallbackReport(with: snapshot)
+      }
     }
-    let report = try await requestReport(with: snapshot, compilerArgs: buildSettings.compilerArgs)
-    setReport(for: snapshot.id, buildSettings: buildSettings, report: report)
-    return report
+    setReportTask(for: snapshot.id, buildSettings: buildSettings, reportTask: reportTask)
+    return try await reportTask.value
   }
-}
 
-private extension DiagnosticReportManager {
-  func requestReport(with snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
-    -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
-  {
+  private func requestReport(
+    with snapshot: DocumentSnapshot,
+    compilerArgs: [String]
+  ) async throws -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport {
     try Task.checkCancellation()
 
     let keys = self.keys
@@ -111,13 +107,16 @@ private extension DiagnosticReportManager {
       // Check that the document wasn't modified while we were getting diagnostics. This could happen because we are
       // calling `fullDocumentDiagnosticReport` from `publishDiagnosticsIfNeeded` outside of `messageHandlingQueue`
       // and thus a concurrent edit is possible while we are waiting for the sourcekitd request to return a result.
-      throw ResponseError.unknown("Document was modified while loading document")
+      throw ResponseError.unknown("Document was modified while loading diagnostics")
     }
 
-    let supportsCodeDescription = self.clientHasDiagnosticsCodeDescriptionSupport
     var diagnostics: [Diagnostic] = []
     dict[keys.diagnostics]?.forEach { _, diag in
-      if let diag = Diagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) {
+      if let diag = Diagnostic(
+        diag,
+        in: snapshot,
+        useEducationalNoteAsCode: self.clientHasDiagnosticsCodeDescriptionSupport
+      ) {
         diagnostics.append(diag)
       }
       return true
@@ -126,9 +125,9 @@ private extension DiagnosticReportManager {
     return RelatedFullDocumentDiagnosticReport(items: diagnostics)
   }
 
-  func requestFallbackReport(with snapshot: DocumentSnapshot) async throws
-    -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
-  {
+  private func requestFallbackReport(
+    with snapshot: DocumentSnapshot
+  ) async throws -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport {
     // If we don't have build settings or we only have fallback build settings,
     // sourcekitd won't be able to give us accurate semantic diagnostics.
     // Fall back to providing syntactic diagnostics from the built-in
@@ -145,31 +144,33 @@ private extension DiagnosticReportManager {
     return RelatedFullDocumentDiagnosticReport(items: diagnostics)
   }
 
-  /// The report for the given document snapshot and buildSettings.
-  func report(for snapshotID: DocumentSnapshot.ID, buildSettings: SwiftCompileCommand?)
-    -> RelatedFullDocumentDiagnosticReport?
-  {
-    return reportCache.first(where: { $0.snapshotID == snapshotID && $0.buildSettings == buildSettings })?.report
+  /// The reportTask for the given document snapshot and buildSettings.
+  private func reportTask(
+    for snapshotID: DocumentSnapshot.ID,
+    buildSettings: SwiftCompileCommand?
+  ) -> ReportTask? {
+    return reportTaskCache.first(where: { $0.snapshotID == snapshotID && $0.buildSettings == buildSettings })?
+      .reportTask
   }
 
-  /// Set the report for the given document snapshot and buildSettings.
+  /// Set the reportTask for the given document snapshot and buildSettings.
   ///
   /// If we are already storing `cacheSize` many reports, the oldest one
   /// will get discarded.
-  func setReport(
+  private func setReportTask(
     for snapshotID: DocumentSnapshot.ID,
     buildSettings: SwiftCompileCommand?,
-    report: RelatedFullDocumentDiagnosticReport
+    reportTask: ReportTask
   ) {
-    reportCache.insert((snapshotID, buildSettings, report), at: 0)
+    reportTaskCache.insert((snapshotID, buildSettings, reportTask), at: 0)
 
-    // Remove any reports for old versions of this document.
-    reportCache.removeAll(where: { $0.snapshotID < snapshotID })
+    // Remove any reportTasks for old versions of this document.
+    reportTaskCache.removeAll(where: { $0.snapshotID < snapshotID })
 
-    // If we still have more than `cacheSize` reports, delete the ones that
+    // If we still have more than `cacheSize` reportTasks, delete the ones that
     // were produced last. We can always re-request them on-demand.
-    while reportCache.count > cacheSize {
-      reportCache.removeLast()
+    while reportTaskCache.count > cacheSize {
+      reportTaskCache.removeLast()
     }
   }
 }