Fix infinite loop issue with standalone projects

.eslintrc.js

@@ -221,6 +221,7 @@ module.exports = {
             '@typescript-eslint/no-unused-vars-experimental': 'off',
             '@typescript-eslint/dot-notation': 'off',
             '@typescript-eslint/no-unsafe-argument': 'off',
+            'func-names': 'off',
             'new-cap': 'off',
             'no-shadow': 'off',
             'no-void': 'off'

src/LanguageServer.spec.ts

@@ -11,7 +11,7 @@ import { TextDocument } from 'vscode-languageserver-textdocument';
 import type { Program } from './Program';
 import * as assert from 'assert';
 import type { PartialDiagnostic } from './testHelpers.spec';
-import { expectZeroDiagnostics, normalizeDiagnostics, trim } from './testHelpers.spec';
+import { createInactivityStub, expectZeroDiagnostics, normalizeDiagnostics, trim } from './testHelpers.spec';
 import { isBrsFile, isLiteralString } from './astUtils/reflection';
 import { createVisitor, WalkMode } from './astUtils/visitors';
 import { tempDir, rootDir } from './testHelpers.spec';
@@ -23,6 +23,7 @@ import { LogLevel, Logger, createLogger } from './logging';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import { standardizePath } from 'roku-deploy';
 import undent from 'undent';
+import { ProjectManager } from './lsp/ProjectManager';
 
 const sinon = createSandbox();
 
@@ -84,6 +85,8 @@ describe('LanguageServer', () => {
 
     beforeEach(() => {
         sinon.restore();
+        fsExtra.emptyDirSync(tempDir);
+
         server = new LanguageServer();
         server['busyStatusTracker'] = new BusyStatusTracker();
         workspaceFolders = [workspacePath];
@@ -95,9 +98,11 @@ describe('LanguageServer', () => {
         });
         server['hasConfigurationCapability'] = true;
     });
+
     afterEach(() => {
         sinon.restore();
         fsExtra.emptyDirSync(tempDir);
+
         server['dispose']();
         LanguageServer.enableThreadingDefault = enableThreadingDefault;
     });
@@ -123,6 +128,44 @@ describe('LanguageServer', () => {
         }
     }
 
+    it('does not cause infinite loop of project creation', async () => {
+        //add a project with a files array that includes (and then excludes) a file
+        fsExtra.outputFileSync(s`${rootDir}/bsconfig.json`, JSON.stringify({
+            files: ['source/**/*', '!source/**/*.spec.bs']
+        }));
+
+        server['run']();
+
+        function setSyncedDocument(srcPath: string, text: string, version = 1) {
+            //force an open text document
+            const document = TextDocument.create(util.pathToUri(
+                util.standardizePath(srcPath
+                )
+            ), 'brightscript', 1, `sub main()\nend sub`);
+            (server['documents']['_syncedDocuments'] as Map<string, TextDocument>).set(document.uri, document);
+        }
+
+        //wait for the projects to finish loading up
+        await server['syncProjects']();
+
+        //this bug was causing an infinite async loop of new project creations. So monitor the creation of new projects for evaluation later
+        const { stub, promise: createProjectsSettled } = createInactivityStub(ProjectManager.prototype as any, 'constructProject', 400, sinon);
+
+        setSyncedDocument(s`${rootDir}/source/lib1.spec.bs`, 'sub lib1()\nend sub');
+        setSyncedDocument(s`${rootDir}/source/lib2.spec.bs`, 'sub lib2()\nend sub');
+
+        // open a file that is excluded by the project, so it should trigger a standalone project.
+        await server['onTextDocumentDidChangeContent']({
+            document: TextDocument.create(util.pathToUri(s`${rootDir}/source/lib1.spec.bs`), 'brightscript', 1, `sub main()\nend sub`)
+        });
+
+        //wait for the "create projects" deferred debounce to settle
+        await createProjectsSettled;
+
+        //test passes if we've only made 2 new projects (one for each of the standalone projects)
+        expect(stub.callCount).to.eql(2);
+    });
+
     describe('onDidChangeConfiguration', () => {
         async function doTest(startingConfigs: WorkspaceConfigWithExtras[], endingConfigs: WorkspaceConfigWithExtras[]) {
             (server as any)['connection'] = connection;
@@ -637,6 +680,7 @@ describe('LanguageServer', () => {
             expect(
                 filterer.filter([
                     s`${workspacePath}/src/source/file.brs`,
+                    //this file should be excluded
                     s`${workspacePath}/dist/source/file.brs`
                 ])
             ).to.eql([

src/LanguageServer.ts

@@ -135,7 +135,6 @@ export class LanguageServer {
         });
 
         this.projectManager.busyStatusTracker.on('active-runs-change', (event) => {
-            console.log(event);
             this.sendBusyStatus();
         });
     }

src/lsp/PathFilterer.spec.ts

@@ -178,3 +178,47 @@ describe('PathFilterer', () => {
         });
     });
 });
+
+describe('PathCollection', () => {
+    function doTest(globs: string[], filePath: string, expected: boolean) {
+        const collection = new PathCollection({
+            rootDir: rootDir,
+            globs: globs
+        });
+        expect(collection.isMatch(filePath)).to.equal(expected);
+    }
+
+    it('includes a file that matches a single pattern', () => {
+        doTest([
+            '**/*.brs'
+        ], s`${rootDir}/alpha.brs`, true);
+    });
+
+    it('includes a file that matches the 2nd pattern', () => {
+        doTest([
+            '**/*beta*.brs',
+            '**/*alpha*.brs'
+        ], s`${rootDir}/alpha.brs`, true);
+    });
+
+    it('includes a file that is included then excluded then included again', () => {
+        doTest([
+            '**/*.brs',
+            '!**/a*.brs',
+            '**/alpha.brs'
+        ], s`${rootDir}/alpha.brs`, true);
+    });
+
+    it('excludes a file that does not match the pattern', () => {
+        doTest([
+            '**/beta.brs'
+        ], s`${rootDir}/alpha.brs`, false);
+    });
+
+    it('excludes a file that matches first pattern but is excluded from the second pattern', () => {
+        doTest([
+            '**/*.brs',
+            '!**/alpha.brs'
+        ], s`${rootDir}/alpha.brs`, false);
+    });
+});

src/lsp/PathFilterer.ts

@@ -133,7 +133,8 @@ export class PathFilterer {
         this.logger.debug('registerExcludeMatcher', matcher);
 
         const collection = new PathCollection({
-            matcher: matcher
+            matcher: matcher,
+            isExcludePattern: false
         });
         this.excludeCollections.push(collection);
         return () => {
@@ -168,33 +169,55 @@ export class PathCollection {
             globs: string[];
         } | {
             matcher: (path: string) => boolean;
+            isExcludePattern: boolean;
         }
     ) {
         if ('globs' in options) {
             //build matcher patterns from the globs
-            for (const glob of options.globs ?? []) {
+            for (let glob of options.globs ?? []) {
+                let isExcludePattern = glob.startsWith('!');
+                if (isExcludePattern) {
+                    glob = glob.substring(1);
+                }
                 const pattern = path.resolve(
                     options.rootDir,
                     glob
                 ).replace(/\\+/g, '/');
-                this.matchers.push(
-                    micromatch.matcher(pattern)
-                );
+                this.matchers.push({
+                    pattern: pattern,
+                    isMatch: micromatch.matcher(pattern),
+                    isExcludePattern: isExcludePattern
+                });
             }
         } else {
-            this.matchers.push(options.matcher);
+            this.matchers.push({
+                isMatch: options.matcher,
+                isExcludePattern: options.isExcludePattern
+            });
         }
     }
 
-    private matchers: Array<(string) => boolean> = [];
+    private matchers: Array<{
+        pattern?: string;
+        isMatch: (string) => boolean;
+        isExcludePattern: boolean;
+    }> = [];
+
     public isMatch(path: string) {
+        let keep = false;
         //coerce the path into a normalized form and unix slashes
         path = util.standardizePath(path).replace(/\\+/g, '/');
         for (let matcher of this.matchers) {
-            if (matcher(path)) {
-                return true;
+            //exclusion pattern: do not keep the path if it matches
+            if (matcher.isExcludePattern) {
+                if (matcher.isMatch(path)) {
+                    keep = false;
+                }
+                //inclusion pattern: keep the path if it matches
+            } else {
+                keep = keep || matcher.isMatch(path);
             }
         }
-        return false;
+        return keep;
     }
 }

src/lsp/Project.spec.ts

@@ -70,7 +70,7 @@ describe('Project', () => {
         });
     });
 
-    describe('setFile', () => {
+    describe('applyFileChanges', () => {
         it('skips setting the file if the contents have not changed', async () => {
             await project.activate({ projectPath: rootDir } as any);
             //initial set should be true
@@ -100,6 +100,43 @@ describe('Project', () => {
                 }]))[0].status
             ).to.eql('accepted');
         });
+
+        it('always includes a status', async () => {
+            await project.activate({
+                projectPath: rootDir
+            } as any);
+
+            project['builder'].options.files = [
+                'source/**/*',
+                '!source/**/*.spec.bs'
+            ];
+
+            //set file that maches files array
+            expect((await project['applyFileChanges']([{
+                fileContents: '',
+                srcPath: s`${rootDir}/source/main.bs`,
+                type: 'set'
+            }]))[0].status).to.eql('accepted');
+
+            //delete this file that matches a file in the program
+            expect((await project['applyFileChanges']([{
+                srcPath: s`${rootDir}/source/main.bs`,
+                type: 'delete'
+            }]))[0].status).to.eql('accepted');
+
+            //set file that does not match files array files array
+            expect((await project['applyFileChanges']([{
+                fileContents: '',
+                srcPath: s`${rootDir}/source/main.spec.bs`,
+                type: 'set'
+            }]))[0].status).to.eql('rejected');
+
+            //delete directory is "reject" because those should be unraveled into individual files on the outside
+            expect((await project['applyFileChanges']([{
+                srcPath: s`${rootDir}/source`,
+                type: 'delete'
+            }]))[0].status).to.eql('rejected');
+        });
     });
 
     describe('activate', () => {

src/lsp/Project.ts

@@ -227,7 +227,7 @@ export class Project implements LspProject {
             const action = result[i];
             let didChangeThisFile = false;
             //if this is a `set` and the file matches the project's files array, set it
-            if (action.type === 'set' && this.willAcceptFile(action.srcPath)) {
+            if (action.type === 'set' && Project.willAcceptFile(action.srcPath, this.builder.program.options.files, this.builder.program.options.rootDir)) {
                 //load the file contents from disk if we don't have an in memory copy
                 const fileContents = action.fileContents ?? util.readFileSync(action.srcPath)?.toString();
 
@@ -255,6 +255,10 @@ export class Project implements LspProject {
                 didChangeThisFile = this.removeFileOrDirectory(action.srcPath);
                 //if we deleted at least one file, mark this action as accepted
                 action.status = didChangeThisFile ? 'accepted' : 'rejected';
+
+                //we did not handle this action, so reject
+            } else {
+                action.status = 'rejected';
             }
             didChangeFiles = didChangeFiles || didChangeThisFile;
         }
@@ -271,12 +275,12 @@ export class Project implements LspProject {
     /**
      * Determine if this project will accept the file at the specified path (i.e. does it match a pattern in the project's files array)
      */
-    private willAcceptFile(srcPath: string) {
+    public static willAcceptFile(srcPath: string, files: BsConfig['files'], rootDir: string) {
         srcPath = util.standardizePath(srcPath);
-        if (rokuDeploy.getDestPath(srcPath, this.builder.program.options.files, this.builder.program.options.rootDir) !== undefined) {
+        if (rokuDeploy.getDestPath(srcPath, files, rootDir) !== undefined) {
             return true;
             //is this exact path in the `files` array? (this check is mostly for standalone projects)
-        } else if ((this.builder.program.options.files as StandardizedFileEntry[]).find(x => s`${x.src}` === srcPath)) {
+        } else if ((files as StandardizedFileEntry[]).find(x => s`${x.src}` === srcPath)) {
             return true;
         }
         return false;

src/lsp/ProjectManager.spec.ts

@@ -513,7 +513,7 @@ describe('ProjectManager', () => {
             await onNextDiagnostics();
 
             //there should NOT be a standalone project
-            expect(manager['standaloneProjects'].length).to.eql(0);
+            expect(manager['standaloneProjects'].size).to.eql(0);
         });
 
         it('converts a missing file to a delete', async () => {
@@ -740,13 +740,15 @@ describe('ProjectManager', () => {
                 allowStandaloneProject: true
             }]);
             await onNextDiagnostics();
-            expect(manager['standaloneProjects'][0]?.srcPath).to.eql(s`${rootDir}/source/main.brs`);
+            expect(
+                [...manager['standaloneProjects'].values()][0]?.srcPath
+            ).to.eql(s`${rootDir}/source/main.brs`);
 
             //it deletes the standalone project when the file is closed
             await manager.handleFileClose({
                 srcPath: `${rootDir}/source/main.brs`
             });
-            expect(manager['standaloneProjects']).to.be.empty;
+            expect(manager['standaloneProjects'].size).to.eql(0);
         });
     });
 });