Fix issues with the .gitignore loading for pathFilterer (#1325)

* Fix issues with the .gitignore loading for pathFilterer

* Apply suggestions from code review

---------

Co-authored-by: Christopher Dwyer-Perkins <chris@inverted-solutions.com>

src/LanguageServer.spec.ts

@@ -22,6 +22,7 @@ import type { Project } from './lsp/Project';
 import { LogLevel, Logger, createLogger } from './logging';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import { standardizePath } from 'roku-deploy';
+import undent from 'undent';
 
 const sinon = createSandbox();
 
@@ -578,6 +579,133 @@ describe('LanguageServer', () => {
         });
     });
 
+    describe('rebuildPathFilterer', () => {
+        let workspaceConfigs: WorkspaceConfigWithExtras[] = [];
+        beforeEach(() => {
+            workspaceConfigs = [
+                {
+                    bsconfigPath: undefined,
+                    languageServer: {
+                        enableThreading: true,
+                        logLevel: 'info'
+                    },
+                    workspaceFolder: workspacePath,
+                    excludePatterns: []
+                } as WorkspaceConfigWithExtras
+            ];
+            server['connection'] = connection as any;
+            sinon.stub(server as any, 'getWorkspaceConfigs').callsFake(() => Promise.resolve(workspaceConfigs));
+        });
+
+        it('allows files from dist by default', async () => {
+            const filterer = await server['rebuildPathFilterer']();
+            //certain files are allowed through by default
+            expect(
+                filterer.filter([
+                    s`${rootDir}/manifest`,
+                    s`${rootDir}/dist/file.brs`,
+                    s`${rootDir}/source/file.brs`
+                ])
+            ).to.eql([
+                s`${rootDir}/manifest`,
+                s`${rootDir}/dist/file.brs`,
+                s`${rootDir}/source/file.brs`
+            ]);
+        });
+
+        it('filters out some standard locations by default', async () => {
+            const filterer = await server['rebuildPathFilterer']();
+
+            expect(
+                filterer.filter([
+                    s`${workspacePath}/node_modules/file.brs`,
+                    s`${workspacePath}/.git/file.brs`,
+                    s`${workspacePath}/out/file.brs`,
+                    s`${workspacePath}/.roku-deploy-staging/file.brs`
+                ])
+            ).to.eql([]);
+        });
+
+        it('properly handles a .gitignore list', async () => {
+            fsExtra.outputFileSync(s`${workspacePath}/.gitignore`, undent`
+                dist/
+            `);
+
+            const filterer = await server['rebuildPathFilterer']();
+
+            //filters files that appear in a .gitignore list
+            expect(
+                filterer.filter([
+                    s`${workspacePath}/src/source/file.brs`,
+                    s`${workspacePath}/dist/source/file.brs`
+                ])
+            ).to.eql([
+                s`${workspacePath}/src/source/file.brs`
+            ]);
+        });
+
+        it('does not crash for path outside of workspaceFolder', async () => {
+            fsExtra.outputFileSync(s`${workspacePath}/.gitignore`, undent`
+                dist/
+            `);
+
+            const filterer = await server['rebuildPathFilterer']();
+
+            //filters files that appear in a .gitignore list
+            expect(
+                filterer.filter([
+                    s`${workspacePath}/../flavor1/src/source/file.brs`
+                ])
+            ).to.eql([
+                //since the path is outside the workspace, it does not match the .gitignore patter, and thus is not excluded
+                s`${workspacePath}/../flavor1/src/source/file.brs`
+            ]);
+        });
+
+        it('a gitignore file from any workspace will apply to all workspaces', async () => {
+            workspaceConfigs = [{
+                bsconfigPath: undefined,
+                languageServer: {
+                    enableThreading: true,
+                    logLevel: 'info'
+                },
+                workspaceFolder: s`${tempDir}/flavor1`,
+                excludePatterns: []
+            }, {
+                bsconfigPath: undefined,
+                languageServer: {
+                    enableThreading: true,
+                    logLevel: 'info'
+                },
+                workspaceFolder: s`${tempDir}/flavor2`,
+                excludePatterns: []
+            }] as WorkspaceConfigWithExtras[];
+            fsExtra.outputFileSync(s`${workspaceConfigs[0].workspaceFolder}/.gitignore`, undent`
+                dist/
+            `);
+            fsExtra.outputFileSync(s`${workspaceConfigs[1].workspaceFolder}/.gitignore`, undent`
+                out/
+            `);
+
+            const filterer = await server['rebuildPathFilterer']();
+
+            //filters files that appear in a .gitignore list
+            expect(
+                filterer.filter([
+                    //included files
+                    s`${workspaceConfigs[0].workspaceFolder}/src/source/file.brs`,
+                    s`${workspaceConfigs[1].workspaceFolder}/src/source/file.brs`,
+                    //excluded files
+                    s`${workspaceConfigs[0].workspaceFolder}/dist/source/file.brs`,
+                    s`${workspaceConfigs[1].workspaceFolder}/out/source/file.brs`
+                ])
+            ).to.eql([
+                s`${workspaceConfigs[0].workspaceFolder}/src/source/file.brs`,
+                s`${workspaceConfigs[1].workspaceFolder}/src/source/file.brs`
+            ]);
+        });
+    });
+
     describe('onDidChangeWatchedFiles', () => {
         it('does not trigger revalidates when changes are in files which are not tracked', async () => {
             server.run();

src/LanguageServer.ts

@@ -594,9 +594,9 @@ export class LanguageServer {
      */
     private async rebuildPathFilterer() {
         this.pathFilterer.clear();
-        const workspaceFolders = await this.connection.workspace.getWorkspaceFolders();
-        await Promise.all(workspaceFolders.map(async (workspaceFolder) => {
-            const rootDir = util.uriToPath(workspaceFolder.uri);
+        const workspaceConfigs = await this.getWorkspaceConfigs();
+        await Promise.all(workspaceConfigs.map(async (workspaceConfig) => {
+            const rootDir = util.uriToPath(workspaceConfig.workspaceFolder);
 
             //always exclude everything from these common folders
             this.pathFilterer.registerExcludeList(rootDir, [
@@ -606,17 +606,22 @@ export class LanguageServer {
                 '**/.roku-deploy-staging/**/*'
             ]);
             //get any `files.exclude` patterns from the client from this workspace
-            const workspaceExcludeGlobs = await this.getWorkspaceExcludeGlobs(rootDir);
-            this.pathFilterer.registerExcludeList(rootDir, workspaceExcludeGlobs);
+            this.pathFilterer.registerExcludeList(rootDir, workspaceConfig.excludePatterns);
 
             //get any .gitignore patterns from the client from this workspace
             const gitignorePath = path.resolve(rootDir, '.gitignore');
             if (await fsExtra.pathExists(gitignorePath)) {
-                const matcher = ignore().add(
+                const matcher = ignore({ ignoreCase: true }).add(
                     fsExtra.readFileSync(gitignorePath).toString()
                 );
-                this.pathFilterer.registerExcludeMatcher((path: string) => {
-                    return matcher.test(path).ignored;
+                this.pathFilterer.registerExcludeMatcher((p: string) => {
+                    const relPath = path.relative(rootDir, p);
+                    if (ignore.isPathValid(relPath)) {
+                        return matcher.test(relPath).ignored;
+                    } else {
+                        //we do not have a valid relative path, so we cannot determine if it is ignored...thus it is NOT ignored
+                        return false;
+                    }
                 });
             }
         }));

src/lsp/PathFilterer.spec.ts

@@ -2,12 +2,18 @@ import { PathCollection, PathFilterer } from './PathFilterer';
 import { cwd, rootDir } from '../testHelpers.spec';
 import { expect } from 'chai';
 import { standardizePath as s } from '../util';
+import { createSandbox } from 'sinon';
+const sinon = createSandbox();
 
 describe('PathFilterer', () => {
     let filterer: PathFilterer;
 
     beforeEach(() => {
         filterer = new PathFilterer();
+        sinon.restore();
+    });
+    afterEach(() => {
+        sinon.restore();
     });
 
     it('allows all files through when no filters exist', () => {
@@ -155,4 +161,20 @@ describe('PathFilterer', () => {
         ).to.eql([]);
     });
 
+    describe('registerExcludeMatcher', () => {
+        it('calls the callback function on every path', () => {
+            const spy = sinon.spy();
+            filterer.registerExcludeMatcher(spy);
+            filterer.filter([
+                s`${rootDir}/a.brs`,
+                s`${rootDir}/b.txt`,
+                s`${rootDir}/c.brs`
+            ]);
+            expect(spy.getCalls().map(x => s`${x.args[0]}`)).to.eql([
+                s`${rootDir}/a.brs`,
+                s`${rootDir}/b.txt`,
+                s`${rootDir}/c.brs`
+            ]);
+        });
+    });
 });

src/lsp/PathFilterer.ts

@@ -135,6 +135,7 @@ export class PathFilterer {
         const collection = new PathCollection({
             matcher: matcher
         });
+        this.excludeCollections.push(collection);
         return () => {
             this.removeCollection(collection);
         };