[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 <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>

pkg/js_ast/lib/src/nodes.dart

@@ -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 {

pkg/js_ast/lib/src/precedence.dart

@@ -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;

pkg/js_ast/lib/src/printer.dart

@@ -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(']');

pkg/js_ast/test/print_1_test.dart

@@ -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)');
+}

pkg/js_ast/test/print_exponentiation_test.dart

@@ -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)');
+}

pkg/js_ast/test/print_for_in_test.dart

@@ -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  ;',
+  );
+}

pkg/js_ast/test/print_helper.dart

@@ -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();
+}

pkg/js_ast/test/print_new_test.dart

@@ -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)');
+}