Skip to content

Commit 22d85dd

Browse files
adams85adams85
authored
Fix bug in the parenthesizing logic of ForStatement (#369)
1 parent 6954aeb commit 22d85dd

File tree

7 files changed

+366
-9
lines changed

7 files changed

+366
-9
lines changed

‎src/Esprima/Utils/AstToJavascriptConverter.Enums.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ protected internal enum ExpressionFlags
3838

3939
IsMethod = 1 << 16,
4040

41-
InOperatorIsAmbiguousInDeclaration = 1 << 24, // automatically propagated to sub-expressions
41+
IsInAmbiguousInOperatorContext = 1 << 24, // automatically propagated to sub-expressions
4242

4343
IsLeftMostInArrowFunctionBody = 1 << 25, // automatically combined and propagated to sub-expressions
4444
IsInsideArrowFunctionBody = 1 << 26, // automatically propagated to sub-expressions
@@ -52,6 +52,6 @@ protected internal enum ExpressionFlags
5252

5353
IsInsideStatementExpression = 1 << 31, // automatically propagated to sub-expressions
5454

55-
IsInPotentiallyAmbiguousContext = InOperatorIsAmbiguousInDeclaration | IsInsideArrowFunctionBody | IsInsideNewCallee | IsInsideLeftHandSideExpression | IsInsideStatementExpression,
55+
IsInPotentiallyAmbiguousContext = IsInAmbiguousInOperatorContext | IsInsideArrowFunctionBody | IsInsideNewCallee | IsInsideLeftHandSideExpression | IsInsideStatementExpression,
5656
}
5757
}

‎src/Esprima/Utils/AstToJavascriptConverter.Helpers.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ protected ExpressionFlags DisambiguateExpression(Expression expression, Expressi
137137
{
138138
if (flags.HasFlagFast(ExpressionFlags.NeedsBrackets))
139139
{
140-
return flags & ~ExpressionFlags.InOperatorIsAmbiguousInDeclaration;
140+
return flags & ~ExpressionFlags.IsInAmbiguousInOperatorContext;
141141
}
142142

143143
// Puts the left-most expression in brackets if necessary (in cases where it would be interpreted differently without brackets).
@@ -148,12 +148,12 @@ protected ExpressionFlags DisambiguateExpression(Expression expression, Expressi
148148
flags.HasFlagFast(ExpressionFlags.IsInsideArrowFunctionBody | ExpressionFlags.IsLeftMostInArrowFunctionBody) && ExpressionIsAmbiguousAsArrowFunctionBody(expression) ||
149149
flags.HasFlagFast(ExpressionFlags.IsInsideNewCallee | ExpressionFlags.IsLeftMostInNewCallee) && ExpressionIsAmbiguousAsNewCallee(expression))
150150
{
151-
return (flags | ExpressionFlags.NeedsBrackets) & ~ExpressionFlags.InOperatorIsAmbiguousInDeclaration;
151+
return (flags | ExpressionFlags.NeedsBrackets) & ~ExpressionFlags.IsInAmbiguousInOperatorContext;
152152
}
153153
// Edge case: for (var a = b = (c in d in e) in x);
154-
else if (flags.HasFlagFast(ExpressionFlags.InOperatorIsAmbiguousInDeclaration) && expression is BinaryExpression { Operator: BinaryOperator.In })
154+
else if (flags.HasFlagFast(ExpressionFlags.IsInAmbiguousInOperatorContext) && expression is BinaryExpression { Operator: BinaryOperator.In })
155155
{
156-
return (flags | ExpressionFlags.NeedsBrackets) & ~ExpressionFlags.InOperatorIsAmbiguousInDeclaration;
156+
return (flags | ExpressionFlags.NeedsBrackets) & ~ExpressionFlags.IsInAmbiguousInOperatorContext;
157157
}
158158
}
159159

‎src/Esprima/Utils/AstToJavascriptConverter.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ binaryExpression.Right is UnaryExpression rightUnaryExpression &&
514514

515515
VisitExpression(conditionalExpression.Consequent, SubExpressionFlags(operandNeedsBrackets, isLeftMost: false), static (@this, expression, flags) =>
516516
// Edge case: 'in' operators in for...in loop declarations are not ambigous when they are in the consequent part of the conditional expression.
517-
@this.DisambiguateExpression(expression, ~ExpressionFlags.InOperatorIsAmbiguousInDeclaration & @this.PropagateExpressionFlags(flags)));
517+
@this.DisambiguateExpression(expression, ~ExpressionFlags.IsInAmbiguousInOperatorContext & @this.PropagateExpressionFlags(flags)));
518518

519519
// Alternate expressions with the same precendence as ternary operator are unambiguous without brackets, even conditional expressions because of right-to-left associativity.
520520
operandNeedsBrackets = GetOperatorPrecedence(conditionalExpression, out _) > GetOperatorPrecedence(conditionalExpression.Alternate, out _);
@@ -812,7 +812,7 @@ binaryExpression.Right is UnaryExpression rightUnaryExpression &&
812812
}
813813
else
814814
{
815-
VisitRootExpression(forStatement.Init.As<Expression>(), RootExpressionFlags(needsBrackets: false));
815+
VisitRootExpression(forStatement.Init.As<Expression>(), ExpressionFlags.IsInAmbiguousInOperatorContext | RootExpressionFlags(needsBrackets: false));
816816
}
817817
}
818818

