Skip to content

Commit

Permalink
Diff-informed PR analysis
Browse files Browse the repository at this point in the history
  • Loading branch information
cklin committed Oct 21, 2024
1 parent 78cfb2f commit e3c191f
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 2 deletions.
15 changes: 15 additions & 0 deletions src/analyze-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from "path";
import { performance } from "perf_hooks";

import * as core from "@actions/core";
import * as github from "@actions/github";

import * as actionsUtil from "./actions-util";
import {
Expand All @@ -12,6 +13,7 @@ import {
runCleanup,
runFinalize,
runQueries,
setupDiffInformedQueryRun,
warnIfGoInstalledAfterInit,
} from "./analyze";
import { getApiDetails, getGitHubVersion } from "./api-client";
Expand Down Expand Up @@ -260,6 +262,18 @@ async function run() {
logger,
);

const pull_request = github.context.payload.pull_request;
const diffRangePackDir =
pull_request === undefined
? undefined
: await setupDiffInformedQueryRun(
pull_request.base.ref as string,
pull_request.head.ref as string,
codeql,
logger,
features,
);

await warnIfGoInstalledAfterInit(config, logger);
await runAutobuildIfLegacyGoWorkflow(config, logger);

Expand All @@ -278,6 +292,7 @@ async function run() {
memory,
util.getAddSnippetsFlag(actionsUtil.getRequiredInput("add-snippets")),
threads,
diffRangePackDir,
actionsUtil.getOptionalInput("category"),
config,
logger,
Expand Down
1 change: 1 addition & 0 deletions src/analyze.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ test("status report fields", async (t) => {
addSnippetsFlag,
threadsFlag,
undefined,
undefined,
config,
getRunnerLogger(true),
createFeatures([Feature.QaTelemetryEnabled]),
Expand Down
207 changes: 205 additions & 2 deletions src/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { safeWhich } from "@chrisgavin/safe-which";
import del from "del";
import * as yaml from "js-yaml";

import * as actionsUtil from "./actions-util";
import { setupCppAutobuild } from "./autobuild";
import {
CODEQL_VERSION_ANALYSIS_SUMMARY_V2,
Expand Down Expand Up @@ -234,23 +235,225 @@ async function finalizeDatabaseCreation(
};
}

/**
* Set up the diff-informed analysis feature.
*
* @param baseRef The base branch name, used for calculating the diff range.
* @param headRef The head branch name, used for calculating the diff range.
* @param codeql
* @param logger
* @param features
* @returns Absolute path to the directory containing the extension pack for
* the diff range information, or `undefined` if the feature is disabled.
*/
export async function setupDiffInformedQueryRun(
baseRef: string,
headRef: string,
codeql: CodeQL,
logger: Logger,
features: FeatureEnablement,
): Promise<string | undefined> {
if (!(await features.getValue(Feature.DiffInformedQueries, codeql))) {
return undefined;
}
logger.startGroup("Generating diff range extension pack");
const diffRanges = await getPullRequestEditedDiffRanges(
baseRef,
headRef,
logger,
);
const diffRangePackDir = writeDiffRangeDataExtensionPack(logger, diffRanges);
logger.endGroup();
return diffRangePackDir;
}

/**
* Return the file line ranges that were added or modified in the pull request.
*
* @param baseRef The base branch name, used for calculating the diff range.
* @param headRef The head branch name, used for calculating the diff range.
* @param logger
* @returns An array of tuples, where each tuple contains the absolute path of a
* file, the start line and the end line (both 1-based and inclusive) of an
* added or modified range in that file. Returns `undefined` if the action was
* not triggered by a pull request or if there was an error.
*/
async function getPullRequestEditedDiffRanges(
baseRef: string,
headRef: string,
logger: Logger,
): Promise<Array<[string, number, number]> | undefined> {
const checkoutPath = actionsUtil.getOptionalInput("checkout_path");
if (checkoutPath === undefined) {
return undefined;
}

// To compute the merge bases between the base branch and the PR topic branch,
// we need to fetch the commit graph from the branch heads to those merge
// babes. The following 4-step procedure does so while limiting the amount of
// history fetched.

// Step 1: Deepen from the PR merge commit to the base branch head and the PR
// topic branch head, so that the PR merge commit is no longer considered a
// grafted commit.
await actionsUtil.deepenGitHistory();
// Step 2: Fetch the base branch shallow history. This step ensures that the
// base branch name is present in the local repository. Normally the base
// branch name would be added by Step 4. However, if the base branch head is
// an ancestor of the PR topic branch head, Step 4 would fail without doing
// anything, so we need to fetch the base branch explicitly.
await actionsUtil.gitFetch(baseRef, ["--depth=1"]);
// Step 3: Fetch the PR topic branch history, stopping when we reach commits
// that are reachable from the base branch head.
await actionsUtil.gitFetch(headRef, [`--shallow-exclude=${baseRef}`]);
// Step 4: Fetch the base branch history, stopping when we reach commits that
// are reachable from the PR topic branch head.
await actionsUtil.gitFetch(baseRef, [`--shallow-exclude=${headRef}`]);
// Step 5: Deepen the history so that we have the merge bases between the base
// branch and the PR topic branch.
await actionsUtil.deepenGitHistory();

// To compute the exact same diff as GitHub would compute for the PR, we need
// to use the same merge base as GitHub. That is easy to do if there is only
// one merge base, which is by far the most common case. If there are multiple
// merge bases, we stop without producing a diff range.
const mergeBases = await actionsUtil.getAllGitMergeBases([baseRef, headRef]);
logger.info(`Merge bases: ${mergeBases.join(", ")}`);
if (mergeBases.length !== 1) {
return undefined;
}

const diffHunkHeaders = await actionsUtil.getGitDiffHunkHeaders(
mergeBases[0],
headRef,
);
if (diffHunkHeaders === undefined) {
return undefined;
}

const results = new Array<[string, number, number]>();

let changedFile = "";
for (const line of diffHunkHeaders) {
if (line.startsWith("+++ ")) {
const filePath = actionsUtil.decodeGitFilePath(line.substring(4));
if (filePath.startsWith("b/")) {
changedFile = filePath.substring(2);
} else {
// Not an actual file path; probably /dev/null
changedFile = "";
}
continue;
}
if (line.startsWith("@@ ")) {
if (changedFile === "") continue;

const match = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/);
if (match === null) {
logger.info(
`Failed to parse diff hunk header line: ${line}. Skipping.`,
);
continue;
}
const startLine = parseInt(match[1], 10);
const endLine = startLine + (parseInt(match[2], 10) || 1) - 1;
results.push([path.join(checkoutPath, changedFile), startLine, endLine]);
}
}
return results;
}

/**
* Create an extension pack in the temporary directory that contains the file
* line ranges that were added or modified in the pull request.
*
* @param logger
* @param ranges The file line ranges, as returned by
* `getPullRequestEditedDiffRanges`.
* @returns The absolute path of the directory containing the extension pack, or
* `undefined` if no extension pack was created.
*/
function writeDiffRangeDataExtensionPack(
logger: Logger,
ranges: Array<[string, number, number]> | undefined,
): string | undefined {
if (ranges === undefined) {
return undefined;
}

const diffRangeDir = path.join(
actionsUtil.getTemporaryDirectory(),
"pr-diff-range",
);
fs.mkdirSync(diffRangeDir);
fs.writeFileSync(
path.join(diffRangeDir, "qlpack.yml"),
`
name: codeql-action/pr-diff-range
version: 0.0.0
library: true
extensionTargets:
codeql/util: '*'
dataExtensions:
- pr-diff-range.yml
`,
);

const header = `
extensions:
- addsTo:
pack: codeql/util
extensible: restrictAlertsTo
data:
`;

let data = ranges
.map((range) => {
return ` - ["${range[0]}", ${range[1]}, ${range[2]}]\n`;
})
.join("");
if (!data) {
// Ensure that the data extension is not empty, so that a pull request with
// no edited lines would exclude (instead of accepting) all alerts.
data = ' - ["", 0, 0]\n';
}

const extensionContents = header + data;
const extensionFilePath = path.join(diffRangeDir, "pr-diff-range.yml");
fs.writeFileSync(extensionFilePath, extensionContents);
logger.info(
`Wrote pr-diff-range extension pack to ${extensionFilePath}:\n${extensionContents}`,
);

return diffRangeDir;
}

// Runs queries and creates sarif files in the given folder
export async function runQueries(
sarifFolder: string,
memoryFlag: string,
addSnippetsFlag: string,
threadsFlag: string,
diffRangePackDir: string | undefined,
automationDetailsId: string | undefined,
config: configUtils.Config,
logger: Logger,
features: FeatureEnablement,
): Promise<QueriesStatusReport> {
const statusReport: QueriesStatusReport = {};

const sarifRunPropertyFlag = undefined;
const dataExtensionFlags = diffRangePackDir
? [
`--additional-packs=${diffRangePackDir}`,
"--extension-packs=codeql-action/pr-diff-range",
]
: [];
const sarifRunPropertyFlag = diffRangePackDir
? "--sarif-run-property=incrementalMode=diff-informed"
: undefined;

const codeql = await getCodeQL(config.codeQLCmd);
const queryFlags = [memoryFlag, threadsFlag];
const queryFlags = [memoryFlag, threadsFlag, ...dataExtensionFlags];

for (const language of config.languages) {
try {
Expand Down

0 comments on commit e3c191f

Please sign in to comment.