Move conversion of PR diff-range paths to absolute paths

Author: kaspersv

lib/analyze-action.js

@@ -4,6 +4,7 @@ import * as path from "path";
 import test from "ava";
 import * as sinon from "sinon";
 
+import * as actionsUtil from "./actions-util";
 import { CodeQuality, CodeScanning } from "./analyses";
 import {
   runQueries,
@@ -163,9 +164,13 @@ test("addSarifExtension", (t) => {
 });
 
 test("diffRangeExtensionPackContents", (t) => {
+  sinon
+    .stub(actionsUtil, "getRequiredInput")
+    .withArgs("checkout_path")
+    .returns("/checkout/path");
   const output = diffRangeExtensionPackContents([
     {
-      path: "/checkout/path/main.js",
+      path: "main.js",
       startLine: 10,
       endLine: 20,
     },

lib/init-action-post.js

@@ -6,7 +6,11 @@ import * as io from "@actions/io";
 import * as del from "del";
 import * as yaml from "js-yaml";
 
-import { getTemporaryDirectory, PullRequestBranches } from "./actions-util";
+import {
+  getTemporaryDirectory,
+  getRequiredInput,
+  PullRequestBranches,
+} from "./actions-util";
 import * as analyses from "./analyses";
 import { setupCppAutobuild } from "./autobuild";
 import { type CodeQL } from "./codeql";
@@ -321,14 +325,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/upload-lib.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/upload-sarif-action.js

@@ -191,13 +191,6 @@ 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, "/");
-
   if (fileDiff.patch === undefined) {
     if (fileDiff.changes === 0) {
       // There are situations where a changed file legitimately has no diff.
@@ -212,7 +205,7 @@ function getDiffRanges(
     // to a special diff range that covers the entire file.
     return [
       {
-        path: filename,
+        path: fileDiff.filename,
         startLine: 0,
         endLine: 0,
       },
@@ -247,7 +240,7 @@ function getDiffRanges(
       // Any line that does not start with a "+" or "-" terminates the current
       // range of added lines.
       diffRanges.push({
-        path: filename,
+        path: fileDiff.filename,
         startLine: additionRangeStartLine,
         endLine: currentLine - 1,
       });

src/analyze.test.ts

@@ -6,7 +6,6 @@ import { HTTPError } from "@actions/tool-cache";
 import test from "ava";
 import * as sinon from "sinon";
 
-import * as actionsUtil from "./actions-util";
 import * as analyses from "./analyses";
 import { AnalysisKind, CodeQuality, CodeScanning } from "./analyses";
 import * as api from "./api-client";
@@ -972,10 +971,6 @@ function runFilterAlertsByDiffRange(
   input: SarifFile,
   diffRanges: diffUtils.DiffThunkRange[],
 ): SarifFile {
-  sinon
-    .stub(actionsUtil, "getRequiredInput")
-    .withArgs("checkout_path")
-    .returns("/checkout/path");
   sinon.stub(diffUtils, "readDiffRangesJsonFile").returns(diffRanges);
   return uploadLib.filterAlertsByDiffRange(getRunnerLogger(true), input);
 }
@@ -986,7 +981,7 @@ test("filterAlertsByDiffRange filters out alerts outside diff-range", (t) => {
   );
   const actualOutput = runFilterAlertsByDiffRange(input, [
     {
-      path: "/checkout/path/main.js",
+      path: "main.js",
       startLine: 1,
       endLine: 3,
     },

src/analyze.ts

@@ -1143,8 +1143,6 @@ export function filterAlertsByDiffRange(
     return sarif;
   }
 
-  const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
-
   for (const run of sarif.runs) {
     if (run.results) {
       run.results = run.results.filter((result) => {
@@ -1159,19 +1157,14 @@ export function filterAlertsByDiffRange(
           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)),