Move conversion of PR diff-range paths to absolute paths

This commit is contained in:
Kasper Svendsen
2025-10-28 13:23:31 +01:00
parent df4e1992c0
commit 4eb247591f
10 changed files with 56 additions and 75 deletions
+6 -1
View File
@@ -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,
@@ -161,9 +162,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,
},
+21 -9
View File
@@ -5,7 +5,11 @@ import { performance } from "perf_hooks";
import * as io from "@actions/io";
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";
@@ -257,14 +261,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
+10 -14
View File
@@ -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,
},
+2 -9
View File
@@ -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,
});
+1 -6
View File
@@ -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,
},
+1 -8
View File
@@ -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,11 +1157,6 @@ 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
@@ -1171,7 +1164,7 @@ export function filterAlertsByDiffRange(
// 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)),