Emit the afterProgramValidate event even when validation is cancell…

…ed (#1381)

* Prevent simultaneous validate() for same project (was causing lost diagnostics)

* Fix lint issues

* Throw exceptions in sequencer (to ensure we don't get stuck in validate forever)

* Emit the `afterProgramValidate` event even when validation is cancelled

src/Program.spec.ts

@@ -1,7 +1,7 @@
 import { assert, expect } from './chai-config.spec';
 import * as pick from 'object.pick';
 import * as sinonImport from 'sinon';
-import { CompletionItemKind, Position, Range } from 'vscode-languageserver';
+import { CancellationTokenSource, CompletionItemKind, Position, Range } from 'vscode-languageserver';
 import * as fsExtra from 'fs-extra';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import type { BrsFile } from './files/BrsFile';
@@ -20,9 +20,10 @@ import { isBrsFile } from './astUtils/reflection';
 import type { LiteralExpression } from './parser/Expression';
 import type { AstEditor } from './astUtils/AstEditor';
 import { tempDir, rootDir, stagingDir } from './testHelpers.spec';
-import type { BsDiagnostic } from './interfaces';
+import type { BsDiagnostic, CompilerPlugin } from './interfaces';
 import { createLogger } from './logging';
 import { Scope } from './Scope';
+import undent from 'undent';
 
 let sinon = sinonImport.createSandbox();
 
@@ -226,6 +227,76 @@ describe('Program', () => {
     });
 
     describe('validate', () => {
+        it('does not lose scope diagnostics in second validation after cancelling the previous validation', async () => {
+            program.setFile('source/Direction.bs', `
+                enum Direction
+                    up = "up"
+                end enum
+            `);
+            program.setFile('source/test.bs', `
+                import "Direction.bs"
+                sub test()
+                    print Direction.down
+                end sub
+            `);
+
+            //add several scopes so we have time to cancel the validation
+            for (let i = 0; i < 3; i++) {
+                program.setFile(`components/Component${i}.xml`, undent`
+                    <component name="Component${i}" extends="Group">
+                        <script uri="pkg:/source/test.bs" />
+                    </component>
+                `);
+            }
+            program.validate();
+            //ensure the diagnostic is there during normal run
+            expectDiagnostics(program, [
+                DiagnosticMessages.unknownEnumValue('down', 'Direction').message
+            ]);
+
+            const cancel = new CancellationTokenSource();
+
+            let count = 0;
+            const plugin = {
+                name: 'cancel validation',
+                beforeProgramValidate: () => {
+                    count++;
+                    //if this is the second validate, remove the plugin and change the file to invalidate the scopes again
+                    if (count === 2) {
+                        program.plugins.remove(plugin);
+                        program.setFile('source/test.bs', program.getFile('source/test.bs').fileContents + `'comment`);
+                    }
+                },
+                afterScopeValidate: () => {
+                    //if the diagnostic is avaiable, we know it's safe to cancel
+                    if (program.getDiagnostics().find(x => x.code === DiagnosticMessages.unknownEnumValue('down', 'Direction').code)) {
+                        cancel.cancel();
+                    }
+                }
+            } as CompilerPlugin;
+            //add a plugin that monitors where we are in the process, so we can cancel the validate at the correct time
+            program.plugins.add(plugin);
+
+            //change the file so it forces a reload
+            program.setFile('source/test.bs', program.getFile('source/test.bs').fileContents + `'comment`);
+
+            //now trigger two validations, the first one will be cancelled, the second one will run to completion
+            await Promise.all([
+                program.validate({
+                    async: true,
+                    cancellationToken: cancel.token
+                }),
+                program.validate({
+                    async: true
+                })
+            ]);
+
+            //ensure the diagnostic is there after a cancelled run (with a subsequent completed run)
+            expectDiagnostics(program, [
+                DiagnosticMessages.unknownEnumValue('down', 'Direction').message
+            ]);
+        });
+
         it('validate (sync) passes along inner exception', () => {
             program.setFile('source/main.brs', ``);
             sinon.stub(Scope.prototype, 'validate').callsFake(() => {

src/Program.ts

@@ -661,7 +661,7 @@ export class Program {
     /**
      * Counter used to track which validation run is being logged
      */
-    private validationRunSequence = 0;
+    private validationRunSequence = 1;
 
     /**
      * How many milliseconds can pass while doing synchronous operations in validate before we register a short timeout (i.e. yield to the event loop)
@@ -677,7 +677,8 @@ export class Program {
     public validate(options: { async: false; cancellationToken?: CancellationToken }): void;
     public validate(options: { async: true; cancellationToken?: CancellationToken }): Promise<void>;
     public validate(options?: { async?: boolean; cancellationToken?: CancellationToken }) {
-        const timeEnd = this.logger.timeStart(LogLevel.log, `Validating project${(this.logger.logLevel as LogLevel) > LogLevel.log ? ` (run ${this.validationRunSequence++})` : ''}`);
+        const validationRunId = this.validationRunSequence++;
+        const timeEnd = this.logger.timeStart(LogLevel.log, `Validating project${(this.logger.logLevel as LogLevel) > LogLevel.log ? ` (run ${validationRunId})` : ''}`);
 
         let previousValidationPromise = this.validatePromise;
         const deferred = new Deferred();
@@ -693,12 +694,25 @@ export class Program {
             throw new Error('Cannot run synchronous validation while an async validation is in progress');
         }
 
+        if (options?.async) {
+            //we're async, so create a new promise chain to resolve after this validation is done
+            this.validatePromise = Promise.resolve(previousValidationPromise).then(() => {
+                return deferred.promise;
+            });
+
+            //we are not async but there's a pending promise, then we cannot run this validation
+        } else if (previousValidationPromise !== undefined) {
+            throw new Error('Cannot run synchronous validation while an async validation is in progress');
+        }
+
         const sequencer = new Sequencer({
             name: 'program.validate',
             cancellationToken: options?.cancellationToken ?? new CancellationTokenSource().token,
             minSyncDuration: this.validationMinSyncDuration
         });
 
+        let beforeProgramValidateWasEmitted = false;
+
         //this sequencer allows us to run in both sync and async mode, depending on whether options.async is enabled.
         //We use this to prevent starving the CPU during long validate cycles when running in a language server context
         sequencer
@@ -711,6 +725,7 @@ export class Program {
             .once(() => {
                 this.diagnostics = [];
                 this.plugins.emit('beforeProgramValidate', this);
+                beforeProgramValidateWasEmitted = true;
             })
             .forEach(Object.values(this.files), (file) => {
                 if (!file.isValidated) {
@@ -738,7 +753,6 @@ export class Program {
             })
             .once(() => {
                 this.detectDuplicateComponentNames();
-                this.plugins.emit('afterProgramValidate', this);
             })
             .onCancel(() => {
                 timeEnd('cancelled');
@@ -747,6 +761,12 @@ export class Program {
                 timeEnd();
             })
             .onComplete(() => {
+                //if we emitted the beforeProgramValidate hook, emit the afterProgramValidate hook as well
+                if (beforeProgramValidateWasEmitted) {
+                    const wasCancelled = options?.cancellationToken?.isCancellationRequested ?? false;
+                    this.plugins.emit('afterProgramValidate', this, wasCancelled);
+                }
+
                 //regardless of the success of the validation, mark this run as complete
                 deferred.resolve();
                 //clear the validatePromise which means we're no longer running a validation
@@ -1533,3 +1553,10 @@ export interface FileTranspileResult {
     map: SourceMapGenerator;
     typedef: string;
 }
+
+export function test(foo) {
+    const bar = foo + 'Hello';
+
+
+    return bar;
+}

src/interfaces.ts

@@ -198,7 +198,7 @@ export interface CompilerPlugin {
     afterPublish?: (builder: ProgramBuilder, files: FileObj[]) => void;
     afterProgramCreate?: (program: Program) => void;
     beforeProgramValidate?: (program: Program) => void;
-    afterProgramValidate?: (program: Program) => void;
+    afterProgramValidate?: (program: Program, wasCancelled: boolean) => void;
     beforeProgramTranspile?: (program: Program, entries: TranspileObj[], editor: AstEditor) => void;
     afterProgramTranspile?: (program: Program, entries: TranspileObj[], editor: AstEditor) => void;
     beforeProgramDispose?: PluginHandler<BeforeProgramDisposeEvent>;

src/lsp/Project.spec.ts

@@ -54,19 +54,18 @@ describe('Project', () => {
             });
 
             //wait for the first validate to finish
-            const validate = project.validate;
-            await Promise.resolve((resolve) => {
-                const validateStub = sinon.stub(project, 'validate').callsFake((...args) => {
-                    return validate.call(project, ...args).then(() => {
-                        validateStub.restore();
-                        resolve();
-                    });
+            await new Promise<void>((resolve) => {
+                const off = project.on('validate-end', () => {
+                    off();
+                    resolve();
                 });
             });
 
             let validationCount = 0;
             let maxValidationCount = 0;
-            //force validations to yield very frequently
+            //force validation cycles to yield very frequently
+            project['builder'].program['validationMinSyncDuration'] = 0.001;
+
             project['builder'].program.plugins.add({
                 name: 'Test',
                 beforeProgramValidate: () => {

src/lsp/Project.ts

@@ -190,7 +190,7 @@ export class Project implements LspProject {
     }
 
     /**
-     * Cancel any active validation that's running
+     * Cancel any active running validation
      */
     public cancelValidate() {
         this.validationCancelToken?.cancel();