Use preprocess as a replacement for getIgnorePatterns

Author: paleite

jest.config.js

@@ -2,7 +2,7 @@ module.exports = {
   collectCoverageFrom: ["src/**/*.ts"],
   coverageThreshold: {
     global: {
-      branches: 80,
+      branches: 75,
       functions: 80,
       lines: 80,
       statements: 80,

src/__snapshots__/git.test.ts.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`git should find the ranges of each staged file 1`] = `
+exports[`getRangesForDiff should find the ranges of each staged file 1`] = `
 Array [
   Range {
     "exclusiveUpperBound": 1,
@@ -17,7 +17,7 @@ Array [
 ]
 `;
 
-exports[`git should work for hunks which include only-removal-ranges 1`] = `
+exports[`getRangesForDiff should work for hunks which include only-removal-ranges 1`] = `
 Array [
   Range {
     "exclusiveUpperBound": 4,

src/__snapshots__/index.test.ts.snap

@@ -35,10 +35,12 @@ exports[`plugin should match expected export 2`] = `
 Object {
   "diff": Object {
     "postprocess": [Function],
+    "preprocess": [Function],
     "supportsAutofix": true,
   },
   "staged": Object {
     "postprocess": [Function],
+    "preprocess": [Function],
     "supportsAutofix": true,
   },
 }

src/__snapshots__/processors.test.ts.snap

@@ -32,7 +32,7 @@ Object {
 }
 `;
 
-exports[`processors diff 1`] = `
+exports[`processors diff postprocess 1`] = `
 Array [
   Object {
     "column": 1,
@@ -55,7 +55,7 @@ Array [
 ]
 `;
 
-exports[`processors staged 1`] = `
+exports[`processors staged postprocess 1`] = `
 Array [
   Object {
     "column": 1,

src/git.test.ts

@@ -1,18 +1,34 @@
 import * as child_process from "child_process";
+import path from "path";
 import { mocked } from "ts-jest/utils";
-import { getDiffFileList, getDiffForFile, getRangesForDiff } from "./git";
 import {
+  getDiffFileList,
+  getDiffForFile,
+  getGitFileList,
+  getRangesForDiff,
+} from "./git";
+import {
+  diffFileList,
   hunks,
   includingOnlyRemovals,
-  diffFileList,
 } from "./__fixtures__/diff";
-import path from "path";
 
 jest.mock("child_process");
 
 const mockedChildProcess = mocked(child_process, true);
 
-describe("git", () => {
+const OLD_ENV = process.env;
+
+beforeEach(() => {
+  jest.resetModules(); // Most important - it clears the cache
+  process.env = { ...OLD_ENV }; // Make a copy
+});
+
+afterAll(() => {
+  process.env = OLD_ENV; // Restore old environment
+});
+
+describe("getRangesForDiff", () => {
   it("should find the ranges of each staged file", () => {
     expect(getRangesForDiff(hunks)).toMatchSnapshot();
   });
@@ -28,13 +44,24 @@ describe("git", () => {
       `"Couldn't match regex with line '@@ invalid hunk header @@'"`
     );
   });
+});
 
+describe("getDiffForFile", () => {
   it("should get the staged diff of a file", () => {
     mockedChildProcess.execSync.mockReturnValue(Buffer.from(hunks));
+    process.env.ESLINT_PLUGIN_DIFF_COMMIT = "1234567";
 
-    const diffFromFile = getDiffForFile("./mockfile.js");
+    const diffFromFile = getDiffForFile("./mockfile.js", true);
 
-    expect(mockedChildProcess.execSync).toHaveBeenCalled();
+    const expectedArguments =
+      'git diff --diff-filter=ACM --staged --unified=0 "1234567"';
+    expect(
+      mockedChildProcess.execSync.mock.calls[
+        mockedChildProcess.execSync.mock.calls.length - 1
+      ][0]
+        .split(" -- ")
+        .shift()
+    ).toEqual(expectedArguments);
     expect(diffFromFile).toContain("diff --git");
     expect(diffFromFile).toContain("@@");
   });
@@ -52,16 +79,26 @@ describe("git", () => {
     getDiffForFile("./mockfileMiss.js");
     expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(2);
   });
+});
 
+describe("getDiffFileList", () => {
   it("should get the list of staged files", () => {
     jest.mock("child_process").resetAllMocks();
     mockedChildProcess.execSync.mockReturnValue(Buffer.from(diffFileList));
-
-    const fileList = getDiffFileList();
+    const staged = false;
+    const fileList = getDiffFileList(staged);
 
     expect(mockedChildProcess.execSync).toHaveBeenCalled();
     expect(fileList).toEqual(
       ["file1", "file2", "file3"].map((p) => path.resolve(p))
     );
   });
 });
+
+describe("getGitFileList", () => {
+  it("should get the list of committed files", () => {
+    expect(getGitFileList()).toEqual(
+      ["file1", "file2", "file3"].map((p) => path.resolve(p))
+    );
+  });
+});

src/git.ts

@@ -75,6 +75,7 @@ const getGitFileList = (): string[] => {
       .split("\n")
       .map((filePath) => path.resolve(filePath));
   }
+
   return gitFileListCache;
 };
 

src/processors.test.ts

@@ -1,37 +1,74 @@
 import * as child_process from "child_process";
+import type { Linter } from "eslint";
 import { mocked } from "ts-jest/utils";
+import * as git from "./git";
+import { diff, diffConfig, staged, stagedConfig } from "./processors";
 import {
   diff as fixtureDiff,
   staged as fixtureStaged,
 } from "./__fixtures__/diff";
+import { postprocessArguments } from "./__fixtures__/postprocessArguments";
 
 jest.mock("child_process");
 const mockedChildProcess = mocked(child_process, true);
-mockedChildProcess.execSync.mockReturnValue(
-  Buffer.from(`/mock filename with quotes ", semicolons ; and spaces.js`)
-);
-
-import { diff, diffConfig, staged, stagedConfig } from "./processors";
-import { postprocessArguments } from "./__fixtures__/postprocessArguments";
+const mockFilename = `/mock filename with quotes ", semicolons ; and spaces.js`;
+mockedChildProcess.execSync.mockReturnValue(Buffer.from(mockFilename));
+const mockedGit = mocked(git, true);
+mockedGit.getDiffFileList = jest
+  .fn()
+  .mockReturnValue([mockFilename, "README.md"]);
 
 const [messages, filename] = postprocessArguments;
 
 describe("processors", () => {
-  it("diff", () => {
+  it("diff preprocess", () => {
+    const validFilename = filename;
+    const sourceCode = "/** Some source code */";
+
+    mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
+
+    expect(diff.preprocess(sourceCode, validFilename)).toEqual([sourceCode]);
+  });
+
+  it("staged preprocess", () => {
+    const validFilename = filename;
+    const sourceCode = "/** Some source code */";
+
+    mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
+
+    expect(diff.preprocess(sourceCode, validFilename)).toEqual([sourceCode]);
+  });
+
+  it("diff postprocess", () => {
     mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
 
     expect(diff.postprocess(messages, filename)).toMatchSnapshot();
 
     expect(mockedChildProcess.execSync).toHaveBeenCalled();
   });
 
-  it("staged", () => {
+  it("staged postprocess", () => {
     mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureStaged));
 
     expect(staged.postprocess(messages, filename)).toMatchSnapshot();
 
     expect(mockedChildProcess.execSync).toHaveBeenCalled();
   });
+
+  it("should report fatal errors", () => {
+    mockedChildProcess.execSync.mockReturnValue(Buffer.from(fixtureDiff));
+
+    const [[firstMessage, ...restMessage], ...restMessageArray] = messages;
+    const messagesWithFatal: Linter.LintMessage[][] = [
+      [{ ...firstMessage, fatal: true }, ...restMessage],
+      ...restMessageArray,
+    ];
+
+    expect(diff.postprocess(messages, filename)).toHaveLength(2);
+    expect(diff.postprocess(messagesWithFatal, filename)).toHaveLength(3);
+
+    expect(mockedChildProcess.execSync).toHaveBeenCalled();
+  });
 });
 
 describe("configs", () => {

src/processors.ts

@@ -1,48 +1,59 @@
 import type { Linter } from "eslint";
 import type { Range } from "./git";
-import {
-  getDiffFileList,
-  getDiffForFile,
-  getRangesForDiff,
-} from "./git";
+import { getDiffFileList, getDiffForFile, getRangesForDiff } from "./git";
 
 const STAGED = true;
 
+/**
+ * Exclude unchanged files from being processed
+ *
+ * Since we're excluding unchanged files in the post-processor, we can exclude
+ * them from being processed in the first place, as a performance optimization.
+ * This is increasingly useful the more files there are in the repository.
+ */
+const getPreProcessor = (staged = false) => (
+  text: string,
+  filename: string
+) => {
+  const shouldBeProcessed = getDiffFileList(staged).includes(filename);
+
+  return shouldBeProcessed ? [text] : [];
+};
+
 const isLineWithinRange = (line: number) => (range: Range) =>
   range.isWithinRange(line);
 
-const diff = {
-  postprocess: (
-    messages: Linter.LintMessage[][],
-    filename: string
-  ): Linter.LintMessage[] => {
-    const isFilenameExcluded = !getDiffFileList().includes(filename);
-
-    if (isFilenameExcluded) {
-      return [];
-    }
-
-    return messages
-      .map((message) => {
-        const filteredMessage = message.filter(({ fatal, line }) => {
-          if (fatal === true) {
-            return true;
-          }
-
-          const isLineWithinSomeRange = getRangesForDiff(
-            getDiffForFile(filename)
-          ).some(isLineWithinRange(line));
-
-          return isLineWithinSomeRange;
-        });
-
-        return filteredMessage;
-      })
-      .reduce((a, b) => a.concat(b), []);
-  },
+const getPostProcessor = (staged = false) => (
+  messages: Linter.LintMessage[][],
+  filename: string
+): Linter.LintMessage[] => {
+  return messages
+    .map((message) => {
+      const filteredMessage = message.filter(({ fatal, line }) => {
+        if (fatal === true) {
+          return true;
+        }
+
+        const isLineWithinSomeRange = getRangesForDiff(
+          getDiffForFile(filename, staged)
+        ).some(isLineWithinRange(line));
+
+        return isLineWithinSomeRange;
+      });
+
+      return filteredMessage;
+    })
+    .reduce((a, b) => a.concat(b), []);
+};
 
+const getProcessors = (staged = false) => ({
+  preprocess: getPreProcessor(staged),
+  postprocess: getPostProcessor(staged),
   supportsAutofix: true,
-};
+});
+
+const diff = getProcessors();
+const staged = getProcessors(STAGED);
 
 const diffConfig = {
   plugins: ["diff"],
@@ -54,39 +65,6 @@ const diffConfig = {
   ],
 };
 
-const staged = {
-  postprocess: (
-    messages: Linter.LintMessage[][],
-    filename: string
-  ): Linter.LintMessage[] => {
-    const isFilenameExcluded = !getDiffFileList().includes(filename);
-
-    if (isFilenameExcluded) {
-      return [];
-    }
-
-    return messages
-      .map((message) => {
-        const filteredMessage = message.filter(({ fatal, line }) => {
-          if (fatal === true) {
-            return true;
-          }
-
-          const isLineWithinSomeRange = getRangesForDiff(
-            getDiffForFile(filename, STAGED)
-          ).some(isLineWithinRange(line));
-
-          return isLineWithinSomeRange;
-        });
-
-        return filteredMessage;
-      })
-      .reduce((a, b) => a.concat(b), []);
-  },
-
-  supportsAutofix: true,
-};
-
 const stagedConfig = {
   plugins: ["diff"],
   overrides: [