getDiffRanges: better fallback for absent patch

This commit is contained in:
Chuan-kai Lin 2025-01-16 09:26:22 -08:00
parent 5889cfd499
commit 071996fbe2
6 changed files with 56 additions and 27 deletions

25
lib/analyze.js generated
View file

@ -243,6 +243,12 @@ async function getFileDiffsWithBasehead(baseRef, headLabel, logger) {
}
}
function getDiffRanges(fileDiff, logger) {
// 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.
@ -253,17 +259,16 @@ function getDiffRanges(fileDiff, logger) {
return [];
}
// If a file is reported to have nonzero changes but no patch, that may be
// due to the file diff being too large. In this case, we should return
// undefined to indicate that we cannot process the diff.
logger.warning(`No patch found for file ${fileDiff.filename} with ${fileDiff.changes} changes.`);
return undefined;
// due to the file diff being too large. In this case, we should fall back
// to a special diff range that covers the entire file.
return [
{
path: filename,
startLine: 0,
endLine: 0,
},
];
}
// 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, "/");
// The 1-based file line number of the current line
let currentLine = 0;
// The 1-based file line number that starts the current range of added lines

File diff suppressed because one or more lines are too long

12
lib/analyze.test.js generated
View file

@ -146,7 +146,13 @@ function runGetDiffRanges(changes, patch) {
});
(0, ava_1.default)("getDiffRanges: file diff too large", async (t) => {
const diffRanges = runGetDiffRanges(1000000, undefined);
t.deepEqual(diffRanges, undefined);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 0,
endLine: 0,
},
]);
});
(0, ava_1.default)("getDiffRanges: diff thunk with single addition range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
@ -303,4 +309,8 @@ function runGetDiffRanges(changes, patch) {
},
]);
});
(0, ava_1.default)("getDiffRanges: malformed thunk header", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ 30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, undefined);
});
//# sourceMappingURL=analyze.test.js.map

File diff suppressed because one or more lines are too long

View file

@ -144,7 +144,13 @@ test("getDiffRanges: file unchanged", async (t) => {
test("getDiffRanges: file diff too large", async (t) => {
const diffRanges = runGetDiffRanges(1000000, undefined);
t.deepEqual(diffRanges, undefined);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 0,
endLine: 0,
},
]);
});
test("getDiffRanges: diff thunk with single addition range", async (t) => {
@ -308,3 +314,8 @@ test("getDiffRanges: no diff context lines", async (t) => {
},
]);
});
test("getDiffRanges: malformed thunk header", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ 30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, undefined);
});

View file

@ -387,6 +387,13 @@ 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.
@ -397,21 +404,17 @@ function getDiffRanges(
return [];
}
// If a file is reported to have nonzero changes but no patch, that may be
// due to the file diff being too large. In this case, we should return
// undefined to indicate that we cannot process the diff.
logger.warning(
`No patch found for file ${fileDiff.filename} with ${fileDiff.changes} changes.`,
);
return undefined;
// due to the file diff being too large. In this case, we should fall back
// to a special diff range that covers the entire file.
return [
{
path: filename,
startLine: 0,
endLine: 0,
},
];
}
// 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, "/");
// The 1-based file line number of the current line
let currentLine = 0;
// The 1-based file line number that starts the current range of added lines