fix: Fix crash on no violations after filtering by ratchet (#46)

__tests__/main.test.ts

@@ -67,6 +67,46 @@ test('runSorald can ratchet from HEAD~', async () => {
   ]);
 });
 
+test('runSorald with ratchet does nothing when there are no violations in changed code', async () => {
+  // arrange
+  const workdir = await helpers.createTempdir();
+  const repo = await git.init(workdir);
+
+  // create a commit with a single violation each of rules 2164 (math on float) and 1854 (dead store)
+  const className = 'Main';
+  const filePath = path.join(workdir, `${className}.java`);
+  const baseMethodBody = `float a = 2;
+  float b = 2; // 1854: dead store
+  b = 3;
+  float c = a / b; // 2164: math on float
+  System.out.println(c);`;
+  const initialClassContent = wrapInClassInMainMethod(
+    className,
+    baseMethodBody
+  );
+  helpers.createFile(filePath, initialClassContent);
+  await repo.add(filePath);
+  await repo.commit('Initial commit');
+
+  // create a new commit with no violations
+  const secondCommitMethodBody = `${baseMethodBody}
+  b = c;
+  System.out.println(b);`;
+  const secondCommitClassContent = wrapInClassInMainMethod(
+    className,
+    secondCommitMethodBody
+  );
+  await helpers.createFile(filePath, secondCommitClassContent);
+  await repo.add(filePath);
+  await repo.commit('Update file');
+
+  const soraldJarUrl =
+    'https://github.com/SpoonLabs/sorald/releases/download/sorald-0.1.0/sorald-0.1.0-jar-with-dependencies.jar';
+  const repairs = main.runSorald(workdir, soraldJarUrl, 'HEAD~');
+
+  await expect(repairs).resolves.toHaveLength(0);
+});
+
 test('runSorald correctly repairs existing violations', async () => {
   // Test that buildbreaker can ratchet from HEAD~. The only reason we use
   // HEAD~ is that it's very easy to setup the test.

src/main.ts

@@ -38,35 +38,23 @@ export async function runSorald(
   await download(soraldJarUrl, jarDstPath);
 
   core.info(`Mining rule violations at ${source}`);
-  const keyToSpecs: Map<number, string[]> = await sorald.mine(
+  const unfilteredKeyToSpecs: Map<number, string[]> = await sorald.mine(
     jarDstPath,
     source,
     'stats.json'
   );
-
-  const changedLines =
-    ratchetFrom === undefined
-      ? undefined
-      : await (async () => {
-          const diff = await repo.diff(ratchetFrom);
-          const worktreeRoot = await repo.getWorktreeRoot();
-          return git.parseChangedLines(diff, worktreeRoot);
-        })();
+  const keyToSpecs = await filterKeyToSpecsByRatchet(
+    unfilteredKeyToSpecs,
+    source,
+    repo,
+    ratchetFrom
+  );
 
   let allRepairs: string[] = [];
   if (keyToSpecs.size > 0) {
     core.info('Found rule violations');
     core.info('Attempting repairs');
-    for (const [ruleKey, unfilteredSpecs] of keyToSpecs.entries()) {
-      const violationSpecs =
-        changedLines === undefined
-          ? unfilteredSpecs
-          : filterViolationSpecsByRatchet(
-              unfilteredSpecs,
-              changedLines,
-              source
-            );
-
+    for (const [ruleKey, violationSpecs] of keyToSpecs.entries()) {
       core.info(`Repairing violations of rule ${ruleKey}: ${violationSpecs}`);
       const statsFile = path.join(source.toString(), `${ruleKey}.json`);
       const repairs = await sorald.repair(
@@ -84,6 +72,43 @@ export async function runSorald(
   return allRepairs;
 }
 
+/**
+ * Filter the violation specs such that only those present in changed code are
+ * retained, and return a new map where only keys with at least one violation
+ * spec is present after filtering.
+ *
+ * If ratchetFrom is undefined, just return a copy of the keyToSpecs map.
+ */
+async function filterKeyToSpecsByRatchet(
+  keyToSpecs: Map<number, string[]>,
+  source: PathLike,
+  repo: git.Repo,
+  ratchetFrom: string | undefined
+): Promise<Map<number, string[]>> {
+  if (ratchetFrom === undefined) {
+    return new Map(keyToSpecs);
+  }
+
+  const filteredKeyToSpecs: Map<number, string[]> = new Map();
+
+  const diff = await repo.diff(ratchetFrom);
+  const worktreeRoot = await repo.getWorktreeRoot();
+  const changedLines = git.parseChangedLines(diff, worktreeRoot);
+
+  for (const [ruleKey, unfilteredSpecs] of keyToSpecs.entries()) {
+    const filteredSpecs =
+      changedLines === undefined
+        ? unfilteredSpecs
+        : filterViolationSpecsByRatchet(unfilteredSpecs, changedLines, source);
+
+    if (filteredSpecs.length > 0) {
+      filteredKeyToSpecs.set(ruleKey, filteredSpecs);
+    }
+  }
+
+  return filteredKeyToSpecs;
+}
+
 /**
  * Filter out any violation specifiers that aren't present in the changed lines
  * of code.