Respect ignore patterns in FDirSourceFileProvider (#103)

At present, FDirSourceFileProvider does not respect ignore patterns from the input tsconfig.

Because of this, we run into issues in owa where, if you build packages in OWA then run good-fences with `-x`, you will get fence errors about non-exported members under `lib`.

This change makes FDirSourceFileProvider aware of the ignore patterns in the passed in tsconfig (not recursively, however). As part of this change, I also

- add an explicit dependency on @types/picomatch so we can check in our own code
- turns on esModuleInterop so we can import picomatch directly
- removes deprecated types from @types/commander, since commander ships with types now.
- Updates the integration test to handle ignore patterns (in a very artificial way)
- Updates the integration test to test the FDir provider, and normalize result orders (since result ordering is nondeterministic in the fdir provider).
- Adds a way to clear results so you can run good-fences multiple times in the same process without accumulating errors.

package.json

@@ -26,10 +26,10 @@
     },
     "devDependencies": {
         "@types/cli-progress": "^3.9.2",
-        "@types/commander": "^2.12.2",
         "@types/jest": "^26.0.15",
         "@types/node": "^12.7.8",
         "@types/nodegit": "^0.27.2",
+        "@types/picomatch": "^2.3.0",
         "husky": "^4.3.8",
         "jest": "^26.6.0",
         "prettier": "^2.2.1",

sample/src/componentB/ignored/ignoredFileWithIllegalImports.ts

@@ -0,0 +1,5 @@
+import helperA2 from '../../componentA/helperA2';
+
+export default function ignoredComponent() {
+    helperA2();
+}

sample/tsconfig.json

@@ -8,6 +8,6 @@
         "noImplicitAny": false,
         "noUnusedLocals": true
     },
-    "exclude": ["lib"],
+    "exclude": ["lib", "src/**/ignored/**/*.ts"],
     "include": ["src/**/*"]
 }

src/core/FdirSourceFileProvider.ts

@@ -14,9 +14,11 @@ import {
     ParsedCommandLine,
     preProcessFile,
 } from 'typescript';
+import picomatch from 'picomatch';
 
 export class FDirSourceFileProvider implements SourceFileProvider {
     parsedCommandLine: ParsedCommandLine;
+    excludePatternsPicoMatchers: picomatch.Matcher[];
     matchPath: MatchPathAsync;
     private sourceFileGlob: string;
     private extensionsToCheckDuringImportResolution: string[];
@@ -50,6 +52,16 @@ export class FDirSourceFileProvider implements SourceFileProvider {
             }
         );
 
