Make sure swiftc diagnostics are cleaned up after new swift commands (#915)

This will fix the issue mentioned. There is also an issue that SourceKit
and swiftc have different ranges reported which I'll reach out about

Issue: #912

Author: award999

src/DiagnosticsManager.ts

@@ -190,12 +190,13 @@ export class DiagnosticsManager implements vscode.Disposable {
     }
 
     private removeSwiftcDiagnostics() {
-        this.diagnosticCollection.forEach((uri, diagnostics) => {
+        this.allDiagnostics.forEach((diagnostics, path) => {
             const newDiagnostics = diagnostics.slice();
             this.removeDiagnostics(newDiagnostics, d => this.isSwiftc(d));
             if (diagnostics.length !== newDiagnostics.length) {
-                this.diagnosticCollection.set(uri, newDiagnostics);
+                this.allDiagnostics.set(path, newDiagnostics);
             }
+            this.updateDiagnosticsCollection(vscode.Uri.file(path));
         });
     }
 
@@ -243,10 +244,6 @@ export class DiagnosticsManager implements vscode.Disposable {
         return configuration.diagnosticsCollection !== "onlySourceKit";
     }
 
-    private includeSourceKitDiagnostics(): boolean {
-        return configuration.diagnosticsCollection !== "onlySwiftc";
-    }
-
     private parseDiagnostics(swiftExecution: SwiftExecution): Promise<DiagnosticsMap> {
         return new Promise<DiagnosticsMap>(res => {
             const diagnostics = new Map();

test/suite/DiagnosticsManager.test.ts

@@ -247,6 +247,32 @@ suite("DiagnosticsManager Test Suite", async function () {
                 assert.equal(diagnostics.length, 1);
                 assertHasDiagnostic(mainUri, outputDiagnostic);
             });
+
+            test("New set of swiftc diagnostics clear old list", async () => {
+                let fixture = testSwiftTask("swift", ["build"], workspaceFolder, toolchain);
+                await vscode.tasks.executeTask(fixture.task);
+                let diagnosticsPromise = waitForDiagnostics([mainUri]);
+                // Wait to spawn before writing
+                fixture.process.write(`${mainUri.fsPath}:13:5: error: Cannot find 'foo' in scope`);
+                fixture.process.close(1);
+                await waitForNoRunningTasks();
+                await diagnosticsPromise;
+                let diagnostics = vscode.languages.getDiagnostics(mainUri);
+                // Should only include one
+                assert.equal(diagnostics.length, 1);
+                assertHasDiagnostic(mainUri, outputDiagnostic);
+
+                // Run again but no diagnostics returned
+                fixture = testSwiftTask("swift", ["build"], workspaceFolder, toolchain);
+                await vscode.tasks.executeTask(fixture.task);
+                diagnosticsPromise = waitForDiagnostics([mainUri]);
+                fixture.process.close(0);
+                await waitForNoRunningTasks();
+                await diagnosticsPromise;
+                diagnostics = vscode.languages.getDiagnostics(mainUri);
+                // Should have cleaned up
+                assert.equal(diagnostics.length, 0);
+            });
         });
     });
 
@@ -346,30 +372,6 @@ suite("DiagnosticsManager Test Suite", async function () {
                 assertHasDiagnostic(mainUri, sourcekitErrorDiagnostic);
                 assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
             });
-
-            test("fix swiftc diagnostic", async () => {
-                // Add initial diagnostics
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
-                );
-                workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
-                    swiftcErrorDiagnostic,
-                ]);
-
-                // Have SourceKit some have been fixed
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitWarningDiagnostic]
-                );
-
-                // check kept all
-                assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
-                assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
-                assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
-            });
         });
 
         suite("keepSourceKit", () => {
@@ -477,30 +479,6 @@ suite("DiagnosticsManager Test Suite", async function () {
                 // kept unique swiftc diagnostic
                 assertHasDiagnostic(mainUri, swiftcWarningDiagnostic);
             });
-
-            test("fix swiftc diagnostic", async () => {
-                // Add initial diagnostics
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
-                );
-                workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
-                    swiftcErrorDiagnostic,
-                ]);
-
-                // Have SourceKit some have been fixed
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitWarningDiagnostic]
-                );
-
-                // check kept all
-                assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
-                assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
-                assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
-            });
         });
 
         suite("keepSwiftc", () => {
@@ -583,30 +561,6 @@ suite("DiagnosticsManager Test Suite", async function () {
                 // kept unique SourceKit diagnostic
                 assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
             });
-
-            test("fix swiftc diagnostic", async () => {
-                // Add initial diagnostics
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
-                );
-                workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
-                    swiftcErrorDiagnostic,
-                ]);
-
-                // Have SourceKit some have been fixed
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitWarningDiagnostic]
-                );
-
-                // check kept all
-                assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
-                assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
-                assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
-            });
         });
 
         suite("onlySourceKit", () => {
@@ -734,30 +688,64 @@ suite("DiagnosticsManager Test Suite", async function () {
                 assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
                 assertWithoutDiagnostic(mainUri, sourcekitWarningDiagnostic);
             });
+        });
 
-            test("fix swiftc diagnostic", async () => {
-                // Add initial diagnostics
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitErrorDiagnostic, sourcekitWarningDiagnostic]
-                );
-                workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
-                    swiftcErrorDiagnostic,
-                ]);
+        test("SourceKit removes swiftc diagnostic (SourceKit shows first)", async () => {
+            // Add initial diagnostics
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
+                sourcekitErrorDiagnostic,
+                sourcekitWarningDiagnostic,
+            ]);
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
+                swiftcErrorDiagnostic,
+            ]);
+
+            // Have SourceKit indicate some have been fixed
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
+                sourcekitWarningDiagnostic,
+            ]);
+
+            // check cleaned up stale error
+            assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
+            assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
+            assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
+        });
 
-                // Have SourceKit some have been fixed
-                workspaceContext.diagnostics.handleDiagnostics(
-                    mainUri,
-                    DiagnosticsManager.sourcekit,
-                    [sourcekitWarningDiagnostic]
-                );
+        test("SourceKit removes swiftc diagnostic (swiftc shows first)", async () => {
+            // Add initial diagnostics
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
+                swiftcErrorDiagnostic,
+                swiftcWarningDiagnostic,
+            ]);
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
+                sourcekitErrorDiagnostic,
+            ]);
+
+            // Have SourceKit indicate has been fixed
+            workspaceContext.diagnostics.handleDiagnostics(
+                mainUri,
+                DiagnosticsManager.sourcekit,
+                []
+            );
 
-                // check kept all
-                assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
-                assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
-                assertWithoutDiagnostic(mainUri, sourcekitWarningDiagnostic);
-            });
+            // check cleaned up stale error
+            assertWithoutDiagnostic(mainUri, swiftcErrorDiagnostic);
+            assertWithoutDiagnostic(mainUri, sourcekitErrorDiagnostic);
+            assertHasDiagnostic(mainUri, swiftcWarningDiagnostic);
+        });
+
+        test("don't remove swiftc diagnostics when SourceKit never matched", async () => {
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.swiftc, [
+                swiftcErrorDiagnostic,
+            ]);
+
+            workspaceContext.diagnostics.handleDiagnostics(mainUri, DiagnosticsManager.sourcekit, [
+                sourcekitWarningDiagnostic,
+            ]);
+
+            // Should not have cleaned up swiftc error
+            assertHasDiagnostic(mainUri, swiftcErrorDiagnostic);
+            assertHasDiagnostic(mainUri, sourcekitWarningDiagnostic);
         });
     });
 });