Make buildSettings as part of the key to the cache

Author: joehsieh

Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift

@@ -24,14 +24,15 @@ 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 ids
+  /// The cache that stores reports 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:
     [(
       snapshotID: DocumentSnapshot.ID,
+      buildSettings: SwiftCompileCommand?,
       report: RelatedFullDocumentDiagnosticReport
     )] = []
 
@@ -55,30 +56,41 @@ actor DiagnosticReportManager {
   func diagnosticReport(for snapshot: DocumentSnapshot, buildSettings: SwiftCompileCommand?, useCache: Bool = false)
     async throws -> RelatedFullDocumentDiagnosticReport
   {
-    if useCache, let report = report(for: snapshot.id) {
+    if useCache, let report = report(for: snapshot.id, buildSettings: buildSettings) {
       return report
     }
-    guard let buildSettings = buildSettings, !buildSettings.isFallback else {
+    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)
+      return report
+    }
+    guard !buildSettings.isFallback else {
       logger.log(
         "Producing syntactic diagnostics from the built-in swift-syntax because we have fallback arguments"
       )
-      // If we don't have build settings or we only have fallback build settings,
+      // If 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 fallbackReport(with: snapshot)
-      setReport(for: snapshot.id, report: report)
+      let report = try await requestFallbackReport(with: snapshot)
+      setReport(for: snapshot.id, buildSettings: buildSettings, report: report)
       return report
     }
-
-    let report = try await report(with: snapshot, compilerArgs: buildSettings.compilerArgs)
-    setReport(for: snapshot.id, report: report)
+    let report = try await requestReport(with: snapshot, compilerArgs: buildSettings.compilerArgs)
+    setReport(for: snapshot.id, buildSettings: buildSettings, report: report)
     return report
   }
 }
 
 private extension DiagnosticReportManager {
-  func report(with snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
+  func requestReport(with snapshot: DocumentSnapshot, compilerArgs: [String]) async throws
     -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
   {
     try Task.checkCancellation()
@@ -114,7 +126,7 @@ private extension DiagnosticReportManager {
     return RelatedFullDocumentDiagnosticReport(items: diagnostics)
   }
 
-  func fallbackReport(with snapshot: DocumentSnapshot) async throws
+  func requestFallbackReport(with snapshot: DocumentSnapshot) async throws
     -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport
   {
     // If we don't have build settings or we only have fallback build settings,
@@ -133,17 +145,23 @@ private extension DiagnosticReportManager {
     return RelatedFullDocumentDiagnosticReport(items: diagnostics)
   }
 
-  /// The report for the given document snapshot.
-  func report(for snapshotID: DocumentSnapshot.ID) -> RelatedFullDocumentDiagnosticReport? {
-    return reportCache.first(where: { $0.snapshotID == snapshotID })?.report
+  /// 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
   }
 
-  /// Set the report for the given document snapshot.
+  /// Set the report for the given document snapshot and buildSettings.
   ///
   /// If we are already storing `cacheSize` many reports, the oldest one
   /// will get discarded.
-  func setReport(for snapshotID: DocumentSnapshot.ID, report: RelatedFullDocumentDiagnosticReport) {
-    reportCache.insert((snapshotID, report), at: 0)
+  func setReport(
+    for snapshotID: DocumentSnapshot.ID,
+    buildSettings: SwiftCompileCommand?,
+    report: RelatedFullDocumentDiagnosticReport
+  ) {
+    reportCache.insert((snapshotID, buildSettings, report), at: 0)
 
     // Remove any reports for old versions of this document.
     reportCache.removeAll(where: { $0.snapshotID < snapshotID })

Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

@@ -159,7 +159,12 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
     self.state = .connected
     self.generatedInterfacesPath = options.generatedInterfacesPath.asURL
     try FileManager.default.createDirectory(at: generatedInterfacesPath, withIntermediateDirectories: true)
-    self.diagnosticReportManager = DiagnosticReportManager(sourcekitd: self.sourcekitd, syntaxTreeManager: syntaxTreeManager, documentManager: documentManager, clientHasDiagnosticsCodeDescriptionSupport: capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport)
+    self.diagnosticReportManager = DiagnosticReportManager(
+      sourcekitd: self.sourcekitd,
+      syntaxTreeManager: syntaxTreeManager,
+      documentManager: documentManager,
+      clientHasDiagnosticsCodeDescriptionSupport: capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport
+    )
   }
 
   /// - Important: For testing only
@@ -1280,4 +1285,4 @@ extension sourcekitd_uid_t {
       return nil
     }
   }
-}
+}