refactor: single source of truth for getDiffRangesJsonFilePath and simplified getDiffRangeFilePaths

src/actions-util.ts

@@ -53,6 +53,12 @@ export function getTemporaryDirectory(): string {
     : getRequiredEnvParam("RUNNER_TEMP");
 }
 
+const PR_DIFF_RANGE_JSON_FILENAME = "pr-diff-range.json";
+
+export function getDiffRangesJsonFilePath(): string {
+  return path.join(getTemporaryDirectory(), PR_DIFF_RANGE_JSON_FILENAME);
+}
+
 export function getActionVersion(): string {
   return __CODEQL_ACTION_VERSION__;
 }

src/diff-informed-analysis-utils.ts

@@ -1,5 +1,4 @@
 import * as fs from "fs";
-import * as path from "path";
 
 import * as actionsUtil from "./actions-util";
 import type { PullRequestBranches } from "./actions-util";
@@ -77,16 +76,12 @@ export interface DiffThunkRange {
   endLine: number;
 }
 
-function getDiffRangesJsonFilePath(): string {
-  return path.join(actionsUtil.getTemporaryDirectory(), "pr-diff-range.json");
-}
-
 export function writeDiffRangesJsonFile(
   logger: Logger,
   ranges: DiffThunkRange[],
 ): void {
   const jsonContents = JSON.stringify(ranges, null, 2);
-  const jsonFilePath = getDiffRangesJsonFilePath();
+  const jsonFilePath = actionsUtil.getDiffRangesJsonFilePath();
   fs.writeFileSync(jsonFilePath, jsonContents);
   logger.debug(
     `Wrote pr-diff-range JSON file to ${jsonFilePath}:\n${jsonContents}`,
@@ -96,7 +91,7 @@ export function writeDiffRangesJsonFile(
 export function readDiffRangesJsonFile(
   logger: Logger,
 ): DiffThunkRange[] | undefined {
-  const jsonFilePath = getDiffRangesJsonFilePath();
+  const jsonFilePath = actionsUtil.getDiffRangesJsonFilePath();
   if (!fs.existsSync(jsonFilePath)) {
     logger.debug(`Diff ranges JSON file does not exist at ${jsonFilePath}`);
     return undefined;

src/overlay/index.test.ts

@@ -72,9 +72,13 @@ test.serial(
 
       // Write the overlay changes file, which uses the mocked overlay OIDs
       // and the base database OIDs file
+      const diffRangeFilePath = path.join(tempDir, "pr-diff-range.json");
       const getTempDirStub = sinon
         .stub(actionsUtil, "getTemporaryDirectory")
         .returns(tempDir);
+      const getDiffRangesStub = sinon
+        .stub(actionsUtil, "getDiffRangesJsonFilePath")
+        .returns(diffRangeFilePath);
       const getGitRootStub = sinon
         .stub(gitUtils, "getGitRoot")
         .resolves(sourceRoot);
@@ -85,6 +89,7 @@ test.serial(
       );
       getFileOidsStubForOverlay.restore();
       getTempDirStub.restore();
+      getDiffRangesStub.restore();
       getGitRootStub.restore();
 
       const fileContent = await fs.promises.readFile(changesFilePath, "utf-8");
@@ -143,9 +148,13 @@ test.serial(
         .stub(gitUtils, "getFileOidsUnderPath")
         .resolves(currentOids);
 
+      const diffRangeFilePath = path.join(tempDir, "pr-diff-range.json");
       const getTempDirStub = sinon
         .stub(actionsUtil, "getTemporaryDirectory")
         .returns(tempDir);
+      const getDiffRangesStub = sinon
+        .stub(actionsUtil, "getDiffRangesJsonFilePath")
+        .returns(diffRangeFilePath);
       const getGitRootStub = sinon
         .stub(gitUtils, "getGitRoot")
         .resolves(sourceRoot);
@@ -153,7 +162,7 @@ test.serial(
       // Write a pr-diff-range.json file with diff ranges including
       // "reverted.js" (unchanged OIDs) and "modified.js" (already in OID changes)
       await fs.promises.writeFile(
-        path.join(tempDir, "pr-diff-range.json"),
+        diffRangeFilePath,
         JSON.stringify([
           { path: "reverted.js", startLine: 1, endLine: 10 },
           { path: "modified.js", startLine: 1, endLine: 5 },
@@ -168,6 +177,7 @@ test.serial(
       );
       getFileOidsStubForOverlay.restore();
       getTempDirStub.restore();
+      getDiffRangesStub.restore();
       getGitRootStub.restore();
 
       const fileContent = await fs.promises.readFile(changesFilePath, "utf-8");
@@ -218,9 +228,13 @@ test.serial(
         .stub(gitUtils, "getFileOidsUnderPath")
         .resolves(currentOids);
 
+      const diffRangeFilePath = path.join(tempDir, "pr-diff-range.json");
       const getTempDirStub = sinon
         .stub(actionsUtil, "getTemporaryDirectory")
         .returns(tempDir);
+      const getDiffRangesStub = sinon
+        .stub(actionsUtil, "getDiffRangesJsonFilePath")
+        .returns(diffRangeFilePath);
       const getGitRootStub = sinon
         .stub(gitUtils, "getGitRoot")
         .resolves(sourceRoot);
@@ -233,6 +247,7 @@ test.serial(
       );
       getFileOidsStubForOverlay.restore();
       getTempDirStub.restore();
+      getDiffRangesStub.restore();
       getGitRootStub.restore();
 
       const fileContent = await fs.promises.readFile(changesFilePath, "utf-8");
@@ -286,9 +301,13 @@ test.serial(
         .stub(gitUtils, "getFileOidsUnderPath")
         .resolves(currentOids);
 
+      const diffRangeFilePath = path.join(tempDir, "pr-diff-range.json");
       const getTempDirStub = sinon
         .stub(actionsUtil, "getTemporaryDirectory")
         .returns(tempDir);
+      const getDiffRangesStub = sinon
+        .stub(actionsUtil, "getDiffRangesJsonFilePath")
+        .returns(diffRangeFilePath);
       // getGitRoot returns the repo root (parent of sourceRoot)
       const getGitRootStub = sinon
         .stub(gitUtils, "getGitRoot")
@@ -296,7 +315,7 @@ test.serial(
 
       // Diff ranges use repo-root-relative paths (as returned by the GitHub compare API)
       await fs.promises.writeFile(
-        path.join(tempDir, "pr-diff-range.json"),
+        diffRangeFilePath,
         JSON.stringify([
           { path: "src/app.js", startLine: 1, endLine: 10 },
           { path: "src/lib/util.js", startLine: 5, endLine: 8 },
@@ -311,6 +330,7 @@ test.serial(
       );
       getFileOidsStubForOverlay.restore();
       getTempDirStub.restore();
+      getDiffRangesStub.restore();
       getGitRootStub.restore();
 
       const fileContent = await fs.promises.readFile(changesFilePath, "utf-8");

src/overlay/index.ts

@@ -3,6 +3,7 @@ import * as path from "path";
 
 import * as actionsCache from "@actions/cache";
 
+import * as actionsUtil from "../actions-util";
 import {
   getRequiredInput,
   getTemporaryDirectory,
@@ -175,15 +176,18 @@ async function getDiffRangeFilePaths(
   sourceRoot: string,
   logger: Logger,
 ): Promise<string[]> {
-  const jsonFilePath = path.join(getTemporaryDirectory(), "pr-diff-range.json");
-  if (!fs.existsSync(jsonFilePath)) {
+  const jsonFilePath = actionsUtil.getDiffRangesJsonFilePath();
+
+  let contents: string;
+  try {
+    contents = await fs.promises.readFile(jsonFilePath, "utf8");
+  } catch {
     return [];
   }
+
   let diffRanges: Array<{ path: string }>;
   try {
-    diffRanges = JSON.parse(fs.readFileSync(jsonFilePath, "utf8")) as Array<{
-      path: string;
-    }>;
+    diffRanges = JSON.parse(contents) as Array<{ path: string }>;
   } catch (e) {
     logger.warning(
       `Failed to parse diff ranges JSON file at ${jsonFilePath}: ${e}`,
@@ -193,7 +197,6 @@ async function getDiffRangeFilePaths(
   logger.debug(
     `Read ${diffRanges.length} diff range(s) from ${jsonFilePath} for overlay changes.`,
   );
-  const repoRelativePaths = [...new Set(diffRanges.map((r) => r.path))];
 
   // Diff-range paths are relative to the repo root (from the GitHub compare
   // API), but overlay changed files must be relative to sourceRoot (to match
@@ -203,31 +206,17 @@ async function getDiffRangeFilePaths(
     logger.warning(
       "Cannot determine git root; returning diff range paths as-is.",
     );
-    return repoRelativePaths;
+    return [...new Set(diffRanges.map((r) => r.path))];
   }
 
-  // e.g. if repoRoot=/workspace and sourceRoot=/workspace/src, prefix="src"
-  const sourceRootRelPrefix = path
-    .relative(repoRoot, sourceRoot)
-    .replaceAll(path.sep, "/");
-
-  // If sourceRoot IS the repo root, prefix is "" and all paths pass through.
-  if (sourceRootRelPrefix === "") {
-    return repoRelativePaths;
-  }
-
-  const prefixWithSlash = `${sourceRootRelPrefix}/`;
-  const result: string[] = [];
-  for (const p of repoRelativePaths) {
-    if (p.startsWith(prefixWithSlash)) {
-      result.push(p.slice(prefixWithSlash.length));
-    } else {
-      logger.debug(
-        `Skipping diff range path "${p}" (not under source root "${sourceRootRelPrefix}").`,
-      );
-    }
-  }
-  return result;
+  const relativePaths = diffRanges
+    .map((r) =>
+      path
+        .relative(sourceRoot, path.join(repoRoot, r.path))
+        .replaceAll(path.sep, "/"),
+    )
+    .filter((rel) => !rel.startsWith(".."));
+  return [...new Set(relativePaths)];
 }
 
 // Constants for database caching