refactor: do all filters in a single loop + add missing test on obsolete files

Author: Simon Renoult

src/io/index.ts

@@ -1,18 +1,40 @@
 import Cli from "./cli";
 import Output from "./output";
-import { Path } from "../lib/types";
+import { Options, Path } from "../lib/types";
 import Statistics from "../lib";
+import { execSync } from "child_process";
+import { existsSync } from "fs";
 
 export default async function main(): Promise<void> {
   const options = await Cli.parse();
 
+  warnIfUsingComplexityWithIncompatibleFileTypes(options);
+  assertGitIsInstalled();
+  assertIsGitRootDirectory(options.directory);
+
+  const statistics: Map<Path, Statistics> = await Statistics.compute(options);
+  Cli.cleanup(options);
+  Output.render(statistics, options);
+}
+
+function warnIfUsingComplexityWithIncompatibleFileTypes(options: Options) {
   if (options.complexityStrategy !== "sloc") {
     console.warn(
       "Beware, the 'halstead' and 'cyclomatic' strategies are only available for JavaScript/TypeScript."
     );
   }
+}
+
+function assertGitIsInstalled(): void {
+  try {
+    execSync("which git");
+  } catch (error) {
+    throw new Error("Program 'git' must be installed");
+  }
+}
 
-  const statistics: Map<Path, Statistics> = await Statistics.compute(options);
-  Cli.cleanup(options);
-  Output.render(statistics, options);
+function assertIsGitRootDirectory(directory: string): void {
+  if (!existsSync(`${directory}/.git`)) {
+    throw new Error(`Argument 'dir' must be the git root directory.`);
+  }
 }

src/lib/churn/churn.ts

@@ -1,12 +1,11 @@
 import { execSync } from "child_process";
-import { existsSync } from "fs";
 import * as micromatch from "micromatch";
-import { resolve } from "path";
 
 import { Options, Path } from "../types";
 import { buildDebugger, withDuration } from "../../utils";
+import { resolve } from "path";
+import { existsSync } from "fs";
 
-type ParsedLine = { relativePath: string; commitCount: string };
 const internal = { debug: buildDebugger("churn") };
 const PER_LINE = "\n";
 
