Skip to content

Commit b7437d3

Browse files
committed
Use bqrs diff instead of sarif diff
1 parent d814adc commit b7437d3

File tree

7 files changed

+105
-38072
lines changed

7 files changed

+105
-38072
lines changed

extensions/ql-vscode/src/codeql-cli/cli.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ interface BqrsDecodeOptions {
203203
entities?: string[];
204204
}
205205

206+
interface BqrsDiffOptions {
207+
retainResultSets?: string[];
208+
}
209+
206210
type OnLineCallback = (line: string) => Promise<string | undefined>;
207211

208212
type VersionChangedListener = (
@@ -1255,9 +1259,11 @@ export class CodeQLCliServer implements Disposable {
12551259
async bqrsDiff(
12561260
bqrsPath1: string,
12571261
bqrsPath2: string,
1262+
options?: BqrsDiffOptions,
12581263
): Promise<{
12591264
uniquePath1: string;
12601265
uniquePath2: string;
1266+
path: string;
12611267
cleanup: () => Promise<void>;
12621268
}> {
12631269
const { path, cleanup } = await dir({ unsafeCleanup: true });
@@ -1270,13 +1276,14 @@ export class CodeQLCliServer implements Disposable {
12701276
uniquePath1,
12711277
"--right",
12721278
uniquePath2,
1273-
"--retain-result-sets=''",
1279+
"--retain-result-sets",
1280+
options?.retainResultSets?.join(",") ?? "",
12741281
bqrsPath1,
12751282
bqrsPath2,
12761283
],
12771284
"Diffing bqrs files",
12781285
);
1279-
return { uniquePath1, uniquePath2, cleanup };
1286+
return { uniquePath1, uniquePath2, path, cleanup };
12801287
}
12811288

12821289
private async runInterpretCommand(

extensions/ql-vscode/src/compare/compare-view.ts

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { ViewColumn } from "vscode";
22

33
import type {
44
FromCompareViewMessage,
5-
InterpretedQueryCompareResult,
65
QueryCompareResult,
76
RawQueryCompareResult,
87
ToCompareViewMessage,
@@ -14,7 +13,7 @@ import { extLogger } from "../common/logging/vscode";
1413
import type { CodeQLCliServer } from "../codeql-cli/cli";
1514
import type { DatabaseManager } from "../databases/local-databases";
1615
import { jumpToLocation } from "../databases/local-databases/locations";
17-
import type { BqrsInfo } from "../common/bqrs-cli-types";
16+
import type { BqrsInfo, BqrsResultSetSchema } from "../common/bqrs-cli-types";
1817
import resultsDiff from "./resultsDiff";
1918
import type { CompletedLocalQueryInfo } from "../query-results";
2019
import { assertNever, getErrorMessage } from "../common/helpers-pure";
@@ -36,7 +35,7 @@ import { compareInterpretedResults } from "./interpreted-results";
3635
import { isCanary } from "../config";
3736
import { nanoid } from "nanoid";
3837

39-
interface ComparePair {
38+
export interface ComparePair {
4039
from: CompletedLocalQueryInfo;
4140
fromInfo: CompareQueryInfo;
4241
to: CompletedLocalQueryInfo;
@@ -45,7 +44,7 @@ interface ComparePair {
4544
commonResultSetNames: readonly string[];
4645
}
4746

48-
function findSchema(info: BqrsInfo, name: string) {
47+
export function findSchema(info: BqrsInfo, name: string) {
4948
const schema = info["result-sets"].find((schema) => schema.name === name);
5049
if (schema === undefined) {
5150
throw new Error(`Schema ${name} not found.`);
@@ -182,8 +181,7 @@ export class CompareView extends AbstractWebview<
182181
result = await compareInterpretedResults(
183182
this.databaseManager,
184183
this.cliServer,
185-
this.comparePair.from,
186-
this.comparePair.to,
184+
this.comparePair,
187185
);
188186
} else {
189187
result = await this.compareResults(
@@ -373,18 +371,12 @@ export class CompareView extends AbstractWebview<
373371
const fromPath = from.completedQuery.query.resultsPath;
374372
const toPath = to.completedQuery.query.resultsPath;
375373

376-
const fromSchema = findSchema(fromInfo.schemas, fromResultSetName);
377-
const toSchema = findSchema(toInfo.schemas, toResultSetName);
378-
379-
if (fromSchema.columns.length !== toSchema.columns.length) {
380-
throw new Error("CodeQL Compare: Columns do not match.");
381-
}
382-
if (fromSchema.rows === 0) {
383-
throw new Error("CodeQL Compare: Source query has no results.");
384-
}
385-
if (toSchema.rows === 0) {
386-
throw new Error("CodeQL Compare: Target query has no results.");
387-
}
374+
const { fromSchema, toSchema } = getComparableSchemas(
375+
fromInfo,
376+
toInfo,
377+
fromResultSetName,
378+
toResultSetName,
379+
);
388380

389381
// If the result set names are the same, we use `bqrs diff`. This is more
390382
// efficient, but we can't use it in general as it does not support
@@ -393,13 +385,13 @@ export class CompareView extends AbstractWebview<
393385
const { uniquePath1, uniquePath2, cleanup } =
394386
await this.cliServer.bqrsDiff(fromPath, toPath);
395387
try {
396-
const info1 = await this.cliServer.bqrsInfo(uniquePath1);
397-
const info2 = await this.cliServer.bqrsInfo(uniquePath2);
388+
const uniqueInfo1 = await this.cliServer.bqrsInfo(uniquePath1);
389+
const uniqueInfo2 = await this.cliServer.bqrsInfo(uniquePath2);
398390

399391
// We avoid decoding the results sets if there is no overlap
400392
if (
401-
fromSchema.rows === findSchema(info1, fromResultSetName).rows &&
402-
toSchema.rows === findSchema(info2, toResultSetName).rows
393+
fromSchema.rows === findSchema(uniqueInfo1, fromResultSetName).rows &&
394+
toSchema.rows === findSchema(uniqueInfo2, toResultSetName).rows
403395
) {
404396
throw new Error(
405397
"CodeQL Compare: No overlap between the selected queries.",
@@ -440,3 +432,28 @@ export class CompareView extends AbstractWebview<
440432
}
441433
}
442434
}
435+
436+
/**
437+
* Check that `fromInfo` and `toInfo` have comparable schemas for the given
438+
* result set names and get them if so.
439+
*/
440+
export function getComparableSchemas(
441+
fromInfo: CompareQueryInfo,
442+
toInfo: CompareQueryInfo,
443+
fromResultSetName: string,
444+
toResultSetName: string,
445+
): { fromSchema: BqrsResultSetSchema; toSchema: BqrsResultSetSchema } {
446+
const fromSchema = findSchema(fromInfo.schemas, fromResultSetName);
447+
const toSchema = findSchema(toInfo.schemas, toResultSetName);
448+
449+
if (fromSchema.columns.length !== toSchema.columns.length) {
450+
throw new Error("CodeQL Compare: Columns do not match.");
451+
}
452+
if (fromSchema.rows === 0) {
453+
throw new Error("CodeQL Compare: Source query has no results.");
454+
}
455+
if (toSchema.rows === 0) {
456+
throw new Error("CodeQL Compare: Target query has no results.");
457+
}
458+
return { fromSchema, toSchema };
459+
}
Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,31 @@
11
import { Uri } from "vscode";
2-
import type { Log } from "sarif";
3-
import { pathExists } from "fs-extra";
4-
import { sarifParser } from "../common/sarif-parser";
5-
import type { CompletedLocalQueryInfo } from "../query-results";
2+
import { join } from "path";
63
import type { DatabaseManager } from "../databases/local-databases";
74
import type { CodeQLCliServer } from "../codeql-cli/cli";
8-
import type { InterpretedQueryCompareResult } from "../common/interface-types";
5+
import {
6+
ALERTS_TABLE_NAME,
7+
SELECT_TABLE_NAME,
8+
type InterpretedQueryCompareResult,
9+
} from "../common/interface-types";
910

10-
import { sarifDiff } from "./sarif-diff";
11-
12-
async function getInterpretedResults(
13-
interpretedResultsPath: string,
14-
): Promise<Log | undefined> {
15-
if (!(await pathExists(interpretedResultsPath))) {
16-
return undefined;
17-
}
18-
19-
return await sarifParser(interpretedResultsPath);
20-
}
11+
import { getComparableSchemas, ComparePair } from "./compare-view";
2112

2213
export async function compareInterpretedResults(
2314
databaseManager: DatabaseManager,
2415
cliServer: CodeQLCliServer,
25-
fromQuery: CompletedLocalQueryInfo,
26-
toQuery: CompletedLocalQueryInfo,
16+
comparePair: ComparePair,
2717
): Promise<InterpretedQueryCompareResult> {
18+
const { from: fromQuery, fromInfo, to: toQuery, toInfo } = comparePair;
19+
20+
// `ALERTS_TABLE_NAME` is inserted by `getResultSetNames` into the schema
21+
// names even if it does not occur as a result set. Hence we check for
22+
// `SELECT_TABLE_NAME` first, and use that if it exists.
23+
const tableName = fromInfo.schemaNames.includes(SELECT_TABLE_NAME)
24+
? SELECT_TABLE_NAME
25+
: ALERTS_TABLE_NAME;
26+
27+
getComparableSchemas(fromInfo, toInfo, tableName, tableName);
28+
2829
const database = databaseManager.findDatabaseItem(
2930
Uri.parse(toQuery.initialInfo.databaseInfo.databaseUri),
3031
);
@@ -34,37 +35,46 @@ export async function compareInterpretedResults(
3435
);
3536
}
3637

37-
const [fromResultSet, toResultSet, sourceLocationPrefix] = await Promise.all([
38-
getInterpretedResults(
39-
fromQuery.completedQuery.query.interpretedResultsPath,
40-
),
41-
getInterpretedResults(toQuery.completedQuery.query.interpretedResultsPath),
42-
database.getSourceLocationPrefix(cliServer),
43-
]);
44-
45-
if (!fromResultSet || !toResultSet) {
46-
throw new Error(
47-
"Could not find interpreted results for one or both queries.",
48-
);
49-
}
38+
const { uniquePath1, uniquePath2, path, cleanup } = await cliServer.bqrsDiff(
39+
fromQuery.completedQuery.query.resultsPath,
40+
toQuery.completedQuery.query.resultsPath,
41+
{ retainResultSets: ["nodes", "edges", "subpaths"] },
42+
);
43+
try {
44+
const sarifOutput1 = join(path, "from.sarif");
45+
const sarifOutput2 = join(path, "to.sarif");
5046

51-
const fromResults = fromResultSet.runs[0].results;
52-
const toResults = toResultSet.runs[0].results;
47+
const sourceLocationPrefix =
48+
await database.getSourceLocationPrefix(cliServer);
49+
const sourceArchiveUri = database.sourceArchive;
50+
const sourceInfo =
51+
sourceArchiveUri === undefined
52+
? undefined
53+
: {
54+
sourceArchive: sourceArchiveUri.fsPath,
55+
sourceLocationPrefix,
56+
};
5357

54-
if (!fromResults) {
55-
throw new Error("No results found in the 'from' query.");
56-
}
58+
const fromResultSet = await cliServer.interpretBqrsSarif(
59+
fromQuery.completedQuery.query.metadata!,
60+
uniquePath1,
61+
sarifOutput1,
62+
sourceInfo,
63+
);
64+
const toResultSet = await cliServer.interpretBqrsSarif(
65+
toQuery.completedQuery.query.metadata!,
66+
uniquePath2,
67+
sarifOutput2,
68+
sourceInfo,
69+
);
5770

58-
if (!toResults) {
59-
throw new Error("No results found in the 'to' query.");
71+
return {
72+
kind: "interpreted",
73+
sourceLocationPrefix,
74+
from: fromResultSet.runs[0].results!,
75+
to: toResultSet.runs[0].results!,
76+
};
77+
} finally {
78+
await cleanup();
6079
}
61-
62-
const { from, to } = sarifDiff(fromResults, toResults);
63-
64-
return {
65-
kind: "interpreted",
66-
sourceLocationPrefix,
67-
from,
68-
to,
69-
};
7080
}

0 commit comments

Comments
 (0)