Ensure diagnostics are published after cancellation (#1566)

Author: msujew

package-lock.json

@@ -15,7 +15,7 @@
         "langium:watch": "langium generate --watch"
     },
     "dependencies": {
-        "langium": "~3.1.1"
+        "langium": "~3.1.2"
     },
     "devDependencies": {
         "@types/node": "^18.0.0",

packages/generator-langium/templates/core/.package.json

@@ -95,7 +95,7 @@
     "lint": "eslint src --ext ts"
   },
   "dependencies": {
-    "langium": "3.1.1",
+    "langium": "3.1.2",
     "langium-railroad": "3.1.0",
     "vscode-languageserver": "~9.0.1",
     "ignore": "~5.2.4"

packages/langium-vscode/package.json

@@ -1,5 +1,15 @@
 # Change Log of `langium`
 
+## v3.1.2 (Jul. 2024)
+
+* Fixes a minor issue in how we determine whether a grammar internal type is a primitive or an object type ([#1563](https://github.com/eclipse-langium/langium/pull/1563)).
+* Introduced a new `onDocumentPhase` method for the `DocumentBuilder` interface and fixed stale references in ASTs ([#1566](https://github.com/eclipse-langium/langium/pull/1566)).
+* Better handle empty names in the `DocumentSymbolProvider` ([#1565](https://github.com/eclipse-langium/langium/pull/1565)).
+
+## v3.1.1 (Jun. 2024)
+
+* Fixed an issue in a trigger-happy validation ([#1559](https://github.com/eclipse-langium/langium/pull/1559)).
+
 ## v3.1.0 (Jun. 2024)
 
 ### General Improvements

packages/langium/CHANGELOG.md

@@ -1,6 +1,6 @@
 {
   "name": "langium",
-  "version": "3.1.1",
+  "version": "3.1.2",
   "description": "A language engineering tool for the Language Server Protocol",
   "homepage": "https://langium.org",
   "engines": {

packages/langium/package.json

@@ -25,7 +25,6 @@ import { LangiumGrammarDefinitionProvider } from './lsp/grammar-definition.js';
 import { LangiumGrammarCallHierarchyProvider } from './lsp/grammar-call-hierarchy.js';
 import { LangiumGrammarValidationResourcesCollector } from './validation/validation-resources-collector.js';
 import { LangiumGrammarTypesValidator, registerTypeValidationChecks } from './validation/types-validator.js';
-import { interruptAndCheck } from '../utils/promise-utils.js';
 import { DocumentState } from '../workspace/documents.js';
 
 export type LangiumGrammarAddedServices = {
@@ -104,12 +103,9 @@ export function createLangiumGrammarServices(context: DefaultSharedModuleContext
 
 function addTypeCollectionPhase(sharedServices: LangiumSharedServices, grammarServices: LangiumGrammarServices) {
     const documentBuilder = sharedServices.workspace.DocumentBuilder;
-    documentBuilder.onBuildPhase(DocumentState.IndexedReferences, async (documents, cancelToken) => {
-        for (const document of documents) {
-            await interruptAndCheck(cancelToken);
-            const typeCollector = grammarServices.validation.ValidationResourcesCollector;
-            const grammar = document.parseResult.value as Grammar;
-            (document as LangiumGrammarDocument).validationResources = typeCollector.collectValidationResources(grammar);
-        }
+    documentBuilder.onDocumentPhase(DocumentState.IndexedReferences, async document => {
+        const typeCollector = grammarServices.validation.ValidationResourcesCollector;
+        const grammar = document.parseResult.value as Grammar;
+        (document as LangiumGrammarDocument).validationResources = typeCollector.collectValidationResources(grammar);
     });
 }

packages/langium/src/grammar/langium-grammar-module.ts

@@ -332,17 +332,12 @@ export function addDiagnosticsHandler(connection: Connection, services: LangiumS
             });
         }
     });
-    documentBuilder.onBuildPhase(DocumentState.Validated, async (documents, cancelToken) => {
-        for (const document of documents) {
-            if (document.diagnostics) {
-                connection.sendDiagnostics({
-                    uri: document.uri.toString(),
-                    diagnostics: document.diagnostics
-                });
-            }
-            if (cancelToken.isCancellationRequested) {
-                return;
-            }
+    documentBuilder.onDocumentPhase(DocumentState.Validated, async (document) => {
+        if (document.diagnostics) {
+            connection.sendDiagnostics({
+                uri: document.uri.toString(),
+                diagnostics: document.diagnostics
+            });
         }
     });
 }

packages/langium/src/lsp/language-server.ts

@@ -114,9 +114,12 @@ export class DefaultLinker implements Linker {
                     message: `An error occurred while resolving reference to '${ref.$refText}': ${err}`
                 };
             }
+            // Add the reference to the document's array of references
+            // Only add if the reference has been not been resolved earlier
+            // Otherwise we end up with duplicates
+            // See also implementation of `buildReference`
+            document.references.push(ref);
         }
-        // Add the reference to the document's array of references
-        document.references.push(ref);
     }
 
     unlink(document: LangiumDocument): void {
@@ -159,6 +162,8 @@ export class DefaultLinker implements Linker {
                     }
                     this._ref = refData.node ?? refData.error;
                     this._nodeDescription = refData.descr;
+                    const document = getDocument(node);
+                    document.references.push(this);
                 }
                 return isAstNode(this._ref) ? this._ref : undefined;
             },

packages/langium/src/references/linker.ts

@@ -13,9 +13,9 @@ import type { MaybePromise } from '../utils/promise-utils.js';
 import type { Deferred } from '../utils/promise-utils.js';
 import type { ValidationOptions } from '../validation/document-validator.js';
 import type { IndexManager } from '../workspace/index-manager.js';
-import type { LangiumDocument, LangiumDocuments, LangiumDocumentFactory } from './documents.js';
+import type { LangiumDocument, LangiumDocuments, LangiumDocumentFactory, TextDocumentProvider } from './documents.js';
 import { MultiMap } from '../utils/collections.js';
-import { OperationCancelled, interruptAndCheck } from '../utils/promise-utils.js';
+import { OperationCancelled, interruptAndCheck, isOperationCancelled } from '../utils/promise-utils.js';
 import { stream } from '../utils/stream.js';
 import type { URI } from '../utils/uri-utils.js';
 import { ValidationCategory } from '../validation/validation-registry.js';
@@ -78,10 +78,22 @@ export interface DocumentBuilder {
     onUpdate(callback: DocumentUpdateListener): Disposable;
 
     /**
-     * Notify the given callback when a set of documents has been built reaching a desired target state.
+     * Notify the given callback when a set of documents has been built reaching the specified target state.
      */
     onBuildPhase(targetState: DocumentState, callback: DocumentBuildListener): Disposable;
 
+    /**
+     * Notify the specified callback when a document has been built reaching the specified target state.
+     * Unlike {@link onBuildPhase} the listener is called for every single document.
+     *
+     * There are two main advantages compared to {@link onBuildPhase}:
+     * 1. If the build is cancelled, {@link onDocumentPhase} will still fire for documents that have reached a specific state.
+     *    Meanwhile, {@link onBuildPhase} won't fire for that state.
+     * 2. The {@link DocumentBuilder} ensures that all {@link DocumentPhaseListener} instances are called for a built document.
+     *    Even if the build is cancelled before those listeners were called.
+     */
+    onDocumentPhase(targetState: DocumentState, callback: DocumentPhaseListener): Disposable;
+
     /**
      * Wait until the workspace has reached the specified state for all documents.
      *
@@ -105,6 +117,7 @@ export interface DocumentBuilder {
 
 export type DocumentUpdateListener = (changed: URI[], deleted: URI[]) => void | Promise<void>
 export type DocumentBuildListener = (built: LangiumDocument[], cancelToken: CancellationToken) => void | Promise<void>
+export type DocumentPhaseListener = (built: LangiumDocument, cancelToken: CancellationToken) => void | Promise<void>
 export class DefaultDocumentBuilder implements DocumentBuilder {
 
     updateBuildOptions: BuildOptions = {
@@ -116,17 +129,20 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
 
     protected readonly langiumDocuments: LangiumDocuments;
     protected readonly langiumDocumentFactory: LangiumDocumentFactory;
+    protected readonly textDocuments: TextDocumentProvider | undefined;
     protected readonly indexManager: IndexManager;
     protected readonly serviceRegistry: ServiceRegistry;
     protected readonly updateListeners: DocumentUpdateListener[] = [];
     protected readonly buildPhaseListeners = new MultiMap<DocumentState, DocumentBuildListener>();
+    protected readonly documentPhaseListeners = new MultiMap<DocumentState, DocumentPhaseListener>();
     protected readonly buildState = new Map<string, DocumentBuildState>();
     protected readonly documentBuildWaiters = new Map<string, Deferred<void>>();
     protected currentState = DocumentState.Changed;
 
     constructor(services: LangiumSharedCoreServices) {
         this.langiumDocuments = services.workspace.LangiumDocuments;
         this.langiumDocumentFactory = services.workspace.LangiumDocumentFactory;
+        this.textDocuments = services.workspace.TextDocuments;
         this.indexManager = services.workspace.IndexManager;
         this.serviceRegistry = services.ServiceRegistry;
     }
@@ -209,22 +225,49 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
         // Only allow interrupting the execution after all state changes are done
         await interruptAndCheck(cancelToken);
 
-        // Collect all documents that we should rebuild
-        const rebuildDocuments = this.langiumDocuments.all
-            .filter(doc =>
-                // This includes those that were reported as changed and those that we selected for relinking
-                doc.state < DocumentState.Linked
-                // This includes those for which a previous build has been cancelled
-                || !this.buildState.get(doc.uri.toString())?.completed
-            )
-            .toArray();
+        // Collect and sort all documents that we should rebuild
+        const rebuildDocuments = this.sortDocuments(
+            this.langiumDocuments.all
+                .filter(doc =>
+                    // This includes those that were reported as changed and those that we selected for relinking
+                    doc.state < DocumentState.Linked
+                    // This includes those for which a previous build has been cancelled
+                    || !this.buildState.get(doc.uri.toString())?.completed
+                )
+                .toArray()
+        );
         await this.buildDocuments(rebuildDocuments, this.updateBuildOptions, cancelToken);
     }
 
     protected async emitUpdate(changed: URI[], deleted: URI[]): Promise<void> {
         await Promise.all(this.updateListeners.map(listener => listener(changed, deleted)));
     }
 
+    /**
+     * Sort the given documents by priority. By default, documents with an open text document are prioritized.
+     * This is useful to ensure that visible documents show their diagnostics before all other documents.
+     *
+     * This improves the responsiveness in large workspaces as users usually don't care about diagnostics
+     * in files that are currently not opened in the editor.
+     */
+    protected sortDocuments(documents: LangiumDocument[]): LangiumDocument[] {
+        const hasTextDocument = new Map<LangiumDocument, boolean>();
+        for (const doc of documents) {
+            hasTextDocument.set(doc, Boolean(this.textDocuments?.get(doc.uri.toString())));
+        }
+        return documents.sort((a, b) => {
+            const aHasDoc = hasTextDocument.get(a);
+            const bHasDoc = hasTextDocument.get(b);
+            if (aHasDoc && !bHasDoc) {
+                return -1;
+            } else if (!aHasDoc && bHasDoc) {
+                return 1;
+            } else {
+                return 0;
+            }
+        });
+    }
+
     /**
      * Check whether the given document should be relinked after changes were found in the given URIs.
      */
@@ -314,6 +357,7 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
             await interruptAndCheck(cancelToken);
             await callback(document);
             document.state = targetState;
+            await this.notifyDocumentPhase(document, targetState, cancelToken);
         }
         await this.notifyBuildPhase(filtered, targetState, cancelToken);
         this.currentState = targetState;
@@ -326,6 +370,13 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
         });
     }
 
+    onDocumentPhase(targetState: DocumentState, callback: DocumentPhaseListener): Disposable {
+        this.documentPhaseListeners.add(targetState, callback);
+        return Disposable.create(() => {
+            this.documentPhaseListeners.delete(targetState, callback);
+        });
+    }
+
     waitUntil(state: DocumentState, cancelToken?: CancellationToken): Promise<void>;
     waitUntil(state: DocumentState, uri?: URI, cancelToken?: CancellationToken): Promise<URI | undefined>;
     waitUntil(state: DocumentState, uriOrToken?: URI | CancellationToken, cancelToken?: CancellationToken): Promise<URI | undefined | void> {
@@ -366,6 +417,21 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
         });
     }
 
+    protected async notifyDocumentPhase(document: LangiumDocument, state: DocumentState, cancelToken: CancellationToken): Promise<void> {
+        const listeners = this.documentPhaseListeners.get(state);
+        for (const listener of listeners) {
+            try {
+                await listener(document, cancelToken);
+            } catch (err) {
+                // Ignore cancellation errors
+                // We want to finish the listeners before throwing
+                if (!isOperationCancelled(err)) {
+                    throw err;
+                }
+            }
+        }
+    }
+
     protected async notifyBuildPhase(documents: LangiumDocument[], state: DocumentState, cancelToken: CancellationToken): Promise<void> {
         if (documents.length === 0) {
             // Don't notify when no document has been processed

packages/langium/src/workspace/document-builder.ts

@@ -392,11 +392,13 @@ export interface LangiumDocuments {
 export class DefaultLangiumDocuments implements LangiumDocuments {
 
     protected readonly langiumDocumentFactory: LangiumDocumentFactory;
+    protected readonly serviceRegistry: ServiceRegistry;
 
     protected readonly documentMap: Map<string, LangiumDocument> = new Map();
 
     constructor(services: LangiumSharedCoreServices) {
         this.langiumDocumentFactory = services.workspace.LangiumDocumentFactory;
+        this.serviceRegistry = services.ServiceRegistry;
     }
 
     get all(): Stream<LangiumDocument> {
@@ -449,9 +451,10 @@ export class DefaultLangiumDocuments implements LangiumDocuments {
         const uriString = uri.toString();
         const langiumDoc = this.documentMap.get(uriString);
         if (langiumDoc) {
+            const linker = this.serviceRegistry.getServices(uri).references.Linker;
+            linker.unlink(langiumDoc);
             langiumDoc.state = DocumentState.Changed;
             langiumDoc.precomputedScopes = undefined;
-            langiumDoc.references = [];
             langiumDoc.diagnostics = undefined;
         }
         return langiumDoc;