Fix tests and refactor

Author: paleite

src/__fixtures__/diff.ts

@@ -51,4 +51,6 @@ b/dirty.js
 `;
 
 export const filenamesA = `a/dirty.js
-`
+`
+
+export const diffFileList = "file1\nfile2\nfile3\n"

src/__fixtures__/postprocessArguments.ts

@@ -1,4 +1,4 @@
-import { Linter } from "eslint";
+import type { Linter } from "eslint";
 
 const postprocessArguments = [
   [

src/git.test.ts

@@ -1,7 +1,11 @@
 import * as child_process from "child_process";
 import { mocked } from "ts-jest/utils";
 import { getDiffFileList, getDiffForFile, getRangesForDiff } from "./git";
-import { hunks, includingOnlyRemovals, filenamesAB } from "./__fixtures__/diff";
+import {
+  hunks,
+  includingOnlyRemovals,
+  diffFileList,
+} from "./__fixtures__/diff";
 import path from "path";
 
 jest.mock("child_process");
@@ -29,25 +33,27 @@ describe("git", () => {
 
   it("should hit the cached diff of a file", () => {
     jest.mock("child_process").resetAllMocks();
-    mockedChildProcess.execSync.mockReturnValue(Buffer.from(hunks));
+    mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(hunks));
 
     const diffFromFileA = getDiffForFile("./mockfileCache.js");
     const diffFromFileB = getDiffForFile("./mockfileCache.js");
     expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(1);
     expect(diffFromFileA).toEqual(diffFromFileB);
 
+    mockedChildProcess.execSync.mockReturnValueOnce(Buffer.from(hunks));
     getDiffForFile("./mockfileMiss.js");
     expect(mockedChildProcess.execSync).toHaveBeenCalledTimes(2);
   });
 
   it("should get the list of staged files", () => {
-    mockedChildProcess.execSync.mockReturnValue(Buffer.from(filenamesAB));
+    jest.mock("child_process").resetAllMocks();
+    mockedChildProcess.execSync.mockReturnValue(Buffer.from(diffFileList));
 
     const fileList = getDiffFileList();
 
     expect(mockedChildProcess.execSync).toHaveBeenCalled();
     expect(fileList).toEqual(
-      ["a/dirty.js", "b/dirty.js"].map((p) => path.resolve(p))
+      ["file1", "file2", "file3"].map((p) => path.resolve(p))
     );
   });
 });

src/git.ts

@@ -8,37 +8,45 @@ const sanitizeFilePath = (filePath: string) =>
 const diffCacheKey = (filePath: string, staged: boolean): string =>
   JSON.stringify([path.resolve(filePath), staged]);
 
-const diffCache = new Map<string, string>();
+const setCachedDiff = (filePath: string, staged: boolean, diff: string): void =>
+  void diffCache.set(diffCacheKey(filePath, staged), diff);
+
+const getCachedDiff = (filePath: string, staged: boolean) =>
+  diffCache.get(diffCacheKey(filePath, staged));
 
+const diffCache = new Map<string, string>();
 const getDiffForFile = (filePath: string, staged = false): string => {
-  // Get diff entry from cache
-  const cachedDiff = diffCache.get(diffCacheKey(filePath, staged));
-  if (cachedDiff) {
-    // If entry is not falsy return it
-    return cachedDiff;
-  } else {
-    // Otherwise spawn child_process set it in cache and return it
-    const diff = child_process
+  let diff = getCachedDiff(filePath, staged);
+  if (diff === undefined) {
+    const result = child_process
       .execSync(
         `git diff --diff-filter=ACM --unified=0 HEAD ${
-          staged ? "--staged" : ""
+          staged ? " --staged" : ""
         } -- ${sanitizeFilePath(filePath)}`
       )
       .toString();
-    diffCache.set(diffCacheKey(filePath, staged), diff);
-    return diff;
+    setCachedDiff(filePath, staged, result);
+    diff = result;
   }
+
+  return diff;
 };
 
+let diffFileListCache: string[];
 const getDiffFileList = (staged = false): string[] => {
-  return child_process
-    .execSync(
-      `git diff --diff-filter=ACM HEAD ${staged ? "--staged" : ""} --name-only`
-    )
-    .toString()
-    .split("\n")
-    .filter((filePath) => filePath.length)
-    .map((filePath) => path.resolve(filePath));
+  if (diffFileListCache === undefined) {
+    diffFileListCache = child_process
+      .execSync(
+        `git diff --diff-filter=ACM HEAD --name-only ${
+          staged ? "--staged" : ""
+        }`
+      )
+      .toString()
+      .trim()
+      .split("\n")
+      .map((filePath) => path.resolve(filePath));
+  }
+  return diffFileListCache;
 };
 
 const isHunkHeader = (input: string) => {
@@ -63,12 +71,12 @@ const getRangeForChangedLines = (line: string) => {
   }
 
   const linesCount: number =
-    range.groups?.linesCountDelimiter && range.groups?.linesCount
+    range.groups.linesCountDelimiter && range.groups.linesCount
       ? parseInt(range.groups.linesCount)
       : 1;
 
   const hasAddedLines = linesCount !== 0;
-  const start: number = parseInt(range.groups?.start);
+  const start: number = parseInt(range.groups.start);
   const end = start + linesCount;
 
   return hasAddedLines ? new Range(start, end) : null;

src/processors.ts

@@ -1,40 +1,28 @@
-import {
-  getDiffForFile,
-  getRangesForDiff,
-  getDiffFileList,
-  Range,
-} from "./git";
-import { Linter } from "eslint";
+import type { Range } from "./git";
+import { getDiffForFile, getRangesForDiff, getDiffFileList } from "./git";
+import type { Linter } from "eslint";
 
 const STAGED = true;
 
 const isLineWithinRange = (line: number) => (range: Range) =>
   range.isWithinRange(line);
-let fileList: string[];
 
 const diff = {
   preprocess: (
     text: string,
     filename: string
-  ): { text: string; filename: string }[] => {
-    if (
-      (fileList ? fileList : (fileList = getDiffFileList())).includes(filename)
-    ) {
-      return [{ text, filename }];
-    } else {
-      return [];
-    }
-  },
+  ): { text: string; filename: string }[] =>
+    getDiffFileList().includes(filename) ? [{ text, filename }] : [],
 
   postprocess: (
     messages: Linter.LintMessage[][],
     filename: string
   ): Linter.LintMessage[] =>
     messages
       .map((message) =>
-        message.filter((message) =>
+        message.filter(({ line }) =>
           getRangesForDiff(getDiffForFile(filename)).some(
-            isLineWithinRange(message.line)
+            isLineWithinRange(line)
           )
         )
       )
@@ -57,26 +45,18 @@ const staged = {
   preprocess: (
     text: string,
     filename: string
-  ): { text: string; filename: string }[] => {
-    if (
-      (fileList ? fileList : (fileList = getDiffFileList(true))).includes(
-        filename
-      )
-    ) {
-      return [{ text, filename }];
-    } else {
-      return [];
-    }
-  },
+  ): { text: string; filename: string }[] =>
+    getDiffFileList(STAGED).includes(filename) ? [{ text, filename }] : [],
+
   postprocess: (
     messages: Linter.LintMessage[][],
     filename: string
   ): Linter.LintMessage[] =>
     messages
       .map((message) =>
-        message.filter((message) =>
+        message.filter(({ line }) =>
           getRangesForDiff(getDiffForFile(filename, STAGED)).some(
-            isLineWithinRange(message.line)
+            isLineWithinRange(line)
           )
         )
       )