Calculate a partial fence difference based on git changes since a pro…

…vided oid (smikula#101) * Add a file provider that avoids ts program walk Adds a faster file provider that discovers source and fence files with a fast asyncronous file walk rather than by relying on typescript to perform a syncronous walk + parse of the program. We still use typescript to parse compilerOptions, but since we no longer rely on the typescript program walk to identify files, we stub out file discovery so only the initial config partse happens. Also adds config options to support switching between the two providers, since it will only work when all source files you intend to check fences against fall under your rootDirs. This is the case for OWA, but I don't know what other consumers look like. Depends on u/mahuangh/faster-source-providers-2 * progress -> progressBar * progress -> progressBar * support definition files, simplify resolveImportsFromFile, exclude asset files in fdir source file provider * getScriptFileExtensions * add partial evaluation based on git checkout * remove FdirSourceFileProvider's dependencies * remove looseRootFileDiscovery * remove extra diff (fenceJobs -> normalizedFiles * add json to the moduleFileExtensions in jest * Added tests for getPartialCheck / getFenceDiff * add launch.json & debug current test * Prefer preProcessFile in getTsImportSetFromSourceString * consider tag diffs during partial evaluation * remove commented code from getTsImportSetFromSourceString * update to new getScriptFIleExtensions from u/mahuangh/faster-source-providers-3 * get diffcount of index properly * update log messages to replace -- with new sentences * remove errant \n in fenceless warnings * Update runner.ts * add new options to README * remove warning when skipping tag existence checks * avoid unecessary branch in runner.ts * null -> undefined in config.ts * remove empty else branch * moving diff utils to utils/diffing * remove leftover cruft in diffing from previous refactors * check partialCheckLimit 'number' not 'boolean' * clarify comment on why we need to evaluate source files with changed imports * move diffing tests to test/utils/diffing, and force a full recheck on tag removal * clarifying comments around fenceDiff * Update warning message: removed tags section -> removed tags Co-authored-by: Maxwell Huang-Hobbs <[email protected]> Co-authored-by: Scott Mikula <[email protected]>
zencargo · Sep 16, 2021 · 7acff60 · 7acff60
1 parent 668999c
commit 7acff60
Show file tree

Hide file tree

Showing 23 changed files with 2,016 additions and 25 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -0,0 +1,30 @@
+{
+    // Use IntelliSense to learn about possible attributes.
+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "SingleTest",
+            "type": "node",
+            "request": "launch",
+            "program": "${workspaceRoot}/node_modules/jest-cli/bin/jest.js",
+            "args": ["--testPathPattern", "${fileBasenameNoExtension}"],
+            "cwd": "${workspaceRoot}",
+            "preLaunchTask": null,
+            "runtimeExecutable": null,
+            "runtimeArgs": ["--nolazy"],
+            "env": {
+                "NODE_ENV": "development"
+            },
+            "sourceMaps": true,
+            "outFiles": ["${workspaceRoot}/src/**"],
+            "console": "integratedTerminal",
+            "skipFiles": [
+                "<node_internals>/**/*.js",
+                "${workspaceRoot}/node_modules/jest-*/**",
+                "${workspaceRoot}/node_modules/jsdom/**"
+            ]
+        }
+    ]
+}
diff --git a/README.md b/README.md
@@ -154,11 +154,77 @@ Default           | CLI                                    | API
 
 ### Root Directory
 
-Specify the project root directory.
+Specify the project root directory or directories.
+These are the folders that will be scanned for fences, and if running with
+`--looseRootFileDiscovery`, the directories that will be scanned for source files.
 
 Default         | CLI                                    | API
 ----------------|----------------------------------------|----
-`process.cwd()` | `--rootDir <string>`<br/>`-r <string>` | `rootDir: string`
+`process.cwd()` | `--rootDir <string...>`<br/>`-r <string...>` | `rootDir: string | string[]`
+
+
+### Ignore External Fences
+
+Ignore external fences (e.g. those in `node_modules`).
+
+Default         | CLI                                    | API
+----------------|----------------------------------------|----
+`false`         | `--ignoreExternalFences`<br/>`-i`      | `ignoreExternalFences: boolean`
+
+
+### Loose Root File Discovery
+
+Discover sources from the root directories rather than discovering sources
+from the project file.
+
+Default         | CLI                                    | API
+----------------|----------------------------------------|----
+`false`         | `--looseRootFileDiscovery`<br/>`-x`    | `looseRootFileDiscovery: boolean`
+
+
+### Since Git Hash
+
+Only run on files changed between the current git index and the given commit hash
+or reference name. If the git index is empty, good-fences will check against the
+current HEAD instead.
+
+Default         | CLI                                         | API
+----------------|---------------------------------------------|----
+`undefined`     | `--sinceGitHash <string>`<br/>`-g <string>` | `sinceGitHash: string`
+
+
+### Partial Check Limit
+
+When running in a partial check (e.g. with `--sinceGitHash`), the maximum number
+of source files to check. If more than this number of files have changed in the
+partial check (including fences and source files), good-fences will exit with
+code 0. This is intended for using good-fences as a pre-commit check.
+
+Default         | CLI                                              | API
+----------------|--------------------------------------------------|----
+`undefined`     | `--partialCheckLimit <number>`<br/>`-l <number>` | `partialCheckLimit: number`
+
+
+### Show Progress Bar
+
+Whether a progress bar should be displayed on the process stderr during fence
+checking. Does not show while discovering files, only while actually running
+fences, so it may take several minutes to show on large projects not running
+with `--looseRootFileDiscovery`.
+
+Default         | CLI                                          | API
+----------------|----------------------------------------------|----
+`false`         | `--progressBar <boolean>`<br/>`-p <boolean>` | `maxConcurrentFenceJobs: boolean`
+
+
+### Max Concurrent Fence Jobs
+
+The maximum number of fence jobs to run at the same time. Should be set below MFILE on your machine, as otherwise good-fences will hit EMFILE and crash out.
+
+Default         | CLI                                                   | API
+----------------|-------------------------------------------------------|----
+`6000`          | `--maxConcurrentFenceJobs <number>`<br/>`-j <number>` | `maxConcurrentFenceJobs: number`
+
 
 ## Return value
 

