Skip to content

Commit 08d1198

Browse files
authored
Merge pull request #3248 from github/kaspersv/move-diff-range-absolute-path-conversion
Move conversion of PR diff-range paths to absolute paths
2 parents fd1ca02 + 5e54629 commit 08d1198

12 files changed

+354
-104
lines changed

lib/analyze-action.js

Lines changed: 33 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/init-action-post.js

Lines changed: 5 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.js

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-sarif-action.js

Lines changed: 1 addition & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/analyze.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
defaultSuites,
1111
resolveQuerySuiteAlias,
1212
addSarifExtension,
13+
diffRangeExtensionPackContents,
1314
} from "./analyze";
1415
import { createStubCodeQL } from "./codeql";
1516
import { Feature } from "./feature-flags";
@@ -158,3 +159,22 @@ test("addSarifExtension", (t) => {
158159
t.is(addSarifExtension(RiskAssessment, language), `${language}.csra.sarif`);
159160
}
160161
});
162+
163+
test("diffRangeExtensionPackContents", (t) => {
164+
const output = diffRangeExtensionPackContents(
165+
[
166+
{
167+
path: "main.js",
168+
startLine: 10,
169+
endLine: 20,
170+
},
171+
],
172+
"/checkout/path",
173+
);
174+
175+
const expected = fs.readFileSync(
176+
`${__dirname}/../src/testdata/pr-diff-range.yml`,
177+
"utf8",
178+
);
179+
t.deepEqual(output, expected);
180+
});

src/analyze.ts

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { performance } from "perf_hooks";
55
import * as io from "@actions/io";
66
import * as yaml from "js-yaml";
77

8-
import { getTemporaryDirectory, PullRequestBranches } from "./actions-util";
8+
import {
9+
getTemporaryDirectory,
10+
getRequiredInput,
11+
PullRequestBranches,
12+
} from "./actions-util";
913
import * as analyses from "./analyses";
1014
import { setupCppAutobuild } from "./autobuild";
1115
import { type CodeQL } from "./codeql";
@@ -243,7 +247,12 @@ export async function setupDiffInformedQueryRun(
243247
`Calculating diff ranges for ${branches.base}...${branches.head}`,
244248
);
245249
const diffRanges = await getPullRequestEditedDiffRanges(branches, logger);
246-
const packDir = writeDiffRangeDataExtensionPack(logger, diffRanges);
250+
const checkoutPath = getRequiredInput("checkout_path");
251+
const packDir = writeDiffRangeDataExtensionPack(
252+
logger,
253+
diffRanges,
254+
checkoutPath,
255+
);
247256
if (packDir === undefined) {
248257
logger.warning(
249258
"Cannot create diff range extension pack for diff-informed queries; " +
@@ -259,19 +268,61 @@ export async function setupDiffInformedQueryRun(
259268
);
260269
}
261270

271+
export function diffRangeExtensionPackContents(
272+
ranges: DiffThunkRange[],
273+
checkoutPath: string,
274+
): string {
275+
const header = `
276+
extensions:
277+
- addsTo:
278+
pack: codeql/util
279+
extensible: restrictAlertsTo
280+
checkPresence: false
281+
data:
282+
`;
283+
284+
let data = ranges
285+
.map((range) => {
286+
// Diff-informed queries expect the file path to be absolute. CodeQL always
287+
// uses forward slashes as the path separator, so on Windows we need to
288+
// replace any backslashes with forward slashes.
289+
const filename = path
290+
.join(checkoutPath, range.path)
291+
.replaceAll(path.sep, "/");
292+
293+
// Using yaml.dump() with `forceQuotes: true` ensures that all special
294+
// characters are escaped, and that the path is always rendered as a
295+
// quoted string on a single line.
296+
return (
297+
` - [${yaml.dump(filename, { forceQuotes: true }).trim()}, ` +
298+
`${range.startLine}, ${range.endLine}]\n`
299+
);
300+
})
301+
.join("");
302+
if (!data) {
303+
// Ensure that the data extension is not empty, so that a pull request with
304+
// no edited lines would exclude (instead of accepting) all alerts.
305+
data = ' - ["", 0, 0]\n';
306+
}
307+
308+
return header + data;
309+
}
310+
262311
/**
263312
* Create an extension pack in the temporary directory that contains the file
264313
* line ranges that were added or modified in the pull request.
265314
*
266315
* @param logger
267316
* @param ranges The file line ranges, as returned by
268317
* `getPullRequestEditedDiffRanges`.
318+
* @param checkoutPath The path at which the repository was checked out.
269319
* @returns The absolute path of the directory containing the extension pack, or
270320
* `undefined` if no extension pack was created.
271321
*/
272322
function writeDiffRangeDataExtensionPack(
273323
logger: Logger,
274324
ranges: DiffThunkRange[] | undefined,
325+
checkoutPath: string,
275326
): string | undefined {
276327
if (ranges === undefined) {
277328
return undefined;
@@ -307,32 +358,10 @@ dataExtensions:
307358
`,
308359
);
309360

310-
const header = `
311-
extensions:
312-
- addsTo:
313-
pack: codeql/util
314-
extensible: restrictAlertsTo
315-
checkPresence: false
316-
data:
317-
`;
318-
319-
let data = ranges
320-
.map(
321-
(range) =>
322-
// Using yaml.dump() with `forceQuotes: true` ensures that all special
323-
// characters are escaped, and that the path is always rendered as a
324-
// quoted string on a single line.
325-
` - [${yaml.dump(range.path, { forceQuotes: true }).trim()}, ` +
326-
`${range.startLine}, ${range.endLine}]\n`,
327-
)
328-
.join("");
329-
if (!data) {
330-
// Ensure that the data extension is not empty, so that a pull request with
331-
// no edited lines would exclude (instead of accepting) all alerts.
332-
data = ' - ["", 0, 0]\n';
333-
}
334-
335-
const extensionContents = header + data;
361+
const extensionContents = diffRangeExtensionPackContents(
362+
ranges,
363+
checkoutPath,
364+
);
336365
const extensionFilePath = path.join(diffRangeDir, "pr-diff-range.yml");
337366
fs.writeFileSync(extensionFilePath, extensionContents);
338367
logger.debug(

0 commit comments

Comments
 (0)