Skip to content

Commit 9391d36

Browse files
author
Max Schaefer
committed
JavaScript: Teach extractor to tolerate assignment patterns in AST.
Our database representation of ASTs does not use assignment patterns, instead encoding the relevant information directly in the associated function/loop/assignment. We convert from an AST with assignment patterns to one without during parsing, so the extractor does not expect any assignment patterns to be present in the AST. Due to a bug in the parser, this can currently happen for malformed programs. While we should fix that bug once it gets fixed in Acorn, it also makes sense for the extractor to be more robust, so this PR teaches the `ASTExtractor` pass to raise a parse error when it encounters an assignment pattern, and all other passes to simply ignore them.
1 parent f6029bd commit 9391d36

File tree

6 files changed

+144
-11
lines changed

6 files changed

+144
-11
lines changed

javascript/extractor/src/com/semmle/js/ast/DefaultVisitor.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,11 @@ public R visit(AssignmentExpression nd, C c) {
9797
}
9898

9999
@Override
100-
public R visit(AssignmentPattern nd, C q) {
101-
throw new CatastrophicError("Assignment patterns should not appear in the AST.");
100+
public R visit(AssignmentPattern nd, C c) {
101+
// assignment patterns should not appear in the AST, but can do for malformed
102+
// programs; the ASTExtractor raises a ParseError in this case, other visitors
103+
// should just ignore them
104+
return visit(nd.getLeft(), c);
102105
}
103106

104107
@Override

javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
package com.semmle.js.extractor;
22

3+
import java.util.ArrayList;
4+
import java.util.Collections;
5+
import java.util.List;
6+
import java.util.Set;
7+
import java.util.Stack;
8+
39
import com.semmle.js.ast.AClass;
410
import com.semmle.js.ast.AFunction;
511
import com.semmle.js.ast.AFunctionExpression;
612
import com.semmle.js.ast.ArrayExpression;
713
import com.semmle.js.ast.ArrayPattern;
814
import com.semmle.js.ast.ArrowFunctionExpression;
915
import com.semmle.js.ast.AssignmentExpression;
16+
import com.semmle.js.ast.AssignmentPattern;
1017
import com.semmle.js.ast.AwaitExpression;
1118
import com.semmle.js.ast.BinaryExpression;
1219
import com.semmle.js.ast.BindExpression;
@@ -103,6 +110,7 @@
103110
import com.semmle.js.extractor.ExtractorConfig.SourceType;
104111
import com.semmle.js.extractor.ScopeManager.DeclKind;
105112
import com.semmle.js.extractor.ScopeManager.Scope;
113+
import com.semmle.js.parser.ParseError;
106114
import com.semmle.ts.ast.ArrayTypeExpr;
107115
import com.semmle.ts.ast.ConditionalTypeExpr;
108116
import com.semmle.ts.ast.DecoratorList;
@@ -144,11 +152,6 @@
144152
import com.semmle.util.collections.CollectionUtil;
145153
import com.semmle.util.trap.TrapWriter;
146154
import com.semmle.util.trap.TrapWriter.Label;
147-
import java.util.ArrayList;
148-
import java.util.Collections;
149-
import java.util.List;
150-
import java.util.Set;
151-
import java.util.Stack;
152155

153156
/** Extractor for AST-based information; invoked by the {@link JSExtractor}. */
154157
public class ASTExtractor {
@@ -307,6 +310,7 @@ private class V extends DefaultVisitor<Context, Label> {
307310
private final Platform platform;
308311
private final SourceType sourceType;
309312
private boolean isStrict;
313+
private List<ParseError> additionalErrors = new ArrayList<>();
310314

311315
public V(Platform platform, SourceType sourceType) {
312316
this.platform = platform;
@@ -2054,14 +2058,22 @@ public Label visit(XMLDotDotExpression nd, Context c) {
20542058
visit(nd.getRight(), key, 1, IdContext.label);
20552059
return key;
20562060
}
2061+
2062+
@Override
2063+
public Label visit(AssignmentPattern nd, Context c) {
2064+
additionalErrors.add(new ParseError("Unexpected assignment pattern.", nd.getLoc().getStart()));
2065+
return super.visit(nd, c);
2066+
}
20572067
}
20582068

2059-
public void extract(Node root, Platform platform, SourceType sourceType, int toplevelKind) {
2069+
public List<ParseError> extract(Node root, Platform platform, SourceType sourceType, int toplevelKind) {
20602070
lexicalExtractor.getMetrics().startPhase(ExtractionPhase.ASTExtractor_extract);
20612071
trapwriter.addTuple("toplevels", toplevelLabel, toplevelKind);
20622072
locationManager.emitNodeLocation(root, toplevelLabel);
20632073

2064-
root.accept(new V(platform, sourceType), null);
2074+
V visitor = new V(platform, sourceType);
2075+
root.accept(visitor, null);
20652076
lexicalExtractor.getMetrics().stopPhase(ExtractionPhase.ASTExtractor_extract);
2077+
return visitor.additionalErrors;
20662078
}
20672079
}

javascript/extractor/src/com/semmle/js/extractor/JSExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public Pair<Label, LoCInfo> extract(
113113
new JSDocExtractor(textualExtractor).extract(lexicalExtractor.getComments());
114114
lexicalExtractor.purge();
115115

116-
scriptExtractor.extract(ast, platform, sourceType, toplevelKind);
116+
parserRes.getErrors().addAll(scriptExtractor.extract(ast, platform, sourceType, toplevelKind));
117117
new CFGExtractor(scriptExtractor).extract(ast);
118118
} else {
119119
lexicalExtractor =

javascript/extractor/src/com/semmle/js/extractor/Main.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class Main {
3737
* A version identifier that should be updated every time the extractor changes in such a way that
3838
* it may produce different tuples for the same file under the same {@link ExtractorConfig}.
3939
*/
40-
public static final String EXTRACTOR_VERSION = "2019-11-26";
40+
public static final String EXTRACTOR_VERSION = "2020-01-06";
4141

4242
public static final Pattern NEWLINE = Pattern.compile("\n");
4343

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
(x = 0) = y
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
#10000=@"/invalid-assignment-pattern.js;sourcefile"
2+
files(#10000,"/invalid-assignment-pattern.js","invalid-assignment-pattern","js",0)
3+
#10001=@"/;folder"
4+
folders(#10001,"/","")
5+
containerparent(#10001,#10000)
6+
#10002=@"loc,{#10000},0,0,0,0"
7+
locations_default(#10002,#10000,0,0,0,0)
8+
hasLocation(#10000,#10002)
9+
#20000=@"global_scope"
10+
scopes(#20000,0)
11+
#20001=@"script;{#10000},1,1"
12+
#20002=*
13+
lines(#20002,#20001,"(x = 0) = y","
14+
")
15+
#20003=@"loc,{#10000},1,1,1,11"
16+
locations_default(#20003,#10000,1,1,1,11)
17+
hasLocation(#20002,#20003)
18+
numlines(#20001,1,1,0)
19+
#20004=*
20+
tokeninfo(#20004,8,#20001,0,"(")
21+
#20005=@"loc,{#10000},1,1,1,1"
22+
locations_default(#20005,#10000,1,1,1,1)
23+
hasLocation(#20004,#20005)
24+
#20006=*
25+
tokeninfo(#20006,6,#20001,1,"x")
26+
#20007=@"loc,{#10000},1,2,1,2"
27+
locations_default(#20007,#10000,1,2,1,2)
28+
hasLocation(#20006,#20007)
29+
#20008=*
30+
tokeninfo(#20008,8,#20001,2,"=")
31+
#20009=@"loc,{#10000},1,4,1,4"
32+
locations_default(#20009,#10000,1,4,1,4)
33+
hasLocation(#20008,#20009)
34+
#20010=*
35+
tokeninfo(#20010,3,#20001,3,"0")
36+
#20011=@"loc,{#10000},1,6,1,6"
37+
locations_default(#20011,#10000,1,6,1,6)
38+
hasLocation(#20010,#20011)
39+
#20012=*
40+
tokeninfo(#20012,8,#20001,4,")")
41+
#20013=@"loc,{#10000},1,7,1,7"
42+
locations_default(#20013,#10000,1,7,1,7)
43+
hasLocation(#20012,#20013)
44+
#20014=*
45+
tokeninfo(#20014,8,#20001,5,"=")
46+
#20015=@"loc,{#10000},1,9,1,9"
47+
locations_default(#20015,#10000,1,9,1,9)
48+
hasLocation(#20014,#20015)
49+
#20016=*
50+
tokeninfo(#20016,6,#20001,6,"y")
51+
#20017=@"loc,{#10000},1,11,1,11"
52+
locations_default(#20017,#10000,1,11,1,11)
53+
hasLocation(#20016,#20017)
54+
#20018=*
55+
tokeninfo(#20018,0,#20001,7,"")
56+
#20019=@"loc,{#10000},2,1,2,0"
57+
locations_default(#20019,#10000,2,1,2,0)
58+
hasLocation(#20018,#20019)
59+
toplevels(#20001,0)
60+
#20020=@"loc,{#10000},1,1,2,0"
61+
locations_default(#20020,#10000,1,1,2,0)
62+
hasLocation(#20001,#20020)
63+
#20021=*
64+
stmts(#20021,2,#20001,0,"(x = 0) = y")
65+
hasLocation(#20021,#20003)
66+
stmtContainers(#20021,#20001)
67+
#20022=*
68+
exprs(#20022,47,#20021,0,"(x = 0) = y")
69+
hasLocation(#20022,#20003)
70+
enclosingStmt(#20022,#20021)
71+
exprContainers(#20022,#20001)
72+
#20023=*
73+
exprs(#20023,63,#20022,0,"(x = 0)")
74+
#20024=@"loc,{#10000},1,1,1,7"
75+
locations_default(#20024,#10000,1,1,1,7)
76+
hasLocation(#20023,#20024)
77+
enclosingStmt(#20023,#20021)
78+
exprContainers(#20023,#20001)
79+
#20025=*
80+
exprs(#20025,79,#20023,0,"x")
81+
hasLocation(#20025,#20007)
82+
enclosingStmt(#20025,#20021)
83+
exprContainers(#20025,#20001)
84+
#20026=*
85+
exprs(#20026,79,#20022,1,"y")
86+
hasLocation(#20026,#20017)
87+
enclosingStmt(#20026,#20021)
88+
exprContainers(#20026,#20001)
89+
literals("y","y",#20026)
90+
#20027=@"var;{y};{#20000}"
91+
variables(#20027,"y",#20000)
92+
bind(#20026,#20027)
93+
#20028=*
94+
entry_cfg_node(#20028,#20001)
95+
#20029=@"loc,{#10000},1,1,1,0"
96+
locations_default(#20029,#10000,1,1,1,0)
97+
hasLocation(#20028,#20029)
98+
#20030=*
99+
exit_cfg_node(#20030,#20001)
100+
hasLocation(#20030,#20019)
101+
successor(#20021,#20023)
102+
successor(#20026,#20022)
103+
successor(#20023,#20025)
104+
successor(#20025,#20026)
105+
successor(#20022,#20030)
106+
successor(#20028,#20021)
107+
#20031=*
108+
jsParseErrors(#20031,#20001,"Error: Unexpected assignment pattern.","(x = 0) = y
109+
")
110+
hasLocation(#20031,#20007)
111+
#20032=*
112+
lines(#20032,#20001,"(x = 0) = y","
113+
")
114+
hasLocation(#20032,#20003)
115+
numlines(#20001,1,0,0)
116+
numlines(#10000,1,1,0)
117+
filetype(#10000,"javascript")

0 commit comments

Comments
 (0)