Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 0f1559a

Browse filesBrowse files
authored
Merge pull request #2699 from github/cklin/diff-informed-file-fallback
getDiffRanges: better fallback for absent patch
2 parents 94f08f3 + 2d608a3 commit 0f1559a
Copy full SHA for 0f1559a

File tree

Expand file treeCollapse file tree

6 files changed

+56
-27
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+56
-27
lines changed

‎lib/analyze.js

Copy file name to clipboardExpand all lines: lib/analyze.js
+15-10Lines changed: 15 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/analyze.js.map

Copy file name to clipboardExpand all lines: lib/analyze.js.map
+1-1Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/analyze.test.js

Copy file name to clipboardExpand all lines: lib/analyze.test.js
+11-1Lines changed: 11 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/analyze.test.js.map

Copy file name to clipboardExpand all lines: lib/analyze.test.js.map
+1-1Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/analyze.test.ts

Copy file name to clipboardExpand all lines: src/analyze.test.ts
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,13 @@ test("getDiffRanges: file unchanged", async (t) => {
144144

145145
test("getDiffRanges: file diff too large", async (t) => {
146146
const diffRanges = runGetDiffRanges(1000000, undefined);
147-
t.deepEqual(diffRanges, undefined);
147+
t.deepEqual(diffRanges, [
148+
{
149+
path: "/checkout/path/test.txt",
150+
startLine: 0,
151+
endLine: 0,
152+
},
153+
]);
148154
});
149155

150156
test("getDiffRanges: diff thunk with single addition range", async (t) => {
@@ -308,3 +314,8 @@ test("getDiffRanges: no diff context lines", async (t) => {
308314
},
309315
]);
310316
});
317+
318+
test("getDiffRanges: malformed thunk header", async (t) => {
319+
const diffRanges = runGetDiffRanges(2, ["@@ 30 +50,2 @@", "+1", "+2"]);
320+
t.deepEqual(diffRanges, undefined);
321+
});

‎src/analyze.ts

Copy file name to clipboardExpand all lines: src/analyze.ts
+16-13Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,13 @@ function getDiffRanges(
387387
fileDiff: FileDiff,
388388
logger: Logger,
389389
): DiffThunkRange[] | undefined {
390+
// Diff-informed queries expect the file path to be absolute. CodeQL always
391+
// uses forward slashes as the path separator, so on Windows we need to
392+
// replace any backslashes with forward slashes.
393+
const filename = path
394+
.join(actionsUtil.getRequiredInput("checkout_path"), fileDiff.filename)
395+
.replaceAll(path.sep, "/");
396+
390397
if (fileDiff.patch === undefined) {
391398
if (fileDiff.changes === 0) {
392399
// There are situations where a changed file legitimately has no diff.
@@ -397,21 +404,17 @@ function getDiffRanges(
397404
return [];
398405
}
399406
// If a file is reported to have nonzero changes but no patch, that may be
400-
// due to the file diff being too large. In this case, we should return
401-
// undefined to indicate that we cannot process the diff.
402-
logger.warning(
403-
`No patch found for file ${fileDiff.filename} with ${fileDiff.changes} changes.`,
404-
);
405-
return undefined;
407+
// due to the file diff being too large. In this case, we should fall back
408+
// to a special diff range that covers the entire file.
409+
return [
410+
{
411+
path: filename,
412+
startLine: 0,
413+
endLine: 0,
414+
},
415+
];
406416
}
407417

408-
// Diff-informed queries expect the file path to be absolute. CodeQL always
409-
// uses forward slashes as the path separator, so on Windows we need to
410-
// replace any backslashes with forward slashes.
411-
const filename = path
412-
.join(actionsUtil.getRequiredInput("checkout_path"), fileDiff.filename)
413-
.replaceAll(path.sep, "/");
414-
415418
// The 1-based file line number of the current line
416419
let currentLine = 0;
417420
// The 1-based file line number that starts the current range of added lines

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.