Skip to content

Commit ff03478

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request github#3049 from asger-semmle/js/fix-cyclic-join
Approved by erik-krogh
2 parents 20cae30 + 2bdf26a commit ff03478

File tree

8 files changed

+249
-69
lines changed

8 files changed

+249
-69
lines changed

‎javascript/extractor/lib/typescript/src/main.ts

Lines changed: 140 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,11 @@ interface PrepareFilesCommand {
6969
command: "prepare-files";
7070
filenames: string[];
7171
}
72+
interface GetMetadataCommand {
73+
command: "get-metadata";
74+
}
7275
type Command = ParseCommand | OpenProjectCommand | CloseProjectCommand
73-
| GetTypeTableCommand | ResetCommand | QuitCommand | PrepareFilesCommand;
76+
| GetTypeTableCommand | ResetCommand | QuitCommand | PrepareFilesCommand | GetMetadataCommand;
7477

7578
/** The state to be shared between commands. */
7679
class State {
@@ -91,7 +94,7 @@ const reloadMemoryThresholdMb = getEnvironmentVariable("SEMMLE_TYPESCRIPT_MEMORY
9194
/**
9295
* Debugging method for finding cycles in the TypeScript AST. Should not be used in production.
9396
*
94-
* If cycles are found, additional properties should be added to `isBlacklistedProperty`.
97+
* If cycles are found, the whitelist in `astProperties` is too permissive.
9598
*/
9699
// tslint:disable-next-line:no-unused-variable
97100
function checkCycle(root: any) {
@@ -104,7 +107,8 @@ function checkCycle(root: any) {
104107
obj.$cycle_visiting = true;
105108
for (let k in obj) {
106109
if (!obj.hasOwnProperty(k)) continue;
107-
if (isBlacklistedProperty(k)) continue;
110+
// Ignore numeric and whitelisted properties.
111+
if (+k !== +k && !astPropertySet.has(k)) continue;
108112
if (k === "$cycle_visiting") continue;
109113
let cycle = visit(obj[k]);
110114
if (cycle) {
@@ -122,30 +126,133 @@ function checkCycle(root: any) {
122126
}
123127
}
124128

125-
/**
126-
* A property that should not be serialized as part of the AST, because they
127-
* lead to cycles or are just not needed.
128-
*
129-
* Because of restrictions on `JSON.stringify`, these properties may also not
130-
* be used as part of a command response.
131-
*/
132-
function isBlacklistedProperty(k: string) {
133-
return k === "parent" || k === "pos" || k === "end"
134-
|| k === "symbol" || k === "localSymbol"
135-
|| k === "flowNode" || k === "returnFlowNode" || k === "endFlowNode" || k === "fallthroughFlowNode"
136-
|| k === "nextContainer" || k === "locals"
137-
|| k === "bindDiagnostics" || k === "bindSuggestionDiagnostics";
138-
}
129+
/** Property names to extract from the TypeScript AST. */
130+
const astProperties: string[] = [
131+
"$declarationKind",
132+
"$declaredSignature",
133+
"$end",
134+
"$lineStarts",
135+
"$overloadIndex",
136+
"$pos",
137+
"$resolvedSignature",
138+
"$symbol",
139+
"$tokens",
140+
"$type",
141+
"argument",
142+
"argumentExpression",
143+
"arguments",
144+
"assertsModifier",
145+
"asteriskToken",
146+
"attributes",
147+
"block",
148+
"body",
149+
"caseBlock",
150+
"catchClause",
151+
"checkType",
152+
"children",
153+
"clauses",
154+
"closingElement",
155+
"closingFragment",
156+
"condition",
157+
"constraint",
158+
"constructor",
159+
"declarationList",
160+
"declarations",
161+
"decorators",
162+
"default",
163+
"delete",
164+
"dotDotDotToken",
165+
"elements",
166+
"elementType",
167+
"elementTypes",
168+
"elseStatement",
169+
"escapedText",
170+
"exclamationToken",
171+
"exportClause",
172+
"expression",
173+
"exprName",
174+
"extendsType",
175+
"falseType",
176+
"finallyBlock",
177+
"flags",
178+
"head",
179+
"heritageClauses",
180+
"importClause",
181+
"incrementor",
182+
"indexType",
183+
"init",
184+
"initializer",
185+
"isExportEquals",
186+
"isTypeOf",
187+
"isTypeOnly",
188+
"keywordToken",
189+
"kind",
190+
"label",
191+
"left",
192+
"literal",
193+
"members",
194+
"messageText",
195+
"modifiers",
196+
"moduleReference",
197+
"moduleSpecifier",
198+
"name",
199+
"namedBindings",
200+
"objectType",
201+
"openingElement",
202+
"openingFragment",
203+
"operand",
204+
"operator",
205+
"operatorToken",
206+
"parameterName",
207+
"parameters",
208+
"parseDiagnostics",
209+
"properties",
210+
"propertyName",
211+
"qualifier",
212+
"questionDotToken",
213+
"questionToken",
214+
"right",
215+
"selfClosing",
216+
"statement",
217+
"statements",
218+
"tag",
219+
"tagName",
220+
"template",
221+
"templateSpans",
222+
"text",
223+
"thenStatement",
224+
"token",
225+
"tokenPos",
226+
"trueType",
227+
"tryBlock",
228+
"type",
229+
"typeArguments",
230+
"typeName",
231+
"typeParameter",
232+
"typeParameters",
233+
"types",
234+
"variableDeclaration",
235+
"whenFalse",
236+
"whenTrue",
237+
];
238+
239+
/** Property names used in a parse command response, in addition to the AST itself. */
240+
const astMetaProperties: string[] = [
241+
"ast",
242+
"type",
243+
];
244+
245+
/** Property names to extract in an AST response. */
246+
const astPropertySet = new Set([...astProperties, ...astMetaProperties]);
139247

140248
/**
141-
* Converts (part of) an AST to a JSON string, ignoring parent pointers.
249+
* Converts (part of) an AST to a JSON string, ignoring properties we're not interested in.
142250
*/
143251
function stringifyAST(obj: any) {
144252
return JSON.stringify(obj, (k, v) => {
145-
if (isBlacklistedProperty(k)) {
146-
return undefined;
147-
}
148-
return v;
253+
// Filter out properties that aren't numeric, empty, or whitelisted.
254+
// Note `k` is the empty string for the root object, which is also covered by +k === +k.
255+
return (+k === +k || astPropertySet.has(k)) ? v : undefined;
149256
});
150257
}
151258

@@ -155,8 +262,6 @@ function extractFile(filename: string): string {
155262
return stringifyAST({
156263
type: "ast",
157264
ast,
158-
nodeFlags: ts.NodeFlags,
159-
syntaxKinds: ts.SyntaxKind
160265
});
161266
}
162267

@@ -522,6 +627,14 @@ function handlePrepareFilesCommand(command: PrepareFilesCommand) {
522627
});
523628
}
524629

630+
function handleGetMetadataCommand(command: GetMetadataCommand) {
631+
console.log(JSON.stringify({
632+
type: "metadata",
633+
syntaxKinds: ts.SyntaxKind,
634+
nodeFlags: ts.NodeFlags,
635+
}));
636+
}
637+
525638
function reset() {
526639
state = new State();
527640
state.typeTable.restrictedExpansion = getEnvironmentVariable("SEMMLE_TYPESCRIPT_NO_EXPANSION", Boolean, true);
@@ -582,6 +695,9 @@ function runReadLineInterface() {
582695
case "reset":
583696
handleResetCommand(req);
584697
break;
698+
case "get-metadata":
699+
handleGetMetadataCommand(req);
700+
break;
585701
case "quit":
586702
rl.close();
587703
break;

‎javascript/extractor/src/com/semmle/js/parser/TypeScriptASTConverter.java

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22

33
import java.util.ArrayList;
44
import java.util.Collections;
5-
import java.util.LinkedHashMap;
65
import java.util.List;
7-
import java.util.Map;
86
import java.util.regex.Matcher;
97
import java.util.regex.Pattern;
108

@@ -163,10 +161,7 @@
163161
*/
164162
public class TypeScriptASTConverter {
165163
private String source;
166-
private final JsonObject nodeFlags;
167-
private final JsonObject syntaxKinds;
168-
private final Map<Integer, String> nodeFlagMap = new LinkedHashMap<>();
169-
private final Map<Integer, String> syntaxKindMap = new LinkedHashMap<>();
164+
private final TypeScriptParserMetadata metadata;
170165
private int[] lineStarts;
171166

172167
private int syntaxKindExtends;
@@ -180,22 +175,9 @@ public class TypeScriptASTConverter {
180175
private static final Pattern WHITESPACE_END_PAREN =
181176
Pattern.compile("^" + WHITESPACE_CHAR + "*\\)");
182177

183-
TypeScriptASTConverter(JsonObject nodeFlags, JsonObject syntaxKinds) {
184-
this.nodeFlags = nodeFlags;
185-
this.syntaxKinds = syntaxKinds;
186-
makeEnumIdMap(nodeFlags, nodeFlagMap);
187-
makeEnumIdMap(syntaxKinds, syntaxKindMap);
188-
this.syntaxKindExtends = getSyntaxKind("ExtendsKeyword");
189-
}
190-
191-
/** Builds a mapping from ID to name given a TypeScript enum object. */
192-
private void makeEnumIdMap(JsonObject enumObject, Map<Integer, String> idToName) {
193-
for (Map.Entry<String, JsonElement> entry : enumObject.entrySet()) {
194-
JsonPrimitive prim = entry.getValue().getAsJsonPrimitive();
195-
if (prim.isNumber() && !idToName.containsKey(prim.getAsInt())) {
196-
idToName.put(prim.getAsInt(), entry.getKey());
197-
}
198-
}
178+
TypeScriptASTConverter(TypeScriptParserMetadata metadata) {
179+
this.metadata = metadata;
180+
this.syntaxKindExtends = metadata.getSyntaxKindId("ExtendsKeyword");
199181
}
200182

201183
/**
@@ -1617,7 +1599,7 @@ private Node convertMappedType(JsonObject node, SourceLocation loc) throws Parse
16171599
private Node convertMetaProperty(JsonObject node, SourceLocation loc) throws ParseError {
16181600
Position metaStart = loc.getStart();
16191601
String keywordKind =
1620-
syntaxKinds.get(node.getAsJsonPrimitive("keywordToken").getAsInt() + "").getAsString();
1602+
metadata.getSyntaxKindName(node.getAsJsonPrimitive("keywordToken").getAsInt());
16211603
String identifier = keywordKind.equals("ImportKeyword") ? "import" : "new";
16221604
Position metaEnd =
16231605
new Position(
@@ -1995,7 +1977,7 @@ private Node convertPrefixUnaryExpression(JsonObject node, SourceLocation loc) t
19951977

19961978
private String getOperator(JsonObject node) throws ParseError {
19971979
int operatorId = node.get("operator").getAsInt();
1998-
switch (syntaxKindMap.get(operatorId)) {
1980+
switch (metadata.getSyntaxKindName(operatorId)) {
19991981
case "PlusPlusToken":
20001982
return "++";
20011983
case "MinusMinusToken":
@@ -2219,7 +2201,7 @@ private Node convertTypeOfExpression(JsonObject node, SourceLocation loc) throws
22192201
}
22202202

22212203
private Node convertTypeOperator(JsonObject node, SourceLocation loc) throws ParseError {
2222-
String operator = syntaxKinds.get("" + node.get("operator").getAsInt()).getAsString();
2204+
String operator = metadata.getSyntaxKindName(node.get("operator").getAsInt());
22232205
if (operator.equals("KeyOfKeyword")) {
22242206
return new UnaryTypeExpr(loc, UnaryTypeExpr.Kind.Keyof, convertChildAsType(node, "type"));
22252207
}
@@ -2537,29 +2519,14 @@ private int getMemberModifierKeywords(JsonObject node) {
25372519
* <tt>ts.NodeFlags</tt> in enum.
25382520
*/
25392521
private boolean hasFlag(JsonObject node, String flagName) {
2540-
JsonElement flagDescriptor = this.nodeFlags.get(flagName);
2541-
if (flagDescriptor == null) {
2542-
throw new RuntimeException(
2543-
"Incompatible version of TypeScript installed. Missing node flag " + flagName);
2544-
}
2545-
int flagId = flagDescriptor.getAsInt();
2522+
int flagId = metadata.getNodeFlagId(flagName);
25462523
JsonElement flags = node.get("flags");
25472524
if (flags instanceof JsonPrimitive) {
25482525
return (flags.getAsInt() & flagId) != 0;
25492526
}
25502527
return false;
25512528
}
25522529

2553-
/** Gets the numeric value of the syntax kind enum with the given name. */
2554-
private int getSyntaxKind(String syntaxKind) {
2555-
JsonElement descriptor = this.syntaxKinds.get(syntaxKind);
2556-
if (descriptor == null) {
2557-
throw new RuntimeException(
2558-
"Incompatible version of TypeScript installed. Missing syntax kind " + syntaxKind);
2559-
}
2560-
return descriptor.getAsInt();
2561-
}
2562-
25632530
/** Check whether a node has a child with a given name. */
25642531
private boolean hasChild(JsonObject node, String prop) {
25652532
if (!node.has(prop)) return false;
@@ -2581,7 +2548,7 @@ private String getKind(JsonElement node) {
25812548
if (node instanceof JsonObject) {
25822549
JsonElement kind = ((JsonObject) node).get("kind");
25832550
if (kind instanceof JsonPrimitive && ((JsonPrimitive) kind).isNumber())
2584-
return syntaxKindMap.get(kind.getAsInt());
2551+
return metadata.getSyntaxKindName(kind.getAsInt());
25852552
}
25862553
return null;
25872554
}

‎javascript/extractor/src/com/semmle/js/parser/TypeScriptParser.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ public class TypeScriptParser {
150150
/** If non-zero, we use this instead of relying on the corresponding environment variable. */
151151
private int typescriptRam = 0;
152152

153+
/** Metadata requested immediately after starting the TypeScript parser. */
154+
private TypeScriptParserMetadata metadata;
155+
153156
/** Sets the amount of RAM to allocate to the TypeScript compiler.s */
154157
public void setTypescriptRam(int megabytes) {
155158
this.typescriptRam = megabytes;
@@ -297,6 +300,7 @@ private void setupParserWrapper() {
297300
InputStream is = parserWrapperProcess.getInputStream();
298301
InputStreamReader isr = new InputStreamReader(is, "UTF-8");
299302
fromParserWrapper = new BufferedReader(isr);
303+
this.loadMetadata();
300304
} catch (IOException e) {
301305
throw new CatastrophicError(
302306
"Could not start TypeScript parser wrapper " + "(command: ." + parserWrapperCommand + ")",
@@ -385,6 +389,17 @@ private RuntimeException getExceptionFromMalformedResponse(String response, Exce
385389
return new CatastrophicError("Unexpected response from TypeScript parser wrapper:\n" + response, e);
386390
}
387391

392+
/**
393+
* Requests metadata from the TypeScript process. See {@link TypeScriptParserMetadata}.
394+
*/
395+
private void loadMetadata() {
396+
JsonObject request = new JsonObject();
397+
request.add("command", new JsonPrimitive("get-metadata"));
398+
JsonObject response = talkToParserWrapper(request);
399+
checkResponseType(response, "metadata");
400+
this.metadata = new TypeScriptParserMetadata(response);
401+
}
402+
388403
/**
389404
* Returns the AST for a given source file.
390405
*
@@ -402,11 +417,9 @@ public Result parse(File sourceFile, String source, ExtractionMetrics metrics) {
402417
metrics.stopPhase(ExtractionMetrics.ExtractionPhase.TypeScriptParser_talkToParserWrapper);
403418
try {
404419
checkResponseType(response, "ast");
405-
JsonObject nodeFlags = response.get("nodeFlags").getAsJsonObject();
406-
JsonObject syntaxKinds = response.get("syntaxKinds").getAsJsonObject();
407420
JsonObject ast = response.get("ast").getAsJsonObject();
408421
metrics.startPhase(ExtractionMetrics.ExtractionPhase.TypeScriptASTConverter_convertAST);
409-
Result converted = new TypeScriptASTConverter(nodeFlags, syntaxKinds).convertAST(ast, source);
422+
Result converted = new TypeScriptASTConverter(metadata).convertAST(ast, source);
410423
metrics.stopPhase(ExtractionMetrics.ExtractionPhase.TypeScriptASTConverter_convertAST);
411424
return converted;
412425
} catch (IllegalStateException e) {

0 commit comments

Comments
 (0)