+        const baseUrl = this.parsedCommandLine.options.baseUrl ?? path.dirname(configFileName);
+        this.excludePatternsPicoMatchers = (this.parsedCommandLine.raw?.exclude ?? []).map(
+            (excludePattern: string) => {
+                const matcher = picomatch(excludePattern);
+                return (pathToCheck: string) => {
+                    return matcher(path.relative(baseUrl, pathToCheck));
+                };
+            }
+        );
+
         this.sourceFileGlob = `**/*@(${getScriptFileExtensions({
             // Derive these settings from the typescript project itself
             allowJs: this.parsedCommandLine.options.allowJs || false,
@@ -77,10 +89,7 @@ export class FDirSourceFileProvider implements SourceFileProvider {
             includeDefinitions: true,
         });
 
-        this.matchPath = createMatchPathAsync(
-            this.parsedCommandLine.options.baseUrl,
-            this.parsedCommandLine.options.paths
-        );
+        this.matchPath = createMatchPathAsync(baseUrl, this.parsedCommandLine.options.paths ?? {});
     }
 
     async getSourceFiles(searchRoots?: string[]): Promise<string[]> {
@@ -95,7 +104,13 @@ export class FDirSourceFileProvider implements SourceFileProvider {
             )
         );
 
-        return [...new Set<string>(allRootsDiscoveredFiles.reduce((a, b) => a.concat(b), []))];
+        return [
+            ...new Set<string>(allRootsDiscoveredFiles.reduce((a, b) => a.concat(b), [])),
+        ].filter((p: string) => !this.isPathExcluded(p));
+    }
+
+    private isPathExcluded(path: string) {
+        return this.excludePatternsPicoMatchers.some(isMatch => isMatch(path));
     }
 
     async getImportsForFile(filePath: string): Promise<string[]> {

src/core/cli.ts

@@ -7,7 +7,7 @@ async function main() {
     const packageVersion = require('../../package.json').version;
 
     // Parse command line options
-    const program = commander
+    const program = commander.program
         .version(packageVersion)
         .option('-p, --project <string> ', 'tsconfig.json file')
         .option('-r, --rootDir <string...>', 'root directories of the project')

src/core/result.ts

@@ -4,11 +4,18 @@ import GoodFencesError from '../types/GoodFencesError';
 import GoodFencesResult from '../types/GoodFencesResult';
 import ImportRecord from './ImportRecord';
 
-const result: GoodFencesResult = {
+let result: GoodFencesResult = {
     errors: [],
     warnings: [],
 };
 
+export function resetResult() {
+    result = {
+        errors: [],
+        warnings: [],
+    };
+}
+
 export function getResult() {
     return result;
 }

src/core/runner.ts

@@ -3,7 +3,7 @@ import getOptions, { setOptions } from '../utils/getOptions';
 import validateFile from '../validation/validateFile';
 import TypeScriptProgram from './TypeScriptProgram';
 import normalizePath from '../utils/normalizePath';
-import { getResult, reportWarning } from './result';
+import { getResult, reportWarning, resetResult } from './result';
 import { validateTagsExist } from '../validation/validateTagsExist';
 import { SourceFileProvider } from './SourceFileProvider';
 import { FDirSourceFileProvider } from './FdirSourceFileProvider';
@@ -98,5 +98,9 @@ export async function run(rawOptions: RawOptions) {
         options.progress
     );
 
-    return getResult();
+    const result = getResult();
+    // Reset the global results object so so that future runs
+    // do not have the results from this run.
+    resetResult();
+    return result;
 }

test/endToEnd/endToEndTests.ts

@@ -1,4 +1,5 @@
 import { run } from '../../src/core/runner';
+import GoodFencesError from '../../src/types/GoodFencesError';
 import GoodFencesResult from '../../src/types/GoodFencesResult';
 import normalizePath from '../../src/utils/normalizePath';
 
@@ -15,6 +16,26 @@ describe('runner', () => {
         // Assert
         removeDetailedMessages(actualResults);
         normalizePaths(expectedResults);
+        normalizeOrder(actualResults);
+        normalizeOrder(expectedResults);
+        expect(actualResults).toEqual(expectedResults);
+    });
+
+    it('returns the expected results with looseRootFileDiscovery', async () => {
+        // Arrange
+        const expectedResults = require('./endToEndTests.expected.json');
+
+        // Act
+        const actualResults = await run({
+            rootDir: './sample',
+            looseRootFileDiscovery: true,
+        });
+
+        // Assert
+        removeDetailedMessages(actualResults);
+        normalizePaths(expectedResults);
+        normalizeOrder(actualResults);
+        normalizeOrder(expectedResults);
         expect(actualResults).toEqual(expectedResults);
     });
 });
@@ -29,6 +50,15 @@ function removeDetailedMessages(results: GoodFencesResult) {
     }
 }
 
+function normalizeOrder(results: GoodFencesResult) {
+    results.errors.sort((a: GoodFencesError, b: GoodFencesError): number =>
+        a.sourceFile.localeCompare(b.sourceFile)
+    );
+    results.warnings.sort((a: GoodFencesError, b: GoodFencesError): number =>
+        a.sourceFile.localeCompare(b.sourceFile)
+    );
+}
+
 function normalizePaths(results: GoodFencesResult) {
     for (const error of results.errors) {
         error.fencePath = normalizePath(error.fencePath);

test/utils/__mocks__/fs.ts

@@ -0,0 +1,12 @@
+// We mock the fs module here because in newer version of node,
+// the builtin module epxorts are non-writeable. This means that
+// spyOn(fs, 'readFileSync') will error in beforeEach and no mocks
+// will actually be set.
+//
+// By providing a mock module here and calling jest.mock('fs')
+// before any imports, we replace any imports of fs with
+// this module, which has mutable exports.
+
+export function readFileSync() {
+    throw new Error('readFileSync mock was not overridden');
+}

test/utils/loadConfigTests.ts

@@ -1,3 +1,4 @@
+jest.mock('fs');
 import * as fs from 'fs';
 import RawConfig from '../../src/types/rawConfig/RawConfig';
 import loadConfig, { normalizeExportRules } from '../../src/utils/loadConfig';

tsconfig.json

@@ -8,6 +8,7 @@
         "noImplicitAny": true,
         "noUnusedLocals": true,
         "noImplicitThis": true,
+        "esModuleInterop": true,
         "skipLibCheck": true
     },
     "exclude": ["node_modules", "lib"],

yarn.lock

@@ -664,13 +664,6 @@
   dependencies:
     "@types/node" "*"
 
-"@types/commander@^2.12.2":
-  version "2.12.2"
-  resolved "https://registry.yarnpkg.com/@types/commander/-/commander-2.12.2.tgz#183041a23842d4281478fa5d23c5ca78e6fd08ae"
-  integrity sha512-0QEFiR8ljcHp9bAbWxecjVRuAMr16ivPiGOw6KFQBVrVd0RQIcM3xKdRisH2EDWgVWujiYtHwhSkSUoAAGzH7Q==
-  dependencies:
-    commander "*"
-
 "@types/graceful-fs@^4.1.2":
   version "4.1.3"
   resolved "https://registry.yarnpkg.com/@types/graceful-fs/-/graceful-fs-4.1.3.tgz#039af35fe26bec35003e8d86d2ee9c586354348f"
@@ -754,6 +747,11 @@
   resolved "https://registry.yarnpkg.com/@types/parse-json/-/parse-json-4.0.0.tgz#2f8bb441434d163b35fb8ffdccd7138927ffb8c0"
   integrity sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA==
 
+"@types/picomatch@^2.3.0":
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/@types/picomatch/-/picomatch-2.3.0.tgz#75db5e75a713c5a83d5b76780c3da84a82806003"
+  integrity sha512-O397rnSS9iQI4OirieAtsDqvCj4+3eY1J+EPdNTKuHuRWIfUoGyzX294o8C4KJYaLqgSrd2o60c5EqCU8Zv02g==
+
 "@types/prettier@^2.0.0":
   version "2.1.5"
   resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.1.5.tgz#b6ab3bba29e16b821d84e09ecfaded462b816b00"
@@ -1318,7 +1316,7 @@ combined-stream@^1.0.6, combined-stream@~1.0.6:
   dependencies:
     delayed-stream "~1.0.0"
 
-commander@*, commander@^7.2.0:
+commander@^7.2.0:
   version "7.2.0"
   resolved "https://registry.yarnpkg.com/commander/-/commander-7.2.0.tgz#a36cb57d0b501ce108e4d20559a150a391d97ab7"
   integrity sha512-QrWXB+ZQSVPmIWIhtEO9H+gwHaMGYiF5ChvoJ+K9ZGHG/sVsa6yiesAD1GC/x46sET00Xlwo1u49RVVVzvcSkw==