mirror of
https://github.com/dart-lang/sdk
synced 2024-10-04 17:04:56 +00:00
[js_ast] Fix js_ast printer bugs
* Parenthesize calls within `new`-expression targets to prevent the call arguments being associated with the `new`-expression. * Negation of exponentiation and negation as the left operand of exponentiation are syntax errors, so parenthesize the exponentiation or left operand. "-2 ** n" --> "(-2) ** n" or "-(2 ** n)" * "e in e" needs to be parenthesized in more places in a for-statement to avoid printing a for-in statement. The missing place was the right operand of a binary operator. "for (i = (0 in o) || 1 in o" --> "for (i = (0 in o) || (1 in o)" Bug: #54534 Bug: b/313833546 Bug: b/314023208 Bug: b/314041897 Change-Id: I12fcd75ac5546a425b21cd68a9bda3786ceb243d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347423 Commit-Queue: Stephen Adams <sra@google.com> Reviewed-by: Sigmund Cherem <sigmund@google.com>
This commit is contained in:
parent
af89e96b06
commit
cb16f98c0a
|
@ -1739,6 +1739,9 @@ class New extends Call {
|
|||
|
||||
@override
|
||||
New _clone() => New(target, arguments);
|
||||
|
||||
@override
|
||||
int get precedenceLevel => LEFT_HAND_SIDE;
|
||||
}
|
||||
|
||||
class Binary extends Expression {
|
||||
|
|
|
@ -18,6 +18,7 @@ const ADDITIVE = SHIFT + 1;
|
|||
const MULTIPLICATIVE = ADDITIVE + 1;
|
||||
const EXPONENTIATION = MULTIPLICATIVE + 1;
|
||||
const UNARY = EXPONENTIATION + 1;
|
||||
const CALL = UNARY + 1;
|
||||
const UPDATE = UNARY + 1;
|
||||
const CALL = UPDATE + 1;
|
||||
const LEFT_HAND_SIDE = CALL + 1;
|
||||
const PRIMARY = LEFT_HAND_SIDE + 1;
|
||||
|
|
|
@ -92,6 +92,37 @@ class Printer implements NodeVisitor<void> {
|
|||
int _charCount = 0;
|
||||
bool inForInit = false;
|
||||
bool atStatementBegin = false;
|
||||
|
||||
// The JavaScript grammar has two sets of related productions for property
|
||||
// accesses - for MemberExpression and CallExpression. A subset of
|
||||
// productions that illustrate the two sets:
|
||||
//
|
||||
// MemberExpression :
|
||||
// PrimaryExpression
|
||||
// MemberExpression . IdentifierName
|
||||
// new MemberExpression Arguments
|
||||
// ...
|
||||
//
|
||||
// CallExpression :
|
||||
// MemberExpression Arguments
|
||||
// CallExpression Arguments
|
||||
// CallExpression . IdentifierName
|
||||
// ...
|
||||
//
|
||||
// This means that a call can be in the 'function' part of another call, but
|
||||
// not in the 'function' part of a `new` expression. When printing a `new`
|
||||
// expression, a call in the 'function' part needs to be in parentheses to
|
||||
// ensure that the arguments of the call are not mistaken for the arguments of
|
||||
// the enclosing `new` expression.
|
||||
//
|
||||
// We handle the difference in required parenthesization by making the
|
||||
// required precedence of the receiver of an access be context-dependent.
|
||||
// Both "MemberExpression . IdentifierName" and "CallExpression
|
||||
// . IdentifierName" are represented as a PropertyAccess AST node. The context
|
||||
// is tracked by [inNewTarget], which is true only during the printing of
|
||||
// the 'function' part of a NewExpression.
|
||||
bool inNewTarget = false;
|
||||
|
||||
bool pendingSemicolon = false;
|
||||
bool pendingSpace = false;
|
||||
|
||||
|
@ -680,9 +711,11 @@ class Printer implements NodeVisitor<void> {
|
|||
(node is NamedFunction ||
|
||||
node is FunctionExpression ||
|
||||
node is ObjectInitializer));
|
||||
final savedInForInit = inForInit;
|
||||
if (needsParentheses) {
|
||||
inForInit = false;
|
||||
atStatementBegin = false;
|
||||
inNewTarget = false;
|
||||
out('(');
|
||||
visit(node);
|
||||
out(')');
|
||||
|
@ -691,6 +724,7 @@ class Printer implements NodeVisitor<void> {
|
|||
atStatementBegin = newAtStatementBegin;
|
||||
visit(node);
|
||||
}
|
||||
inForInit = savedInForInit;
|
||||
}
|
||||
|
||||
@override
|
||||
|
@ -857,12 +891,16 @@ class Printer implements NodeVisitor<void> {
|
|||
@override
|
||||
void visitNew(New node) {
|
||||
out('new ');
|
||||
final savedInNewTarget = inNewTarget;
|
||||
inNewTarget = true;
|
||||
visitNestedExpression(node.target, LEFT_HAND_SIDE,
|
||||
newInForInit: inForInit, newAtStatementBegin: false);
|
||||
out('(');
|
||||
inNewTarget = false;
|
||||
visitCommaSeparated(node.arguments, ASSIGNMENT,
|
||||
newInForInit: false, newAtStatementBegin: false);
|
||||
out(')');
|
||||
inNewTarget = savedInNewTarget;
|
||||
}
|
||||
|
||||
@override
|
||||
|
@ -955,9 +993,14 @@ class Printer implements NodeVisitor<void> {
|
|||
rightPrecedenceRequirement = UNARY;
|
||||
break;
|
||||
case '**':
|
||||
// 'a ** b ** c' parses as 'a ** (b ** c)', so the left must have higher
|
||||
// precedence.
|
||||
leftPrecedenceRequirement = UNARY;
|
||||
// Exponentiation associates to the right, so `a ** b ** c` parses as `a
|
||||
// ** (b ** c)`. To generate the appropriate output, the left has a
|
||||
// higher precedence than the current node. The next precedence level
|
||||
// ([UNARY]), is skipped as the left hand side of an exponentiation
|
||||
// operator [must be an UPDATE
|
||||
// expression](https://tc39.es/ecma262/#sec-exp-operator). Skipping
|
||||
// [UNARY] avoids printing `-1 ** 2`, which is a syntax error.
|
||||
leftPrecedenceRequirement = UPDATE;
|
||||
rightPrecedenceRequirement = EXPONENTIATION;
|
||||
break;
|
||||
default:
|
||||
|
@ -1068,7 +1111,8 @@ class Printer implements NodeVisitor<void> {
|
|||
|
||||
@override
|
||||
void visitAccess(PropertyAccess access) {
|
||||
visitNestedExpression(access.receiver, CALL,
|
||||
final precedence = inNewTarget ? LEFT_HAND_SIDE : CALL;
|
||||
visitNestedExpression(access.receiver, precedence,
|
||||
newInForInit: inForInit, newAtStatementBegin: atStatementBegin);
|
||||
|
||||
Node selector = _undefer(access.selector);
|
||||
|
@ -1093,6 +1137,7 @@ class Printer implements NodeVisitor<void> {
|
|||
}
|
||||
|
||||
out('[');
|
||||
inNewTarget = false;
|
||||
visitNestedExpression(access.selector, EXPRESSION,
|
||||
newInForInit: false, newAtStatementBegin: false);
|
||||
out(']');
|
||||
|
|
31
pkg/js_ast/test/print_1_test.dart
Normal file
31
pkg/js_ast/test/print_1_test.dart
Normal file
|
@ -0,0 +1,31 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:expect/expect.dart';
|
||||
import 'package:js_ast/js_ast.dart';
|
||||
import 'print_helper.dart';
|
||||
|
||||
void main() {
|
||||
// Basic precedence.
|
||||
final aPlus1 = testExpression('a + 1');
|
||||
final bTimes2 = testExpression('b * 2');
|
||||
|
||||
Expect.type<Binary>(aPlus1);
|
||||
Expect.type<Binary>(bTimes2);
|
||||
|
||||
testExpression('# + x', aPlus1, 'a + 1 + x');
|
||||
testExpression('x + #', aPlus1, 'x + (a + 1)');
|
||||
testExpression('# * x', aPlus1, '(a + 1) * x');
|
||||
testExpression('x * #', aPlus1, 'x * (a + 1)');
|
||||
|
||||
testExpression('# + x', bTimes2, 'b * 2 + x');
|
||||
testExpression('x + #', bTimes2, 'x + b * 2');
|
||||
testExpression('# * x', bTimes2, 'b * 2 * x');
|
||||
testExpression('x * #', bTimes2, 'x * (b * 2)');
|
||||
|
||||
testExpression('# + #', [aPlus1, aPlus1], 'a + 1 + (a + 1)');
|
||||
testExpression('# + #', [bTimes2, bTimes2], 'b * 2 + b * 2');
|
||||
testExpression('# * #', [aPlus1, aPlus1], '(a + 1) * (a + 1)');
|
||||
testExpression('# * #', [bTimes2, bTimes2], 'b * 2 * (b * 2)');
|
||||
}
|
36
pkg/js_ast/test/print_exponentiation_test.dart
Normal file
36
pkg/js_ast/test/print_exponentiation_test.dart
Normal file
|
@ -0,0 +1,36 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:js_ast/js_ast.dart';
|
||||
import 'print_helper.dart';
|
||||
|
||||
void main() {
|
||||
final aPowB = testExpression('a ** b');
|
||||
|
||||
testExpression('# ** c', aPowB, '(a ** b) ** c');
|
||||
testExpression('c ** #', aPowB, 'c ** a ** b');
|
||||
|
||||
// Miniparser parses with incorrect association:
|
||||
testExpression('a ** b ** c', '(a ** b) ** c');
|
||||
|
||||
testExpression('(a ** b) ** c', '(a ** b) ** c');
|
||||
testExpression('a ** (b ** c)', 'a ** b ** c');
|
||||
testExpression('a **= b');
|
||||
|
||||
// `-a**b` is a JavaScript parse error. Parentheses are required to
|
||||
// disambiguate between `(-a)**b` and `-(a**b)`.
|
||||
|
||||
testExpression('-(2 ** n)');
|
||||
|
||||
testExpression('(-(2)) ** n', '(-2) ** n');
|
||||
testExpression('(-2) ** n', '(-2) ** n');
|
||||
|
||||
final minus2 = js.number(-2);
|
||||
final negated2 = js('-#', js.number(2));
|
||||
|
||||
testExpression('# ** x', minus2, '(-2) ** x');
|
||||
testExpression('# ** x', negated2, '(-2) ** x');
|
||||
|
||||
testExpression('-(2 ** n)');
|
||||
}
|
48
pkg/js_ast/test/print_for_in_test.dart
Normal file
48
pkg/js_ast/test/print_for_in_test.dart
Normal file
|
@ -0,0 +1,48 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'print_helper.dart';
|
||||
|
||||
void main() {
|
||||
// The mini-parser does not recognize for-in statements, mis-parsing them as for-statements.
|
||||
testStatement(
|
||||
'for(a in b; a in b; a in b);',
|
||||
'for ((a in b); a in b; a in b)\n ;',
|
||||
);
|
||||
|
||||
final aInB = testExpression('a in b');
|
||||
|
||||
testStatement(
|
||||
'for(#;#;#);',
|
||||
[aInB, aInB, aInB],
|
||||
'for ((a in b); a in b; a in b)\n ;',
|
||||
);
|
||||
|
||||
testStatement(
|
||||
'for(var v = (# || #);;);',
|
||||
[aInB, aInB],
|
||||
'for (var v = (a in b) || (a in b);;)\n ;',
|
||||
);
|
||||
|
||||
testStatement(
|
||||
'for (u = (a + 1) * (b in z);;);',
|
||||
'for (u = (a + 1) * (b in z);;)\n ;',
|
||||
);
|
||||
|
||||
testStatement(
|
||||
'for (u = (a + 1) * #;;);',
|
||||
aInB,
|
||||
'for (u = (a + 1) * (a in b);;)\n ;',
|
||||
);
|
||||
|
||||
testStatement(
|
||||
'for (u = (1 + a) * 2, v = 1 || (b in z) || 2;;);',
|
||||
'for (u = (1 + a) * 2, v = 1 || (b in z) || 2;;)\n ;',
|
||||
);
|
||||
|
||||
testStatement(
|
||||
'for (var v in x);',
|
||||
'for (var v in x)\n ;',
|
||||
);
|
||||
}
|
63
pkg/js_ast/test/print_helper.dart
Normal file
63
pkg/js_ast/test/print_helper.dart
Normal file
|
@ -0,0 +1,63 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:expect/expect.dart';
|
||||
import 'package:js_ast/js_ast.dart';
|
||||
|
||||
/// Supports three calling patterns:
|
||||
///
|
||||
/// testExpression(template)
|
||||
/// testExpression(template, expected)
|
||||
/// testExpression(template, arguments, expected)
|
||||
///
|
||||
/// `template` is a String, possibly containing `#` placeholders.
|
||||
/// `arguments` can be a String, but only when `expected` is provided.
|
||||
Node testExpression(String expression, [optional1, String? optional2]) {
|
||||
return _test(js.call, expression, optional1, optional2);
|
||||
}
|
||||
|
||||
/// Supports three calling patterns:
|
||||
///
|
||||
/// testStatement(template)
|
||||
/// testStatement(template, expected)
|
||||
/// testStatement(template, arguments, expected)
|
||||
///
|
||||
/// `template` is a String, possibly containing `#` placeholders.
|
||||
/// `arguments` can be a String, but only when `expected` is provided.
|
||||
Node testStatement(String expression, [optional1, String? optional2]) {
|
||||
return _test(js.statement, expression, optional1, optional2);
|
||||
}
|
||||
|
||||
Node _test(Node Function(String, Object?) parse, String expression, optional1,
|
||||
String? optional2) {
|
||||
final String expected;
|
||||
final Object? arguments; // null, List, or Map.
|
||||
|
||||
if (optional2 is String) {
|
||||
expected = optional2;
|
||||
arguments = optional1 ?? const [];
|
||||
} else if (optional1 is String) {
|
||||
expected = optional1;
|
||||
arguments = const [];
|
||||
} else {
|
||||
expected = expression;
|
||||
arguments = optional1;
|
||||
}
|
||||
|
||||
Node node = parse(expression, arguments);
|
||||
String jsText = prettyPrint(node);
|
||||
Expect.stringEquals(expected.trim(), jsText.trim());
|
||||
return node;
|
||||
}
|
||||
|
||||
String prettyPrint(Node node) {
|
||||
JavaScriptPrintingOptions options = JavaScriptPrintingOptions(
|
||||
shouldCompressOutput: false,
|
||||
minifyLocalVariables: false,
|
||||
preferSemicolonToNewlineInMinifiedOutput: false);
|
||||
SimpleJavaScriptPrintingContext context = SimpleJavaScriptPrintingContext();
|
||||
Printer printer = Printer(options, context);
|
||||
printer.visit(node);
|
||||
return context.getText();
|
||||
}
|
41
pkg/js_ast/test/print_new_test.dart
Normal file
41
pkg/js_ast/test/print_new_test.dart
Normal file
|
@ -0,0 +1,41 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:expect/expect.dart';
|
||||
import 'package:js_ast/js_ast.dart';
|
||||
import 'print_helper.dart';
|
||||
|
||||
void main() {
|
||||
final propertyCall = testExpression('a.f(1)');
|
||||
Expect.type<Call>(propertyCall);
|
||||
|
||||
testExpression('#.g(2)', propertyCall, 'a.f(1).g(2)');
|
||||
|
||||
// Calls in the `new` target need to be parenthesized to prevent the call
|
||||
// arguments from being taken as the `new` arguments.
|
||||
testExpression('new #.a()', propertyCall, 'new (a.f(1)).a()');
|
||||
testExpression('new #(2)', testExpression('f(1)'), 'new (f(1))(2)');
|
||||
testExpression('new #(2)', testExpression('f(1).x'), 'new (f(1)).x(2)');
|
||||
testExpression('new #(2)', testExpression('f(1).x()'), 'new (f(1).x())(2)');
|
||||
|
||||
testExpression('new (f.x)()', 'new f.x()');
|
||||
testExpression('new (f().x)()', 'new (f()).x()'); // Also ok: `new (f().x)()`
|
||||
testExpression('new (f.x())()', 'new (f.x())()');
|
||||
|
||||
testExpression('(new f.x(1))(2)', 'new f.x(1)(2)');
|
||||
|
||||
testExpression('new (new f(g(1).x))(2)', 'new new f(g(1).x)(2)');
|
||||
|
||||
testExpression('new f[g(1).x](2)');
|
||||
testExpression('new (f()[g(1).x])(2)', 'new (f())[g(1).x](2)');
|
||||
testExpression('new (f[g(1).x])(2)', 'new f[g(1).x](2)');
|
||||
|
||||
// All the operators that have a second expression that is not protected (by
|
||||
// being inside an argument list or `[]` index) have lower priority than the
|
||||
// `new` MemberExpression, so require parentheses regardless of whether they
|
||||
// contain a call.
|
||||
testExpression('new (f || g)(1)');
|
||||
testExpression('new (f ** g)(3)');
|
||||
testExpression('new (f(1) || g(2))(3)');
|
||||
}
|
Loading…
Reference in a new issue