WIP: Move generation out absolute paths for diff-informed PR ranges

Author: kaspersv

lib/analyze-action.js

@@ -6,7 +6,7 @@ import * as io from "@actions/io";
 import * as del from "del";
 import * as yaml from "js-yaml";
 
-import { getTemporaryDirectory } from "./actions-util";
+import { getTemporaryDirectory, getRequiredInput } from "./actions-util";
 import * as analyses from "./analyses";
 import { setupCppAutobuild } from "./autobuild";
 import { type CodeQL } from "./codeql";
@@ -385,14 +385,22 @@ extensions:
 `;
 
   let data = ranges
-    .map(
-      (range) =>
-        // Using yaml.dump() with `forceQuotes: true` ensures that all special
-        // characters are escaped, and that the path is always rendered as a
-        // quoted string on a single line.
-        `      - [${yaml.dump(range.path, { forceQuotes: true }).trim()}, ` +
-        `${range.startLine}, ${range.endLine}]\n`,
-    )
+    .map((range) => {
+      // Diff-informed queries expect the file path to be absolute. CodeQL always
+      // uses forward slashes as the path separator, so on Windows we need to
+      // replace any backslashes with forward slashes.
+      const filename = path
+        .join(getRequiredInput("checkout_path"), range.path)
+        .replaceAll(path.sep, "/");
+
+      // Using yaml.dump() with `forceQuotes: true` ensures that all special
+      // characters are escaped, and that the path is always rendered as a
+      // quoted string on a single line.
+      return (
+        `      - [${yaml.dump(filename, { forceQuotes: true }).trim()}, ` +
+        `${range.startLine}, ${range.endLine}]\n`
+      );
+    })
     .join("");
   if (!data) {
     // Ensure that the data extension is not empty, so that a pull request with

lib/init-action-post.js

@@ -188,10 +188,6 @@ test(
 );
 
 function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
-  sinon
-    .stub(actionsUtil, "getRequiredInput")
-    .withArgs("checkout_path")
-    .returns("/checkout/path");
   return exportedForTesting.getDiffRanges(
     {
       filename: "test.txt",
@@ -211,7 +207,7 @@ test("getDiffRanges: file diff too large", async (t) => {
   const diffRanges = runGetDiffRanges(1000000, undefined);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 0,
       endLine: 0,
     },
@@ -232,7 +228,7 @@ test("getDiffRanges: diff thunk with single addition range", async (t) => {
   ]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 53,
       endLine: 54,
     },
@@ -268,7 +264,7 @@ test("getDiffRanges: diff thunk with single update range", async (t) => {
   ]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 53,
       endLine: 53,
     },
@@ -290,12 +286,12 @@ test("getDiffRanges: diff thunk with addition ranges", async (t) => {
   ]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 53,
       endLine: 53,
     },
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 55,
       endLine: 55,
     },
@@ -322,12 +318,12 @@ test("getDiffRanges: diff thunk with mixed ranges", async (t) => {
   ]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 54,
       endLine: 54,
     },
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 57,
       endLine: 58,
     },
@@ -357,12 +353,12 @@ test("getDiffRanges: multiple diff thunks", async (t) => {
   ]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 53,
       endLine: 54,
     },
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 153,
       endLine: 154,
     },
@@ -373,7 +369,7 @@ test("getDiffRanges: no diff context lines", async (t) => {
   const diffRanges = runGetDiffRanges(2, ["@@ -30 +50,2 @@", "+1", "+2"]);
   t.deepEqual(diffRanges, [
     {
-      path: "/checkout/path/test.txt",
+      path: "test.txt",
       startLine: 50,
       endLine: 51,
     },

lib/init-action.js

@@ -191,12 +191,7 @@ function getDiffRanges(
   fileDiff: FileDiff,
   logger: Logger,
 ): DiffThunkRange[] | undefined {
-  // Diff-informed queries expect the file path to be absolute. CodeQL always
-  // uses forward slashes as the path separator, so on Windows we need to
-  // replace any backslashes with forward slashes.
-  const filename = path
-    .join(actionsUtil.getRequiredInput("checkout_path"), fileDiff.filename)
-    .replaceAll(path.sep, "/");
+  const filename = fileDiff.filename;
 
   if (fileDiff.patch === undefined) {
     if (fileDiff.changes === 0) {

lib/upload-lib.js

@@ -1140,8 +1140,6 @@ function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
     return sarif;
   }
 
-  const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
-
   for (const run of sarif.runs) {
     if (run.results) {
       run.results = run.results.filter((result) => {
@@ -1156,19 +1154,14 @@ function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
           if (!locationUri || locationStartLine === undefined) {
             return false;
           }
-          // CodeQL always uses forward slashes as the path separator, so on Windows we
-          // need to replace any backslashes with forward slashes.
-          const locationPath = path
-            .join(checkoutPath, locationUri)
-            .replaceAll(path.sep, "/");
           // Alert filtering here replicates the same behavior as the restrictAlertsTo
           // extensible predicate in CodeQL. See the restrictAlertsTo documentation
           // https://codeql.github.com/codeql-standard-libraries/csharp/codeql/util/AlertFiltering.qll/predicate.AlertFiltering$restrictAlertsTo.3.html
           // for more details, such as why the filtering applies only to the first line
           // of an alert location.
           return diffRanges.some(
             (range) =>
-              range.path === locationPath &&
+              range.path === locationUri &&
               ((range.startLine <= locationStartLine &&
                 range.endLine >= locationStartLine) ||
                 (range.startLine === 0 && range.endLine === 0)),