notifyBuildPhase was never called for Parsed state (#1572)

* fix: notifyBuildPhase was never called for Parsed state
* Added new unit test "Check all onBuidPhase callbacks"
* Improve code documentation and shorten required code
Co-authored-by: Mark Sujew <[email protected]>

Author: kaisalmen

package.json

@@ -13,6 +13,7 @@
     "build:clean": "npm run clean && npm run build",
     "lint": "npm run lint --workspaces",
     "test": "vitest",
+    "test:run": "vitest --run",
     "test-ui": "vitest --ui",
     "coverage": "vitest run --coverage",
     "validate-exports": "npm run validate-exports --workspace=langium",

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

@@ -352,14 +352,19 @@ export class DefaultDocumentBuilder implements DocumentBuilder {
 
     protected async runCancelable(documents: LangiumDocument[], targetState: DocumentState, cancelToken: CancellationToken,
         callback: (document: LangiumDocument) => MaybePromise<unknown>): Promise<void> {
-        const filtered = documents.filter(e => e.state < targetState);
+        const filtered = documents.filter(doc => doc.state < targetState);
         for (const document of filtered) {
             await interruptAndCheck(cancelToken);
             await callback(document);
             document.state = targetState;
             await this.notifyDocumentPhase(document, targetState, cancelToken);
         }
-        await this.notifyBuildPhase(filtered, targetState, cancelToken);
+
+        // Do not use `filtered` here, as that will miss documents that have previously reached the current target state
+        // For example, this happens in case the cancellation triggers between the processing of two documents
+        // Or files that were picked up during the workspace initialization
+        const targetStateDocs = documents.filter(doc => doc.state === targetState);
+        await this.notifyBuildPhase(targetStateDocs, targetState, cancelToken);
         this.currentState = targetState;
     }
 

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

@@ -82,6 +82,45 @@ describe('DefaultDocumentBuilder', () => {
         expect(called).toBe(true);
     });
 
+    test('Check all onBuidPhase callbacks', async () => {
+        const services = await createServices();
+        const documentFactory = services.shared.workspace.LangiumDocumentFactory;
+        const documents = services.shared.workspace.LangiumDocuments;
+        const document = documentFactory.fromString(`
+            foo 1 A
+            foo 11 B
+            bar A
+            bar B
+        `, URI.parse('file:///test1.txt'));
+        documents.addDocument(document);
+
+        const builder = services.shared.workspace.DocumentBuilder;
+        const awaiting: Array<Promise<void>> = [];
+        builder.onBuildPhase(DocumentState.Parsed, () => {
+            awaiting.push(Promise.resolve());
+        });
+        builder.onBuildPhase(DocumentState.IndexedContent, () => {
+            awaiting.push(Promise.resolve());
+        });
+        builder.onBuildPhase(DocumentState.ComputedScopes, () => {
+            awaiting.push(Promise.resolve());
+        });
+        builder.onBuildPhase(DocumentState.Linked, () => {
+            awaiting.push(Promise.resolve());
+        });
+        builder.onBuildPhase(DocumentState.IndexedReferences, () => {
+            awaiting.push(Promise.resolve());
+        });
+        builder.onBuildPhase(DocumentState.Validated, () => {
+            awaiting.push(Promise.resolve());
+        });
+
+        await builder.build([document], { validation: true });
+        expect(async () => await Promise.all(awaiting)).not.toThrowError();
+        expect(document.state).toBe(DocumentState.Validated);
+        expect(awaiting.length).toBe(6);
+    });
+
     test('resumes document build after cancellation', async () => {
         const services = await createServices();
         const documentFactory = services.shared.workspace.LangiumDocumentFactory;