Skip to content

Commit 0045891

Browse files
aeisenbergaeisenberg
committed
Clean up ast builder code
1 parent 65b5b68 commit 0045891

File tree

5 files changed

+100
-61
lines changed

5 files changed

+100
-61
lines changed

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ export interface TestCompleted {
9393
expected: string;
9494
}
9595

96+
/**
97+
* Optional arguments for the `bqrsDecode` function
98+
*/
99+
interface BqrsDecodeOptions {
100+
/** How many results to get. */
101+
pageSize?: number;
102+
/** The 0-based index of the first result to get. */
103+
offset?: number;
104+
/** The entity names to retrieve from the bqrs file. Default is url, string */
105+
entities?: string[];
106+
}
107+
96108
/**
97109
* This class manages a cli server started by `codeql execute cli-server` to
98110
* run commands without the overhead of starting a new java
@@ -494,12 +506,16 @@ export class CodeQLCliServer implements Disposable {
494506
* Gets the results from a bqrs.
495507
* @param bqrsPath The path to the bqrs.
496508
* @param resultSet The result set to get.
497-
* @param pageSize How many results to get.
498-
* @param offset The 0-based index of the first result to get.
509+
* @param options Optional BqrsDecodeOptions arguments
499510
*/
500-
async bqrsDecode(bqrsPath: string, resultSet: string, pageSize?: number, offset?: number): Promise<DecodedBqrsChunk> {
511+
async bqrsDecode(
512+
bqrsPath: string,
513+
resultSet: string,
514+
{ pageSize, offset, entities = ['url', 'string'] }: BqrsDecodeOptions = {}
515+
): Promise<DecodedBqrsChunk> {
516+
501517
const subcommandArgs = [
502-
'--entities=id,url,string',
518+
`--entities=${entities.join(',')}`,
503519
'--result-set', resultSet,
504520
].concat(
505521
pageSize ? ['--rows', pageSize.toString()] : []

‎extensions/ql-vscode/src/contextual/astBuilder.ts‎

Lines changed: 60 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { AstItem, RootAstItem } from '../astViewer';
88
* A class that wraps a tree of QL results from a query that
99
* has an @kind of graph
1010
*/
11-
// RENAME to ASTParser / ASTCreator
1211
export default class AstBuilder {
1312

1413
private roots: RootAstItem[] | undefined;
@@ -30,10 +29,11 @@ export default class AstBuilder {
3029
}
3130

3231
private async parseRoots(): Promise<RootAstItem[]> {
32+
const options = { entities: ['id', 'url', 'string'] };
3333
const [nodeTuples, edgeTuples, graphProperties] = await Promise.all([
34-
await this.cli.bqrsDecode(this.bqrsPath, 'nodes'),
35-
await this.cli.bqrsDecode(this.bqrsPath, 'edges'),
36-
await this.cli.bqrsDecode(this.bqrsPath, 'graphProperties'),
34+
await this.cli.bqrsDecode(this.bqrsPath, 'nodes', options),
35+
await this.cli.bqrsDecode(this.bqrsPath, 'edges', options),
36+
await this.cli.bqrsDecode(this.bqrsPath, 'graphProperties', options),
3737
]);
3838

3939
if (!this.isValidGraph(graphProperties)) {
@@ -48,61 +48,75 @@ export default class AstBuilder {
4848

4949
// Build up the parent-child relationships
5050
edgeTuples.tuples.forEach(tuple => {
51-
const from = tuple[0] as EntityValue;
52-
const to = tuple[1] as EntityValue;
53-
const toId = to.id!;
54-
const fromId = from.id!;
55-
56-
if (tuple[2] === 'semmle.order') {
57-
astOrder.set(toId, Number(tuple[3]));
58-
} else if (tuple[2] === 'semmle.label') {
59-
childToParent.set(toId, fromId);
60-
let children = parentToChildren.get(fromId);
61-
if (!children) {
62-
parentToChildren.set(fromId, children = []);
51+
const [source, target, tupleType, orderValue] = tuple as [EntityValue, EntityValue, string, string];
52+
const sourceId = source.id!;
53+
const targetId = target.id!;
54+
55+
switch (tupleType) {
56+
case 'semmle.order':
57+
astOrder.set(targetId, Number(orderValue));
58+
break;
59+
60+
case 'semmle.label': {
61+
childToParent.set(targetId, sourceId);
62+
let children = parentToChildren.get(sourceId);
63+
if (!children) {
64+
parentToChildren.set(sourceId, children = []);
65+
}
66+
children.push(targetId);
67+
break;
6368
}
64-
children.push(toId);
69+
70+
default:
71+
// ignore other tupleTypes since they are not needed by the ast viewer
6572
}
6673
});
6774

6875
// populate parents and children
6976
nodeTuples.tuples.forEach(tuple => {
70-
const entity = tuple[0] as EntityValue;
77+
const [entity, tupleType, orderValue] = tuple as [EntityValue, string, string];
7178
const id = entity.id!;
7279

73-
if (tuple[1] === 'semmle.order') {
74-
astOrder.set(id, Number(tuple[2]));
75-
76-
} else if (tuple[1] === 'semmle.label') {
77-
const item = {
78-
id,
79-
label: entity.label,
80-
location: entity.url,
81-
children: [] as AstItem[],
82-
order: Number.MAX_SAFE_INTEGER
83-
};
84-
85-
idToItem.set(id, item as RootAstItem);
86-
const parent = idToItem.get(childToParent.get(id) || -1);
87-
88-
if (parent) {
89-
const astItem = item as AstItem;
90-
astItem.parent = parent;
91-
parent.children.push(astItem);
92-
}
93-
const children = parentToChildren.get(id) || [];
94-
children.forEach(childId => {
95-
const child = idToItem.get(childId) as AstItem | undefined;
96-
if (child) {
97-
child.parent = item;
98-
item.children.push(child);
80+
switch (tupleType) {
81+
case 'semmle.order':
82+
astOrder.set(id, Number(orderValue));
83+
break;
84+
85+
case 'semmle.label': {
86+
const item = {
87+
id,
88+
label: entity.label,
89+
location: entity.url,
90+
children: [] as AstItem[],
91+
order: Number.MAX_SAFE_INTEGER
92+
};
93+
94+
idToItem.set(id, item as RootAstItem);
95+
const parent = idToItem.get(childToParent.get(id) || -1);
96+
97+
if (parent) {
98+
const astItem = item as AstItem;
99+
astItem.parent = parent;
100+
parent.children.push(astItem);
99101
}
100-
});
102+
const children = parentToChildren.get(id) || [];
103+
children.forEach(childId => {
104+
const child = idToItem.get(childId) as AstItem | undefined;
105+
if (child) {
106+
child.parent = item;
107+
item.children.push(child);
108+
}
109+
});
110+
break;
111+
}
112+
113+
default:
114+
// ignore other tupleTypes since they are not needed by the ast viewer
101115
}
102116
});
103117

104118
// find the roots and add the order
105-
for(const [, item] of idToItem) {
119+
for (const [, item] of idToItem) {
106120
item.order = astOrder.has(item.id)
107121
? astOrder.get(item.id)!
108122
: Number.MAX_SAFE_INTEGER;

‎extensions/ql-vscode/src/contextual/definitions.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,16 @@ async function getLinksFromResults(
257257
// TODO: Page this
258258
const allTuples = await cli.bqrsDecode(bqrsPath, SELECT_QUERY_NAME);
259259
for (const tuple of allTuples.tuples) {
260-
const src = tuple[0] as EntityValue;
261-
const dest = tuple[1] as EntityValue;
260+
const [src, dest] = tuple as [EntityValue, EntityValue];
262261
const srcFile = src.url && fileRangeFromURI(src.url, db);
263262
const destFile = dest.url && fileRangeFromURI(dest.url, db);
264263
if (srcFile && destFile && filter(srcFile.uri.toString(), destFile.uri.toString())) {
265-
localLinks.push({ targetRange: destFile.range, targetUri: destFile.uri, originSelectionRange: srcFile.range, originUri: srcFile.uri });
264+
localLinks.push({
265+
targetRange: destFile.range,
266+
targetUri: destFile.uri,
267+
originSelectionRange: srcFile.range,
268+
originUri: srcFile.uri
269+
});
266270
}
267271
}
268272
}

‎extensions/ql-vscode/src/interface.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,10 @@ export class InterfaceManager extends DisposableObject {
348348
const chunk = await this.cliServer.bqrsDecode(
349349
results.query.resultsPaths.resultsPath,
350350
schema.name,
351-
RAW_RESULTS_PAGE_SIZE,
352-
schema.pagination?.offsets[0]
351+
{
352+
offset: schema.pagination?.offsets[0],
353+
pageSize: RAW_RESULTS_PAGE_SIZE
354+
}
353355
);
354356
const adaptedSchema = adaptSchema(schema);
355357
const resultSet = adaptBqrs(adaptedSchema, chunk);
@@ -446,8 +448,10 @@ export class InterfaceManager extends DisposableObject {
446448
const chunk = await this.cliServer.bqrsDecode(
447449
results.query.resultsPaths.resultsPath,
448450
schema.name,
449-
RAW_RESULTS_PAGE_SIZE,
450-
schema.pagination?.offsets[pageNumber]
451+
{
452+
offset: schema.pagination?.offsets[pageNumber],
453+
pageSize: RAW_RESULTS_PAGE_SIZE
454+
}
451455
);
452456
const adaptedSchema = adaptSchema(schema);
453457
const resultSet = adaptBqrs(adaptedSchema, chunk);

‎extensions/ql-vscode/src/vscode-tests/no-workspace/contextual/astBuilder.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import * as fs from 'fs-extra';
2-
// import * as sinonChai from 'sinon-chai';
32
import * as chai from 'chai';
43
import * as chaiAsPromised from 'chai-as-promised';
54
import * as sinon from 'sinon';
5+
66
import AstBuilder from '../../../contextual/astBuilder';
77
import { QueryWithResults } from '../../../run-queries';
88
import { CodeQLCliServer } from '../../../cli';
@@ -55,9 +55,10 @@ describe('AstBuilder', () => {
5555
const astBuilder = createAstBuilder();
5656
const roots = await astBuilder.getRoots();
5757

58-
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'nodes');
59-
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'edges');
60-
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'graphProperties');
58+
const options = { entities: ['id', 'url', 'string'] };
59+
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'nodes', options);
60+
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'edges', options);
61+
expect(mockCli.bqrsDecode).to.have.been.calledWith('/a/b/c', 'graphProperties', options);
6162

6263
expect(roots.map(
6364
r => ({ ...r, children: undefined })

0 commit comments

Comments
 (0)