@@ -1747,7 +1747,7 @@ unaryExpression.Argument is UnaryExpression argumentUnaryExpression &&
17471747
}
17481748
else
17491749
{
1750-
VisitRootExpression(variableDeclarator.Init, ExpressionFlags.InOperatorIsAmbiguousInDeclaration | RootExpressionFlags(needsBrackets: ExpressionNeedsBracketsInList(variableDeclarator.Init)));
1750+
VisitRootExpression(variableDeclarator.Init, ExpressionFlags.IsInAmbiguousInOperatorContext | RootExpressionFlags(needsBrackets: ExpressionNeedsBracketsInList(variableDeclarator.Init)));
17511751
}
17521752
}
17531753

‎test/Esprima.Tests/Fixtures/statement/iteration/for-statement-with-in-decl-init.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
for (var x = ('p' in 'q' in {}); ;) { }

‎test/Esprima.Tests/Fixtures/statement/iteration/for-statement-with-in-decl-init.tree.json‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
{
2+
"type": "Program",
3+
"body": [
4+
{
5+
"type": "ForStatement",
6+
"init": {
7+
"type": "VariableDeclaration",
8+
"declarations": [
9+
{
10+
"type": "VariableDeclarator",
11+
"id": {
12+
"type": "Identifier",
13+
"name": "x",
14+
"range": [
15+
9,
16+
10
17+
],
18+
"loc": {
19+
"start": {
20+
"line": 1,
21+
"column": 9
22+
},
23+
"end": {
24+
"line": 1,
25+
"column": 10
26+
}
27+
}
28+
},
29+
"init": {
30+
"type": "BinaryExpression",
31+
"operator": "in",
32+
"left": {
33+
"type": "BinaryExpression",
34+
"operator": "in",
35+
"left": {
36+
"type": "Literal",
37+
"value": "p",
38+
"raw": "'p'",
39+
"range": [
40+
14,
41+
17
42+
],
43+
"loc": {
44+
"start": {
45+
"line": 1,
46+
"column": 14
47+
},
48+
"end": {
49+
"line": 1,
50+
"column": 17
51+
}
52+
}
53+
},
54+
"right": {
55+
"type": "Literal",
56+
"value": "q",
57+
"raw": "'q'",
58+
"range": [
59+
21,
60+
24
61+
],
62+
"loc": {
63+
"start": {
64+
"line": 1,
65+
"column": 21
66+
},
67+
"end": {
68+
"line": 1,
69+
"column": 24
70+
}
71+
}
72+
},
73+
"range": [
74+
14,
75+
24
76+
],
77+
"loc": {
78+
"start": {
79+
"line": 1,
80+
"column": 14
81+
},
82+
"end": {
83+
"line": 1,
84+
"column": 24
85+
}
86+
}
87+
},
88+
"right": {
89+
"type": "ObjectExpression",
90+
"properties": [],
91+
"range": [
92+
28,
93+
30
94+
],
95+
"loc": {
96+
"start": {
97+
"line": 1,
98+
"column": 28
99+
},
100+
"end": {
101+
"line": 1,
102+
"column": 30
103+
}
104+
}
105+
},
106+
"range": [
107+
14,
108+
30
109+
],
110+
"loc": {
111+
"start": {
112+
"line": 1,
113+
"column": 14
114+
},
115+
"end": {
116+
"line": 1,
117+
"column": 30
118+
}
119+
}
120+
},
121+
"range": [
122+
9,
123+
31
124+
],
125+
"loc": {
126+
"start": {
127+
"line": 1,
128+
"column": 9
129+
},
130+
"end": {
131+
"line": 1,
132+
"column": 31
133+
}
134+
}
135+
}
136+
],
137+
"kind": "var",
138+
"range": [
139+
5,
140+
31
141+
],
142+
"loc": {
143+
"start": {
144+
"line": 1,
145+
"column": 5
146+
},
147+
"end": {
148+
"line": 1,
149+
"column": 31
150+
}
151+
}
152+
},
153+
"test": null,
154+
"update": null,
155+
"body": {
156+
"type": "BlockStatement",
157+
"body": [],
158+
"range": [
159+
36,
160+
39
161+
],
162+
"loc": {
163+
"start": {
164+
"line": 1,
165+
"column": 36
166+
},
167+
"end": {
168+
"line": 1,
169+
"column": 39
170+
}
171+
}
172+
},
173+
"range": [
174+
0,
175+
39
176+
],
177+
"loc": {
178+
"start": {
179+
"line": 1,
180+
"column": 0
181+
},
182+
"end": {
183+
"line": 1,
184+
"column": 39
185+
}
186+
}
187+
}
188+
],
189+
"sourceType": "script",
190+
"strict": false,
191+
"range": [
192+
0,
193+
39
194+
],
195+
"loc": {
196+
"start": {
197+
"line": 1,
198+
"column": 0
199+
},
200+
"end": {
201+
"line": 1,
202+
"column": 39
203+
}
204+
}
205+
}

‎test/Esprima.Tests/Fixtures/statement/iteration/for-statement-with-in-expr-init.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
for (('p' in 'q' in {}); ;) { }

0 commit comments

Comments
 (0)