mrva/vscode-codeql
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Refactor interpreted-data interface in preparation for other interpretations than SARIF

Browse Source
This commit is contained in:
Tom Hvitved
2021-03-25 11:55:38 +01:00
committed by Andrew Eisenberg
parent a2b5ad07ff
commit d9a04ea895
8 changed files with 116 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
extensions/ql-vscode/src/cli.ts
View File

@@ -674,20 +674,13 @@ export class CodeQLCliServer implements Disposable {
return await this.runJsonCodeQlCliCommand<DecodedBqrsChunk>(['bqrs', 'decode'], subcommandArgs, 'Reading bqrs data');
}
async runInterpretCommand(format: string, metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo) {
async runInterpretCommand(format: string, additonalArgs: string[], metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo) {
const args = [
'--output', interpretedResultsPath,
'--format', format,
// Forward all of the query metadata.
...Object.entries(metadata).map(([key, value]) => `-t=${key}=${value}`)
];
if (format == SARIF_FORMAT) {
// TODO: This flag means that we don't group interpreted results
// by primary location. We may want to revisit whether we call
// interpretation with and without this flag, or do some
// grouping client-side.
args.push('--no-group-results');
}
].concat(additonalArgs);
if (sourceInfo !== undefined) {
args.push(
'--source-archive', sourceInfo.sourceArchive,
@@ -710,12 +703,20 @@ export class CodeQLCliServer implements Disposable {
}
async interpretBqrs(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise<sarif.Log> {
await this.runInterpretCommand(SARIF_FORMAT, metadata, resultsPath, interpretedResultsPath, sourceInfo);
const additionalArgs = [
// TODO: This flag means that we don't group interpreted results
// by primary location. We may want to revisit whether we call
// interpretation with and without this flag, or do some
// grouping client-side.
'--no-group-results'
];
await this.runInterpretCommand(SARIF_FORMAT, additionalArgs, metadata, resultsPath, interpretedResultsPath, sourceInfo);
return await sarifParser(interpretedResultsPath);
}
async generateResultsCsv(metadata: QueryMetadata, resultsPath: string, csvPath: string, sourceInfo?: SourceInfo): Promise<void> {
await this.runInterpretCommand(CSV_FORMAT, metadata, resultsPath, csvPath, sourceInfo);
await this.runInterpretCommand(CSV_FORMAT, [], metadata, resultsPath, csvPath, sourceInfo);
}
async sortBqrs(resultsPath: string, sortedResultsPath: string, resultSet: string, sortKeys: number[], sortDirections: SortDirection[]): Promise<void> {
@@ -1039,11 +1040,11 @@ class SplitBuffer {
/**
* A version of startsWith that isn't overriden by a broken version of ms-python.
*
*
* The definition comes from
* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith
* which is CC0/public domain
*
*
* See https://github.com/github/vscode-codeql/issues/802 for more context as to why we need it.
*/
private static startsWith(s: string, searchString: string, position: number): boolean {

41
extensions/ql-vscode/src/interface.ts
View File

@@ -32,7 +32,7 @@ import {
import { Logger } from './logging';
import * as messages from './pure/messages';
import { commandRunner } from './commandRunner';
import { CompletedQueryInfo, interpretResults } from './query-results';
import { CompletedQueryInfo, interpretResultsSarif } from './query-results';
import { QueryEvaluationInfo, tmpDir } from './run-queries';
import { parseSarifLocation, parseSarifPlainTextMessage } from './pure/sarif-utils';
import {
@@ -93,7 +93,7 @@ function numPagesOfResultSet(resultSet: RawResultSet): number {
}
function numInterpretedPages(interpretation: Interpretation | undefined): number {
return Math.ceil((interpretation?.sarif.runs[0].results?.length || 0) / PAGE_SIZE.getValue<number>());
return Math.ceil((interpretation?.data.runs[0].results?.length || 0) / PAGE_SIZE.getValue<number>());
}
export class InterfaceManager extends DisposableObject {
@@ -140,9 +140,9 @@ export class InterfaceManager extends DisposableObject {
this._diagnosticCollection.clear();
if (this.isShowingPanel()) {
void this.postMessage({
t: 'untoggleShowProblems'
});
}
t: 'untoggleShowProblems'
});
}
}
})
);
@@ -474,7 +474,7 @@ export class InterfaceManager extends DisposableObject {
if (this._interpretation === undefined) {
throw new Error('Trying to show interpreted results but interpretation was undefined');
}
if (this._interpretation.sarif.runs[0].results === undefined) {
if (this._interpretation.data.t === 'SarifInterpretationData' && this._interpretation.data.runs[0].results === undefined) {
throw new Error('Trying to show interpreted results but results were undefined');
}
@@ -592,7 +592,7 @@ export class InterfaceManager extends DisposableObject {
return undefined;
}
const sarif = await interpretResults(
const sarif = await interpretResultsSarif(
this.cliServer,
metadata,
resultsPaths,
@@ -608,7 +608,7 @@ export class InterfaceManager extends DisposableObject {
const numTotalResults = sarif.runs[0]?.results?.length || 0;
const interpretation: Interpretation = {
sarif,
data: sarif,
sourceLocationPrefix,
numTruncatedResults: 0,
numTotalResults,
@@ -621,7 +621,6 @@ export class InterfaceManager extends DisposableObject {
private getPageOfInterpretedResults(
pageNumber: number
): Interpretation {
function getPageOfRun(run: Sarif.Run): Sarif.Run {
return {
...run, results: run.results?.slice(
@@ -631,16 +630,21 @@ export class InterfaceManager extends DisposableObject {
};
}
if (this._interpretation === undefined) {
const interp = this._interpretation;
if (interp === undefined) {
throw new Error('Tried to get interpreted results before interpretation finished');
}
if (this._interpretation.sarif.runs.length !== 1) {
void this.logger.log(`Warning: SARIF file had ${this._interpretation.sarif.runs.length} runs, expected 1`);
if (interp.data.t !== 'SarifInterpretationData')
return interp;
if (interp.data.runs.length !== 1) {
void this.logger.log(`Warning: SARIF file had ${interp.data.runs.length} runs, expected 1`);
}
const interp = this._interpretation;
return {
...interp,
sarif: { ...interp.sarif, runs: [getPageOfRun(interp.sarif.runs[0])] },
data: { ...interp.data, runs: [getPageOfRun(interp.data.runs[0])] },
};
}
@@ -730,9 +734,12 @@ export class InterfaceManager extends DisposableObject {
interpretation: Interpretation,
databaseItem: DatabaseItem
): Promise<void> {
const { sarif, sourceLocationPrefix } = interpretation;
const { data, sourceLocationPrefix } = interpretation;
if (!sarif.runs || !sarif.runs[0].results) {
if (data.t !== 'SarifInterpretationData')
return;
if (!data.runs || !data.runs[0].results) {
void this.logger.log(
'Didn\'t find a run in the sarif results. Error processing sarif?'
);
@@ -741,7 +748,7 @@ export class InterfaceManager extends DisposableObject {
const diagnostics: [Uri, ReadonlyArray<Diagnostic>][] = [];
for (const result of sarif.runs[0].results) {
for (const result of data.runs[0].results) {
const message = result.message.text;
if (message === undefined) {
void this.logger.log('Sarif had result without plaintext message');

24
extensions/ql-vscode/src/pure/interface-types.ts
View File

@@ -12,13 +12,14 @@ export const SELECT_TABLE_NAME = '#select';
export const ALERTS_TABLE_NAME = 'alerts';
export type RawTableResultSet = { t: 'RawResultSet' } & RawResultSet;
export type PathTableResultSet = {
t: 'SarifResultSet';
export type InterpretedResultSet<T> = {
t: 'InterpretedResultSet';
readonly schema: ResultSetSchema;
name: string;
} & Interpretation;
interpretation: InterpretationT<T>;
};
export type ResultSet = RawTableResultSet | PathTableResultSet;
export type ResultSet = RawTableResultSet | InterpretedResultSet<InterpretationData>;
/**
* Only ever show this many rows in a raw result table.
@@ -46,7 +47,14 @@ export interface PreviousExecution {
durationSeconds: number;
}
export interface Interpretation {
export type SarifInterpretationData = {
t: 'SarifInterpretationData';
} & sarif.Log;
// Add more interpretation data kinds when needed (e.g., graph data)
export type InterpretationData = SarifInterpretationData;
export interface InterpretationT<T> {
sourceLocationPrefix: string;
numTruncatedResults: number;
numTotalResults: number;
@@ -55,9 +63,11 @@ export interface Interpretation {
* they appear in the sarif file.
*/
sortState?: InterpretedResultsSortState;
sarif: sarif.Log;
data: T;
}
export type Interpretation = InterpretationT<InterpretationData>;
export interface ResultsPaths {
resultsPath: string;
interpretedResultsPath: string;
@@ -358,7 +368,7 @@ export function getDefaultResultSetName(
return [
ALERTS_TABLE_NAME,
SELECT_TABLE_NAME,
resultSetNames[0],
resultSetNames[0]
].filter((resultSetName) => resultSetNames.includes(resultSetName))[0];
}

17
extensions/ql-vscode/src/query-results.ts
View File

@@ -3,7 +3,6 @@ import { CancellationTokenSource, env } from 'vscode';
import { QueryWithResults, QueryEvaluationInfo } from './run-queries';
import * as messages from './pure/messages';
import * as cli from './cli';
import * as sarif from 'sarif';
import * as fs from 'fs-extra';
import * as path from 'path';
import {
@@ -11,7 +10,8 @@ import {
SortedResultSetInfo,
QueryMetadata,
InterpretedResultsSortState,
ResultsPaths
ResultsPaths,
SarifInterpretationData
} from './pure/interface-types';
import { QueryHistoryConfig } from './config';
import { DatabaseInfo } from './pure/interface-types';
@@ -148,19 +148,20 @@ export class CompletedQueryInfo implements QueryWithResults {
/**
* Call cli command to interpret results.
* Call cli command to interpret SARIF results.
*/
export async function interpretResults(
server: cli.CodeQLCliServer,
export async function interpretResultsSarif(
cli: cli.CodeQLCliServer,
metadata: QueryMetadata | undefined,
resultsPaths: ResultsPaths,
sourceInfo?: cli.SourceInfo
): Promise<sarif.Log> {
): Promise<SarifInterpretationData> {
const { resultsPath, interpretedResultsPath } = resultsPaths;
if (await fs.pathExists(interpretedResultsPath)) {
return JSON.parse(await fs.readFile(interpretedResultsPath, 'utf8'));
return { ...JSON.parse(await fs.readFile(interpretedResultsPath, 'utf8')), t: 'SarifInterpretationData' };
}
return await server.interpretBqrs(ensureMetadataIsComplete(metadata), resultsPath, interpretedResultsPath, sourceInfo);
const res = await cli.interpretBqrsSarif(ensureMetadataIsComplete(metadata), resultsPath, interpretedResultsPath, sourceInfo);
return { ...res, t: 'SarifInterpretationData' };
}
export function ensureMetadataIsComplete(metadata: QueryMetadata | undefined) {

18
extensions/ql-vscode/src/view/alert-table.tsx
View File

@@ -5,7 +5,7 @@ import * as Keys from '../pure/result-keys';
import * as octicons from './octicons';
import { className, renderLocation, ResultTableProps, zebraStripe, selectableZebraStripe, jumpToLocation, nextSortDirection, emptyQueryResultsMessage } from './result-table-utils';
import { onNavigation, NavigationEvent } from './results';
import { PathTableResultSet } from '../pure/interface-types';
import { InterpretedResultSet, SarifInterpretationData } from '../pure/interface-types';
import {
parseSarifPlainTextMessage,
parseSarifLocation,
@@ -15,7 +15,7 @@ import { InterpretedResultsSortColumn, SortDirection, InterpretedResultsSortStat
import { vscode } from './vscode-api';
import { isWholeFileLoc, isLineColumnLoc } from '../pure/bqrs-utils';
export type PathTableProps = ResultTableProps & { resultSet: PathTableResultSet };
export type PathTableProps = ResultTableProps & { resultSet: InterpretedResultSet<SarifInterpretationData> };
export interface PathTableState {
expanded: { [k: string]: boolean };
selectedPathNode: undefined | Keys.PathNode;
@@ -51,7 +51,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
}
sortClass(column: InterpretedResultsSortColumn): string {
const sortState = this.props.resultSet.sortState;
const sortState = this.props.resultSet.interpretation.sortState;
if (sortState !== undefined && sortState.sortBy === column) {
return sortState.sortDirection === SortDirection.asc ? 'sort-asc' : 'sort-desc';
}
@@ -61,7 +61,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
}
getNextSortState(column: InterpretedResultsSortColumn): InterpretedResultsSortState | undefined {
const oldSortState = this.props.resultSet.sortState;
const oldSortState = this.props.resultSet.interpretation.sortState;
const prevDirection = oldSortState && oldSortState.sortBy === column ? oldSortState.sortDirection : undefined;
const nextDirection = nextSortDirection(prevDirection, true);
return nextDirection === undefined ? undefined :
@@ -94,7 +94,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
</thead>;
const rows: JSX.Element[] = [];
const { numTruncatedResults, sourceLocationPrefix } = resultSet;
const { numTruncatedResults, sourceLocationPrefix } = resultSet.interpretation;
function renderRelatedLocations(msg: string, relatedLocations: Sarif.Location[]): JSX.Element[] {
const relatedLocationsById: { [k: string]: Sarif.Location } = {};
@@ -188,13 +188,13 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
return (e) => this.toggle(e, indices);
};
if (!resultSet.sarif.runs?.[0]?.results?.length) {
if (!resultSet.interpretation.data.runs?.[0]?.results?.length) {
return this.renderNoResults();
}
let expansionIndex = 0;
resultSet.sarif.runs[0].results.forEach((result, resultIndex) => {
resultSet.interpretation.data.runs[0].results.forEach((result, resultIndex) => {
const text = result.message.text || '[no text]';
const msg: JSX.Element[] =
result.relatedLocations === undefined ?
@@ -307,7 +307,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
const { selectedPathNode } = prevState;
if (selectedPathNode === undefined) return prevState;
const path = Keys.getPath(this.props.resultSet.sarif, selectedPathNode);
const path = Keys.getPath(this.props.resultSet.interpretation.data, selectedPathNode);
if (path === undefined) return prevState;
const nextIndex = selectedPathNode.pathNodeIndex + event.direction;
@@ -318,7 +318,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
return prevState;
}
const loc = parseSarifLocation(sarifLoc, this.props.resultSet.sourceLocationPrefix);
const loc = parseSarifLocation(sarifLoc, this.props.resultSet.interpretation.sourceLocationPrefix);
if (isNoLocation(loc)) {
return prevState;
}

32
extensions/ql-vscode/src/view/result-tables.tsx
View File

@@ -61,8 +61,8 @@ function getResultCount(resultSet: ResultSet): number {
switch (resultSet.t) {
case 'RawResultSet':
return resultSet.schema.rows;
case 'SarifResultSet':
return resultSet.numTotalResults;
case 'InterpretedResultSet':
return resultSet.interpretation.numTotalResults;
}
}
@@ -87,27 +87,32 @@ export class ResultTables
this.props.rawResultSets.map((rs) => ({ t: 'RawResultSet', ...rs }));
if (this.props.interpretation != undefined) {
const tableName = this.getInterpretedTableName();
resultSets.push({
t: 'SarifResultSet',
t: 'InterpretedResultSet',
// FIXME: The values of version, columns, tupleCount are
// unused stubs because a SarifResultSet schema isn't used the
// unused stubs because a InterpretedResultSet schema isn't used the
// same way as a RawResultSet. Probably should pull `name` field
// out.
schema: {
name: ALERTS_TABLE_NAME,
name: tableName,
rows: 1,
columns: []
},
name: ALERTS_TABLE_NAME,
...this.props.interpretation,
name: tableName,
interpretation: this.props.interpretation,
});
}
return resultSets;
}
private getInterpretedTableName(): string {
return ALERTS_TABLE_NAME;
}
private getResultSetNames(): string[] {
return this.props.interpretation
? this.props.parsedResultSets.resultSetNames.concat([ALERTS_TABLE_NAME])
? this.props.parsedResultSets.resultSetNames.concat([this.getInterpretedTableName()])
: this.props.parsedResultSets.resultSetNames;
}
@@ -349,8 +354,15 @@ class ResultTable extends React.Component<ResultTableProps, Record<string, never
switch (resultSet.t) {
case 'RawResultSet': return <RawTable
{...this.props} resultSet={resultSet} />;
case 'SarifResultSet': return <PathTable
{...this.props} resultSet={resultSet} />;
case 'InterpretedResultSet': {
const data = resultSet.interpretation.data;
switch (data.t) {
case 'SarifInterpretationData': {
const sarifResultSet = { ...resultSet, interpretation: { ...resultSet.interpretation, data } };
return <PathTable {...this.props} resultSet={sarifResultSet} />;
}
}
}
}
}
}

17
extensions/ql-vscode/src/view/results.tsx
View File

@@ -104,7 +104,9 @@ class App extends React.Component<Record<string, never>, ResultsViewState> {
void this.loadResults();
break;
case 'showInterpretedPage':
case 'showInterpretedPage': {
const tableName = ALERTS_TABLE_NAME;
this.updateStateWithNewResultsInfo({
resultsPath: '', // FIXME: Not used for interpreted, refactor so this is not needed
parsedResultSets: {
@@ -114,16 +116,16 @@ class App extends React.Component<Record<string, never>, ResultsViewState> {
resultSetNames: msg.resultSetNames,
pageNumber: msg.pageNumber,
resultSet: {
t: 'SarifResultSet',
name: ALERTS_TABLE_NAME,
t: 'InterpretedResultSet',
name: tableName,
schema: {
name: ALERTS_TABLE_NAME,
name: tableName,
rows: 1,
columns: []
},
...msg.interpretation,
interpretation: msg.interpretation,
},
selectedTable: ALERTS_TABLE_NAME,
selectedTable: tableName,
},
origResultsPaths: undefined as any, // FIXME: Not used for interpreted, refactor so this is not needed
sortedResultsMap: new Map(), // FIXME: Not used for interpreted, refactor so this is not needed
@@ -136,6 +138,7 @@ class App extends React.Component<Record<string, never>, ResultsViewState> {
});
void this.loadResults();
break;
}
case 'resultsUpdating':
this.setState({
isExpectingResultsUpdate: true,
@@ -191,7 +194,7 @@ class App extends React.Component<Record<string, never>, ResultsViewState> {
const resultSet = parsedResultSets.resultSet;
if (!resultSet.t) {
throw new Error(
'Missing result set type. Should be either "SarifResultSet" or "RawResultSet".'
'Missing result set type. Should be either "InterpretedResultSet" or "RawResultSet".'
);
}
return [resultSet];

22
extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts
View File

@@ -5,7 +5,7 @@ import 'mocha';
import 'sinon-chai';
import * as sinon from 'sinon';
import * as chaiAsPromised from 'chai-as-promised';
import { FullQueryInfo, InitialQueryInfo, interpretResults } from '../../query-results';
import { FullQueryInfo, InitialQueryInfo, interpretResultsSarif } from '../../query-results';
import { queriesDir, QueryEvaluationInfo, QueryWithResults, tmpDir } from '../../run-queries';
import { QueryHistoryConfig } from '../../config';
import { EvaluationResult, QueryResultType } from '../../pure/messages';
@@ -183,11 +183,11 @@ describe('query-results', () => {
});
});
it('should interpretResults', async () => {
it('should interpretResultsSarif', async () => {
const spy = sandbox.mock();
spy.returns('1234');
spy.returns({ a: '1234' });
const mockServer = {
interpretBqrs: spy
interpretBqrsSarif: spy
} as unknown as CodeQLCliServer;
const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json');
@@ -198,7 +198,7 @@ describe('query-results', () => {
id: 'my-id' as string | undefined,
scored: undefined
};
const results1 = await interpretResults(
const results1 = await interpretResultsSarif(
mockServer,
metadata,
{
@@ -207,7 +207,7 @@ describe('query-results', () => {
sourceInfo as SourceInfo
);
expect(results1).to.eq('1234');
expect(results1).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' });
expect(spy).to.have.been.calledWith(
metadata,
resultsPath, interpretedResultsPath, sourceInfo
@@ -215,9 +215,9 @@ describe('query-results', () => {
// Try again, but with no id
spy.reset();
spy.returns('1234');
spy.returns({a: '1234'});
delete metadata.id;
const results2 = await interpretResults(
const results2 = await interpretResultsSarif(
mockServer,
metadata,
{
@@ -225,7 +225,7 @@ describe('query-results', () => {
},
sourceInfo as SourceInfo
);
expect(results2).to.eq('1234');
expect(results2).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' });
expect(spy).to.have.been.calledWith(
{ kind: 'my-kind', id: 'dummy-id', scored: undefined },
resultsPath, interpretedResultsPath, sourceInfo
@@ -236,7 +236,7 @@ describe('query-results', () => {
fs.writeFileSync(interpretedResultsPath, JSON.stringify({
a: 6
}), 'utf8');
const results3 = await interpretResults(
const results3 = await interpretResultsSarif(
mockServer,
metadata,
{
@@ -244,7 +244,7 @@ describe('query-results', () => {
},
sourceInfo as SourceInfo
);
expect(results3).to.deep.eq({ a: 6 });
expect(results3).to.deep.eq({ a: 6, t: 'SarifInterpretationData' });
});
describe('splat and slurp', () => {