Merge pull request swiftlang#169 from benlangmuir/diag-cache-fix

[diagnostics] Fix caching of diagnostics across files and reopens

Author: benlangmuir

Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift

@@ -29,7 +29,7 @@ public final class SwiftLanguageServer: LanguageServer {
   // FIXME: ideally we wouldn't need separate management from a parent server in the same process.
   var documentManager: DocumentManager
 
-  var currentDiagnostics: [CachedDiagnostic] = []
+  var currentDiagnostics: [URL: [CachedDiagnostic]] = [:]
 
   var buildSettingsByFile: [URL: FileBuildSettings] = [:]
 
@@ -92,12 +92,14 @@ public final class SwiftLanguageServer: LanguageServer {
       return true
     }
 
-    let result = mergeDiagnostics(old: currentDiagnostics, new: newDiags, stage: stage)
-    currentDiagnostics = result
+    let document = snapshot.document.url
 
-    client.send(PublishDiagnostics(
-      url: snapshot.document.url,
-      diagnostics: result.map { $0.diagnostic }))
+    let result = mergeDiagnostics(
+      old: currentDiagnostics[document] ?? [],
+      new: newDiags, stage: stage)
+    currentDiagnostics[document] = result
+
+    client.send(PublishDiagnostics(url: document, diagnostics: result.map { $0.diagnostic }))
   }
 
   func handleDocumentUpdate(url: URL) {
@@ -275,9 +277,9 @@ extension SwiftLanguageServer {
 
     let url = note.params.textDocument.url
 
-    // Clear the build settings since there's no point in caching
-    // them for a closed file.
+    // Clear settings that should not be cached for closed documents.
     buildSettingsByFile[url] = nil
+    currentDiagnostics[url] = nil
 
     let req = SKRequestDictionary(sourcekitd: sourcekitd)
     req[keys.request] = requests.editor_close

Tests/SourceKitTests/LocalSwiftTests.swift

@@ -164,6 +164,104 @@ final class LocalSwiftTests: XCTestCase {
     })
   }
 
+  func testCrossFileDiagnostics() {
+    let urlA = URL(fileURLWithPath: "/a.swift")
+    let urlB = URL(fileURLWithPath: "/b.swift")
+
+    sk.allowUnexpectedNotification = false
+
+    sk.sendNoteSync(DidOpenTextDocument(textDocument: TextDocumentItem(
+      url: urlA, language: .swift, version: 12,
+      text: """
+      _ = foo()
+      """
+    )), { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - syntactic")
+      // 1 = semantic update finished already
+      // 0 = only syntactic
+      XCTAssertLessThanOrEqual(note.params.diagnostics.count, 1)
+    }, { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      XCTAssertEqual(
+        note.params.diagnostics.first?.range.lowerBound,
+        Position(line: 0, utf16index: 4))
+    })
+
+    sk.sendNoteSync(DidOpenTextDocument(textDocument: TextDocumentItem(
+      url: urlB, language: .swift, version: 12,
+      text: """
+      _ = bar()
+      """
+    )), { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - syntactic")
+      // 1 = semantic update finished already
+      // 0 = only syntactic
+      XCTAssertLessThanOrEqual(note.params.diagnostics.count, 1)
+    }, { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      XCTAssertEqual(
+        note.params.diagnostics.first?.range.lowerBound,
+        Position(line: 0, utf16index: 4))
+    })
+
+    sk.sendNoteSync(DidChangeTextDocument(textDocument: .init(urlA, version: 13), contentChanges: [
+      .init(range: nil, text: "_ = foo()\n")
+    ]), { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for edit 1 - syntactic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+    }, { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for edit 1 - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+    })
+  }
+
+  func testDiagnosticsReopen() {
+    let urlA = URL(fileURLWithPath: "/a.swift")
+    sk.allowUnexpectedNotification = false
+
+    sk.sendNoteSync(DidOpenTextDocument(textDocument: TextDocumentItem(
+      url: urlA, language: .swift, version: 12,
+      text: """
+      _ = foo()
+      """
+    )), { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - syntactic")
+      // 1 = semantic update finished already
+      // 0 = only syntactic
+      XCTAssertLessThanOrEqual(note.params.diagnostics.count, 1)
+    }, { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      XCTAssertEqual(
+        note.params.diagnostics.first?.range.lowerBound,
+        Position(line: 0, utf16index: 4))
+    })
+
+    sk.send(DidCloseTextDocument(textDocument: .init(urlA)))
+
+    sk.sendNoteSync(DidOpenTextDocument(textDocument: TextDocumentItem(
+      url: urlA, language: .swift, version: 13,
+      text: """
+      var
+      """
+    )), { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - syntactic")
+      // 1 = syntactic, no cached semantic diagnostic from previous version
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      XCTAssertEqual(
+        note.params.diagnostics.first?.range.lowerBound,
+        Position(line: 0, utf16index: 3))
+    }, { (note: Notification<PublishDiagnostics>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      XCTAssertEqual(
+        note.params.diagnostics.first?.range.lowerBound,
+        Position(line: 0, utf16index: 3))
+    })
+  }
+
   func testXMLToMarkdownDeclaration() {
     XCTAssertEqual(try! xmlDocumentationToMarkdown("""
       <Declaration>func foo(_ bar: <Type usr="fake">Baz</Type>)</Declaration>

Tests/SourceKitTests/XCTestManifests.swift

@@ -96,6 +96,8 @@ extension LocalSwiftTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__LocalSwiftTests = [
+        ("testCrossFileDiagnostics", testCrossFileDiagnostics),
+        ("testDiagnosticsReopen", testDiagnosticsReopen),
         ("testDocumentSymbolHighlight", testDocumentSymbolHighlight),
         ("testEditing", testEditing),
         ("testEditorPlaceholderParsing", testEditorPlaceholderParsing),