@@ -16,67 +15,19 @@ export default {
 };
 
 async function compute(options: Options): Promise<Map<Path, number>> {
-  assertGitIsInstalled();
-  assertIsGitRootDirectory(options.directory);
-
   const gitLogCommand = buildGitLogCommand(options);
-  const rawStringOfAllChurns = executeGitLogCommand(gitLogCommand);
-  const arrayOfAllChurns = computeChurnsPerFiles(
-    rawStringOfAllChurns,
-    options.directory
-  );
-  const mapOfAllChurns = createMapOfChurnsPerFile(arrayOfAllChurns);
-  return applyUserFilters(mapOfAllChurns, options.filter);
-}
-
-function applyUserFilters(
-  allChurns: Map<Path, number>,
-  filter: string[] | undefined
-): Map<Path, number> {
-  const filteredChurns: Map<Path, number> = new Map();
-  allChurns.forEach((churn: number, path: Path) => {
-    const patchIsAMatch =
-      filter && filter.length > 0
-        ? filter.every((f) => micromatch.isMatch(path, f))
-        : true;
-
-    if (patchIsAMatch) filteredChurns.set(path, churn);
-  });
-  return filteredChurns;
-}
-
-function createMapOfChurnsPerFile(
-  numberOfTimesFilesChanged: ParsedLine[]
-): Map<Path, number> {
-  return numberOfTimesFilesChanged.reduce(
-    (map: Map<Path, number>, { relativePath, commitCount }: ParsedLine) => {
-      const path: Path = relativePath;
-      const churn = parseInt(commitCount, 10);
-      map.set(path, churn);
-      return map;
-    },
-    new Map()
+  const singleStringWithAllChurns = executeGitLogCommand(gitLogCommand);
+  return computeChurnsPerFiles(
+    singleStringWithAllChurns,
+    options.directory,
+    options.filter
   );
 }
 
 function executeGitLogCommand(gitLogCommand: string): string {
   return execSync(gitLogCommand, { encoding: "utf8", maxBuffer: 32_000_000 });
 }
 
-function assertGitIsInstalled(): void {
-  try {
-    execSync("which git");
-  } catch (error) {
-    throw new Error("Program 'git' must be installed");
-  }
-}
-
-function assertIsGitRootDirectory(directory: string): void {
-  if (!existsSync(`${directory}/.git`)) {
-    throw new Error(`Argument 'dir' must be the git root directory.`);
-  }
-}
-
 function buildGitLogCommand(options: Options): string {
   const isWindows = process.platform === "win32";
 
@@ -102,30 +53,42 @@ function buildGitLogCommand(options: Options): string {
 
 function computeChurnsPerFiles(
   gitLogOutput: string,
-  directory: string
-): ParsedLine[] {
+  directory: string,
+  filters: string[] | undefined
+): Map<Path, number> {
   const changedFiles = gitLogOutput
     .split(PER_LINE)
     .filter((line) => line !== "")
     .sort();
 
-  const changedFilesCount = changedFiles.reduce(
-    (fileAndTimeChanged: { [fileName: string]: number }, fileName) => {
-      fileAndTimeChanged[fileName] = (fileAndTimeChanged[fileName] || 0) + 1;
-      return fileAndTimeChanged;
-    },
-    {}
-  );
+  return changedFiles.reduce((map: Map<Path, number>, path) => {
+    applyFiltersAndExcludeObsoletePath(path, map);
+    return map;
+  }, new Map());
+
+  function applyFiltersAndExcludeObsoletePath(
+    path: string,
+    map: Map<Path, number>
+  ) {
+    if (!filters || !filters.length) {
+      if (pathStillExists(path)) {
+        addOrIncrement(map, path);
+      }
+    } else {
+      const pathHasAMatch = filters.every((f) => micromatch.isMatch(path, f));
+      if (pathHasAMatch) {
+        if (pathStillExists(path)) {
+          addOrIncrement(map, path);
+        }
+      }
+    }
+  }
 
-  return Object.keys(changedFilesCount)
-    .map(
-      (changedFileName) =>
-        ({
-          relativePath: changedFileName,
-          commitCount: changedFilesCount[changedFileName].toString(),
-        } as ParsedLine)
-    )
-    .filter((parsedLine: ParsedLine) => {
-      return existsSync(resolve(directory, parsedLine.relativePath));
-    });
+  function addOrIncrement(map: Map<Path, number>, path: string) {
+    map.set(path, (map.get(path) ?? 0) + 1);
+  }
+
+  function pathStillExists(fileName: string) {
+    return existsSync(resolve(directory, fileName));
+  }
 }

test/fixtures/test-repository.fixture.ts

@@ -15,6 +15,7 @@ export default class TestRepositoryFixture {
     commits?: number;
     lines?: number;
     date?: string;
+    removed?: boolean;
   }[] = [];
 
   constructor() {
@@ -27,6 +28,7 @@ export default class TestRepositoryFixture {
     commits?: number;
     lines?: number;
     date?: string;
+    removed?: boolean;
   }): this {
     this.files.push(args);
     return this;
@@ -44,6 +46,7 @@ export default class TestRepositoryFixture {
         .withName(file.name)
         .containing(file.content ?? { lines: file.lines ?? 1 })
         .committed({ times: file.commits ?? 1, date: file.date })
+        .isRemoved(file.removed ?? false)
         .writeOnDisk();
     });
 

test/fixtures/versioned-file.fixture.ts

@@ -10,6 +10,7 @@ export default class VersionedFileFixture {
   private numberOfCommitsForFile = 10;
   private content?: string;
   private commitDate?: string;
+  private removed = false;
 
   withName(name: string): VersionedFileFixture {
     this.name = name;
@@ -31,6 +32,11 @@ export default class VersionedFileFixture {
     return this;
   }
 
+  isRemoved(value: boolean): VersionedFileFixture {
+    this.removed = value;
+    return this;
+  }
+
   writeOnDisk(): void {
     for (let i = 0; i < this.numberOfCommitsForFile; i++) {
       if (i === 0) {
@@ -41,6 +47,10 @@ export default class VersionedFileFixture {
       }
       this.commitFile(i);
     }
+
+    if (this.removed) {
+      this.removeAndCommit();
+    }
   }
 
   private commitFile(commitNumber: number): void {
@@ -58,7 +68,7 @@ export default class VersionedFileFixture {
 
   private modifyFileWithoutChangingItsLength(commitNumber: number): void {
     appendFileSync(
-      `${this.repositoryLocation}${sep}${this.name}`,
+      `${this.getFileLocation()}`,
       `// change for commit #${commitNumber + 1} `
     );
   }
@@ -71,10 +81,28 @@ export default class VersionedFileFixture {
         .map((value, index) => `console.log(${index});`)
         .join("\n");
 
-    writeFileSync(`${this.repositoryLocation}${sep}${this.name}`, fileContent);
+    writeFileSync(`${this.getFileLocation()}`, fileContent);
   }
 
   private addFileToRepository(): void {
     execSync(`git -C ${this.repositoryLocation} add --all`);
   }
+
+  private removeAndCommit() {
+    const message = `"${this.name}: removed"`;
+    const commands = [
+      `git -C ${this.repositoryLocation} rm ${this.getFileLocation()}`,
+      `git -C ${this.repositoryLocation} commit --message=${message}`,
+    ].join("&&");
+    try {
+      execSync(commands);
+    } catch (e: any) {
+      console.log(e.stdout.toString());
+      throw e;
+    }
+  }
+
+  private getFileLocation(): string {
+    return `${this.repositoryLocation}${sep}${this.name}`;
+  }
 }