Skip to content

Commit

Permalink
Merge pull request #2699 from github/cklin/diff-informed-file-fallback
Browse files Browse the repository at this point in the history
getDiffRanges: better fallback for absent patch
  • Loading branch information
cklin authored Jan 16, 2025
2 parents 94f08f3 + 2d608a3 commit 0f1559a
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 27 deletions.
25 changes: 15 additions & 10 deletions lib/analyze.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/analyze.js.map

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion lib/analyze.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/analyze.test.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion src/analyze.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
});
29 changes: 16 additions & 13 deletions src/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down

0 comments on commit 0f1559a

Please sign in to comment.