diff --git a/jest.config.js b/jest.config.js
@@ -1,7 +1,14 @@
 module.exports = {
     testEnvironment: 'node',
     moduleDirectories: ['node_modules'],
-    moduleFileExtensions: ['ts', 'js'],
+    moduleFileExtensions: [
+        'ts',
+        'js',
+        // nodegit parses its own package.json file,
+        // so we need this moduleFileExtension to be
+        // specified for tests.
+        'json',
+    ],
     transform: {
         '.ts': '<rootDir>/jest.preprocessor.js',
     },

diff --git a/package.json b/package.json
@@ -19,6 +19,7 @@
         "commander": "^7.2.0",
         "fdir": "^5.1.0",
         "minimatch": "^3.0.4",
+        "nodegit": "^0.27.0",
         "picomatch": "^2.3.0",
         "tsconfig-paths": "^3.10.1",
         "typescript": "^4.0.3"
@@ -28,6 +29,7 @@
         "@types/commander": "^2.12.2",
         "@types/jest": "^26.0.15",
         "@types/node": "^12.7.8",
+        "@types/nodegit": "^0.27.2",
         "husky": "^4.3.8",
         "jest": "^26.6.0",
         "prettier": "^2.2.1",

diff --git a/src/core/cli.ts b/src/core/cli.ts
@@ -11,6 +11,14 @@ async function main() {
         .version(packageVersion)
         .option('-p, --project <string> ', 'tsconfig.json file')
         .option('-r, --rootDir <string...>', 'root directories of the project')
+        .option(
+            '-g, --sinceGitHash <string>',
+            'Infer files and fences to check based on changes since the specified git hash'
+        )
+        .option(
+            '-l, --partialCheckLimit <number>',
+            'Maximum files to check during a partial check run. If more files than this limit are changed, the partial check will be aborted and good-fences will exit with code 0.'
+        )
         .option(
             '-x, --looseRootFileDiscovery',
             '(UNSTABLE) Check source files under rootDirs instead of instantiating a full typescript program.'

diff --git a/src/core/result.ts b/src/core/result.ts
@@ -54,9 +54,9 @@ export function reportConfigError(message: string, configPath: string) {
 
 export function reportWarning(message: string, configPath?: string) {
     let fencePath = configPath + path.sep + 'fence.json';
-    let detailedMessage = `Good-fences warning: ${message}\n`;
+    let detailedMessage = `Good-fences warning: ${message}`;
     if (configPath) {
-        detailedMessage += `    Fence: ${fencePath}`;
+        detailedMessage += `\n    Fence: ${fencePath}`;
     }
 
     const warning: GoodFencesError = {

diff --git a/src/core/runner.ts b/src/core/runner.ts
@@ -3,12 +3,27 @@ import getOptions, { setOptions } from '../utils/getOptions';
 import validateFile from '../validation/validateFile';
 import TypeScriptProgram from './TypeScriptProgram';
 import normalizePath from '../utils/normalizePath';
-import { getResult } from './result';
+import { getResult, reportWarning } from './result';
 import { validateTagsExist } from '../validation/validateTagsExist';
 import { SourceFileProvider } from './SourceFileProvider';
 import { FDirSourceFileProvider } from './FdirSourceFileProvider';
 import NormalizedPath from '../types/NormalizedPath';
 import { runWithConcurrentLimit } from '../utils/runWithConcurrentLimit';
+import getConfigManager from '../utils/getConfigManager';
+import { getFenceAndImportDiffsFromGit } from '../utils/diffing/getFenceAndImportDiffsFromGit';
+import { getPartialCheckFromImportDiffs } from '../utils/diffing/getPartialCheckFromImportDiffs';
+import * as path from 'path';
+import { PartialCheck } from '../types/PartialCheck';
+
+async function getPartialCheck(): Promise<PartialCheck> {
+    let options = getOptions();
+    if (options.sinceGitHash) {
+        const diffs = await getFenceAndImportDiffsFromGit(options.sinceGitHash);
+        if (diffs) {
+            return getPartialCheckFromImportDiffs(diffs);
+        }
+    }
+}
 
 async function getSourceFilesNormalized(
     sourceFileProvider: SourceFileProvider,
@@ -19,24 +34,64 @@ async function getSourceFilesNormalized(
     return normalizedFiles;
 }
 
+async function getSourceFilesFromPartialCheck(
+    sourceFileProvider: SourceFileProvider,
+    partialCheck: PartialCheck
+) {
+    const fenceScopeFiles = await getSourceFilesNormalized(
+        sourceFileProvider,
+        partialCheck.fences.map((fencePath: NormalizedPath) => path.dirname(fencePath))
+    );
+    return Array.from(new Set([...partialCheck.sourceFiles, ...fenceScopeFiles]));
+}
+
 export async function run(rawOptions: RawOptions) {
     // Store options so they can be globally available
     setOptions(rawOptions);
     let options = getOptions();
 
+    let partialCheck = await getPartialCheck();
+    if (options.partialCheckLimit) {
+        if (!partialCheck) {
+            reportWarning(
+                `Skipping fence validation. Could not calculate a partial check, but partialCheckLimit was specified in options`
+            );
+            return getResult();
+        }
+        if (
+            partialCheck.fences.length + partialCheck.sourceFiles.length >
+            options.partialCheckLimit
+        ) {
+            reportWarning(
+                `Skipping fence validation. The partial check had more than ${options.partialCheckLimit} changes`
+            );
+            return getResult();
+        }
+    }
+
     let sourceFileProvider: SourceFileProvider = options.looseRootFileDiscovery
         ? new FDirSourceFileProvider(options.project, options.rootDir)
         : new TypeScriptProgram(options.project);
 
-    // Do some sanity checks on the fences
-    validateTagsExist();
+    if (!partialCheck) {
+        // Validating tags exist requires a full load of all fences
+        // we can't do this in partial check mode.
+        //
+        // Prefetching the full config set here avoids the overhead
+        // of partial fence loading, since we know we are loading
+        // the full fence set.
+        getConfigManager().getAllConfigs();
+        validateTagsExist();
+    }
 
-    const normalizedFiles = await getSourceFilesNormalized(sourceFileProvider);
+    let normalizedFiles = await (partialCheck
+        ? getSourceFilesFromPartialCheck(sourceFileProvider, partialCheck)
+        : getSourceFilesNormalized(sourceFileProvider));
 
-    // Limit the concurrent executed promises because
-    // otherwise we will open all the files at the same time and
-    // hit the MFILE error (when we hit rlimit)
     await runWithConcurrentLimit(
+        // we have to limit the concurrent executed promises because
+        // otherwise we will open all the files at the same time and
+        // hit the MFILE error (when we hit rlimit)
         options.maxConcurrentFenceJobs,
         normalizedFiles,
         (normalizedFile: NormalizedPath) => validateFile(normalizedFile, sourceFileProvider),

diff --git a/src/types/Options.ts b/src/types/Options.ts
@@ -4,6 +4,8 @@ export default interface Options {
     project: NormalizedPath;
     rootDir: NormalizedPath[];
     ignoreExternalFences: boolean;
+    partialCheckLimit: number;
+    sinceGitHash?: string;
     looseRootFileDiscovery: boolean;
     // Maximum number of fence validation jobs that can
     // be run at the same time.

diff --git a/src/types/PartialCheck.ts b/src/types/PartialCheck.ts
@@ -0,0 +1,6 @@
+import NormalizedPath from './NormalizedPath';
+
+export type PartialCheck = {
+    fences: NormalizedPath[];
+    sourceFiles: NormalizedPath[];
+};
diff --git a/src/types/RawOptions.ts b/src/types/RawOptions.ts
@@ -2,6 +2,8 @@ export default interface RawOptions {
     project?: string;
     rootDir?: string | string[];
     ignoreExternalFences?: boolean;
+    sinceGitHash?: string;
+    partialCheckLimit?: number;
     looseRootFileDiscovery?: boolean;
     maxConcurrentJobs?: number;
     progressBar?: boolean;

diff --git a/src/types/config/Config.ts b/src/types/config/Config.ts
@@ -4,8 +4,8 @@ import ExportRule from './ExportRule';
 
 export default interface Config {
     path: NormalizedPath;
-    tags: string[];
-    exports: ExportRule[];
-    dependencies: DependencyRule[];
-    imports: string[];
+    tags: string[] | undefined;
+    exports: ExportRule[] | undefined;
+    dependencies: DependencyRule[] | undefined;
+    imports: string[] | undefined;
 }