Clear diagnostics of untitled files

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java

@@ -95,7 +95,6 @@
 import org.eclipse.lsp4j.PrepareRenameDefaultBehavior;
 import org.eclipse.lsp4j.PrepareRenameParams;
 import org.eclipse.lsp4j.PrepareRenameResult;
-import org.eclipse.lsp4j.PublishDiagnosticsParams;
 import org.eclipse.lsp4j.Range;
 import org.eclipse.lsp4j.ReferenceParams;
 import org.eclipse.lsp4j.RenameFilesParams;
@@ -339,12 +338,21 @@
 
     @Override
     public void didDeleteFiles(DeleteFilesParams params) {
-        exec.submit(() -> {
-            // if a file is deleted, and we were tracking it, we remove our diagnostics
-            for (var f : params.getFiles()) {
-                availableClient().publishDiagnostics(new PublishDiagnosticsParams(f.getUri(), List.of()));
+        // This method might also be called when files are deleted that aren't
+        // tracked by this text document service. "Special care" is needed to
+        // silently ignore such files (without throwing exceptions).
+        for (var f : params.getFiles()) {
+            var loc = Locations.toLoc(f.getUri());
+            // Special care: Cannot use method `language`, because it throws when `safeLanguage` returns an empty optional.
+            var language = safeLanguage(loc);
+            if (language.isPresent()) {
+                // Special care: Cannot use method `facts`, because it throws when `get` returns `null`.
+                var facts = this.facts.get(language.get());
+                if (facts != null) {
+                    facts.remove(loc);
+                }
             }
-        });
+        }
     }
 
     private void triggerAnalyzer(TextDocumentItem doc, Duration delay) {

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/model/ParametricFileFacts.java

@@ -163,10 +163,18 @@ public void close(ISourceLocation file) {
         getFile(file).close();
     }
 
+    public void remove(ISourceLocation file) {
+        var fact = files.remove(file);
+        if (fact != null) {
+            fact.remove();
+        }
+    }
+
     private interface FileFact {
         void invalidateAnalyzer(boolean isClosing);
         void invalidateBuilder(boolean isClosing);
         void close();
+        void remove();
         void calculateAnalyzer(CompletableFuture<Versioned<ITree>> tree, int version, Duration delay);
         void calculateBuilder(CompletableFuture<Versioned<ITree>> tree);
         void reportParseErrors(Versioned<List<Diagnostic>> messages);
@@ -177,6 +185,7 @@ private interface FileFact {
     @SuppressWarnings("java:S3077") // Reads/writes to fields of this class happen sequentially
     private class ActualFileFact implements FileFact {
         private final ISourceLocation file;
+        private volatile boolean removed = false;
 
         // To replace old diagnostics when new diagnostics become available in a
         // thread-safe way, we need to atomically: (1) check if the version of
@@ -237,17 +246,15 @@ public void close() {
                 if ((aMessages.isEmpty() && bMessages.isEmpty()) || !URIResolverRegistry.getInstance().exists(file)) {
                     // If there are no messages for this file or the file has been deleted, can we remove it
                     // else VS Code comes back and we've dropped the messages in our internal data
-                    remove(file);
+                    ParametricFileFacts.this.remove(file);
                 }
             });
         }
 
-        private @Nullable FileFact remove(ISourceLocation file) {
-            var removed = files.remove(file.top());
-            if (removed != null) {
-                removed.clearDiagnostics();
-            }
-            return removed;
+        @Override
+        public void remove() {
+            removed = true;
+            clearDiagnostics();
         }
 
         /**
@@ -363,6 +370,10 @@ public void clearDiagnostics() {
         }
 
         private void sendDiagnostics() {
+            if (removed) {
+                logger.debug("Will not send diagnostics since the file has been removed");
+                return;
+            }
             if (client == null) {
                 logger.debug("Cannot send diagnostics since the client hasn't been registered yet");
                 return;
@@ -383,9 +394,17 @@ private void sendDiagnostics() {
                 "Sending {} diagnostic(s) for {} (parser: v{}; analyzer: v{}; builder: v{})",
                 diagnostics.size(), file, fromParser.version(), fromAnalyzer.version(), fromBuilder.version());
 
-            client.publishDiagnostics(new PublishDiagnosticsParams(
-                Locations.toUri(file).toString(),
-                diagnostics));
+            var uri = Locations.toUri(file).toString();
+            client.publishDiagnostics(new PublishDiagnosticsParams(uri, diagnostics));
+
+            // The file for which diagnostics have just been sent, may have been
+            // deleted concurrently (i.e., after the `removed` check at the
+            // start of this method, but before the `publishDiagnostics` call).
+            // The resulting race may have caused the send to have accidentally
+            // overwritten the clear, so an additional clear might be needed.
+            if (removed) {
+                client.publishDiagnostics(new PublishDiagnosticsParams(uri, Collections.emptyList()));
+            }
         }
 
         /**
@@ -458,6 +477,11 @@ public void close() {
             // NOP
         }
 
+        @Override
+        public void remove() {
+            // NOP
+        }
+
         @Override
         public void calculateAnalyzer(CompletableFuture<Versioned<ITree>> tree, int version, Duration delay) {
             // NOP

rascal-vscode-extension/src/test/vscode-suite/dsl.test.ts

@@ -308,21 +308,31 @@ end
 
     it("completion works", async function() {
         const editor = await ide.openModule(TestWorkspace.picoFile);
-        await editor.setTextAtLine(6, "     aa : natural;");
+        try {
+            await editor.setTextAtLine(6, "     aa : natural;");
 
-        await editor.moveCursor(9, 4);
-        await bench.executeCommand("editor.action.triggerSuggest"); // 'completion', typically triggered with Ctrl+Space
-        await expectCompletions(driver, editor, ["a", "aa"]);
+            await editor.moveCursor(9, 4);
+            await bench.executeCommand("editor.action.triggerSuggest"); // 'completion', typically triggered with Ctrl+Space
+            await expectCompletions(driver, editor, ["a", "aa"]);
+        }
+        finally {
+            await ide.revertOpenChanges();
+        }
     });
 
     it("completion by trigger character works", async function() {
         // We will be typing and introducing parse errors, so this only works with error recovery
         if (!errorRecovery) { this.skip(); }
 
         const editor = await ide.openModule(TestWorkspace.picoFile);
-        await editor.moveCursor(10, 10);
-        await editor.typeText("  x :=");
-        await expectCompletions(driver, editor, ["a", "b", "n", "x"]);
+        try {
+            await editor.moveCursor(10, 10);
+            await editor.typeText("  x :=");
+            await expectCompletions(driver, editor, ["a", "b", "n", "x"]);
+        }
+        finally {
+            await ide.revertOpenChanges();
+        }
     });
 
     it("serializes Rascal values as expected", async function() {

rascal-vscode-extension/src/test/vscode-suite/ide.test.ts

@@ -251,11 +251,17 @@ describe('IDE', function () {
         const importerEditor = await ide.openModule(TestWorkspace.importerFile);
         const importeeEditor = await ide.openModule(TestWorkspace.importeeFile);
 
+        // Add type error
         await importeeEditor.typeTextAt(3, 1, "public str foo;");
         await ide.openModule(TestWorkspace.importerFile);
-
         await ide.triggerTypeChecker(importerEditor, {waitForFinish : true});
         await ide.hasErrorSquiggly(importerEditor);
+
+        // Remove type error (absence of error diagnostics is checked as part of the `cleanup` call in `afterEach`)
+        await ide.openModule(TestWorkspace.importeeFile);
+        await importeeEditor.setTextAtLine(3, "public int foo;");
+        await ide.openModule(TestWorkspace.importerFile);
+        await ide.triggerTypeChecker(importerEditor, {waitForFinish : true});
     });
 
     it("errors in manifest detected", async() => {

rascal-vscode-extension/src/test/vscode-suite/utils.ts

@@ -29,7 +29,7 @@ import { assert, expect } from "chai";
 import { stat, unlink } from "fs/promises";
 import * as os from 'os';
 import { env } from "process";
-import { BottomBarPanel, By, ContentAssist, EditorView, Key, Locator, TerminalView, TextEditor, VSBrowser, WebDriver, WebElement, WebElementCondition, Workbench, until } from "vscode-extension-tester";
+import { BottomBarPanel, By, ContentAssist, EditorView, Key, Locator, MarkerType, TerminalView, TextEditor, VSBrowser, WebDriver, WebElement, WebElementCondition, Workbench, until } from "vscode-extension-tester";
 import path = require("path");
 
 export async function sleep(ms: number) {
@@ -237,6 +237,12 @@ export class IDEOperations {
         const center = await ignoreFails(new Workbench().openNotificationsCenter());
         await ignoreFails(center?.clearAllNotifications());
         await ignoreFails(center?.close());
+
+        // There should be no more error diagnostics
+        const bottomBar = new Workbench().getBottomBar();
+        const problemsView = await bottomBar.openProblemsView();
+        const allVisibleMarkers = await problemsView.getAllVisibleMarkers(MarkerType.Error);
+        expect(allVisibleMarkers.length, "Not all error diagnostics have been cleared").to.equal(0);
     }
 
     assertLineBecomes(editor: TextEditor, lineNumber: number, lineContents: string, msg: string, wait = Delays.verySlow) : Promise<boolean> {