From 41f8c4932ce153ad613d354430e802c2f375d836 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 00:07:27 -0700
Subject: [PATCH 01/12] test/parens: Expose (most) individual test cases to
 Mocha

This makes it a lot easier to see what's going on when test cases
fail, because we can now see which ones passed and which failed in
any given group.

We leave the "Arithmetic" group as a single test, because it
contains a very large number of test cases.
---
 test/parens.ts | 54 ++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/test/parens.ts b/test/parens.ts
index dd20af5f..47df6f30 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -15,7 +15,7 @@ function parseExpression(expr: any) {
   return n.ExpressionStatement.check(ast) ? ast.expression : ast;
 }
 
-function check(expr: any) {
+function check_(expr: string) {
   const ast = parse(expr);
   const reprinted = printer.print(ast).code;
   assert.strictEqual(reprinted, expr);
@@ -25,6 +25,12 @@ function check(expr: any) {
   types.astNodesAreEquivalent.assert(expressionAst, parseExpression(generic));
 }
 
+function check(expr: string) {
+  it(`handles: ${expr}`, () => {
+    check_(expr);
+  });
+}
+
 const operators = [
   "==",
   "!=",
@@ -53,18 +59,18 @@ const operators = [
 
 describe("parens", function () {
   it("Arithmetic", function () {
-    check("1 - 2");
-    check("  2 +2 ");
+    check_("1 - 2");
+    check_("  2 +2 ");
 
     operators.forEach(function (op1) {
       operators.forEach(function (op2) {
-        check("(a " + op1 + " b) " + op2 + " c");
-        check("a " + op1 + " (b " + op2 + " c)");
+        check_("(a " + op1 + " b) " + op2 + " c");
+        check_("a " + op1 + " (b " + op2 + " c)");
       });
     });
   });
 
-  it("Unary", function () {
+  describe("Unary", function () {
     check("(-a).b");
     check("(+a).b");
     check("(!a).b");
@@ -74,14 +80,14 @@ describe("parens", function () {
     check("(delete a.b).c");
   });
 
-  it("Binary", function () {
+  describe("Binary", function () {
     check("(a && b)()");
     check("typeof (a && b)");
     check("(a && b)[c]");
     check("(a && b).c");
   });
 
-  it("Sequence", function () {
+  describe("Sequence", function () {
     check("(a, b)()");
     check("a(b, (c, d), e)");
     check("!(a, b)");
@@ -94,7 +100,7 @@ describe("parens", function () {
     check("a = (1, 2)");
   });
 
-  it("NewExpression", function () {
+  describe("NewExpression", function () {
     check("new (a.b())");
     check("new (a.b())(c)");
     check("new a.b(c)");
@@ -107,7 +113,7 @@ describe("parens", function () {
     check('(new Date)["getTime"]()');
   });
 
-  it("Numbers", function () {
+  describe("Numbers", function () {
     check("(1).foo");
     check("(-1).foo");
     check("+0");
@@ -115,7 +121,7 @@ describe("parens", function () {
     check("(-Infinity).foo");
   });
 
-  it("Assign", function () {
+  describe("Assign", function () {
     check("!(a = false)");
     check("a + (b = 2) + c");
     check("(a = fn)()");
@@ -124,7 +130,7 @@ describe("parens", function () {
     check("(a = b).c");
   });
 
-  it("Function", function () {
+  describe("Function", function () {
     check("a(function (){}.bind(this))");
     check("(function (){}).apply(this, arguments)");
     check("function f() { (function (){}).call(this) }");
@@ -134,12 +140,12 @@ describe("parens", function () {
     check("a || ((x, y={z:1}) => x + y.z)");
   });
 
-  it("ObjectLiteral", function () {
+  describe("ObjectLiteral", function () {
     check("a({b:c(d)}.b)");
     check("({a:b(c)}).a");
   });
 
-  it("ArrowFunctionExpression", () => {
+  describe("ArrowFunctionExpression", () => {
     check("(() => {})()");
     check("test(() => {})");
 
@@ -149,7 +155,7 @@ describe("parens", function () {
     check("(() => {}) + (() => {})");
   });
 
-  it("AwaitExpression", function () {
+  describe("AwaitExpression", function () {
     check("async () => (await a) && (await b)");
     check("async () => +(await a)");
     check("async () => (await f)()");
@@ -159,7 +165,7 @@ describe("parens", function () {
     check("async () => (await a).b");
   });
 
-  it("YieldExpression", function () {
+  describe("YieldExpression", function () {
     check("function* test () { return (yield a) && (yield b) }");
     check("function* test () { return +(yield a) }");
     check("function* test () { return (yield f)() }");
@@ -170,7 +176,7 @@ describe("parens", function () {
     check("function* test () { yield yield foo }");
   });
 
-  it("ArrowFunctionExpression", () => {
+  describe("ArrowFunctionExpression", () => {
     check("(() => {})()");
     check("test(() => {})");
 
@@ -333,16 +339,16 @@ describe("parens", function () {
     assert.strictEqual(printer.print(ast).code, code);
   });
 
-  it("should be added to callees that are function expressions", function () {
+  describe("should be added to callees that are function expressions", function () {
     check("(()=>{})()");
     check("(function (){})()");
   });
 
-  it("should be added to function expressions at start of sequence", function () {
+  describe("should be added to function expressions at start of sequence", function () {
     check("(function (){})(), 0");
   });
 
-  it("issues #504 and #512", function () {
+  describe("issues #504 and #512", function () {
     check("() => ({})['foo']");
     check("() => ({ foo: 123 }[foo] + 2) * 3");
     check("() => ({ foo: 123 }['foo'] + 1 - 2 - 10)");
@@ -350,11 +356,11 @@ describe("parens", function () {
     check("() => (function () { return 456 }())");
   });
 
-  it("should be added to bound arrow function expressions", function () {
+  describe("should be added to bound arrow function expressions", function () {
     check("(()=>{}).bind(x)");
   });
 
-  it("should be added to object destructuring assignment expressions", function () {
+  describe("should be added to object destructuring assignment expressions", function () {
     check("({x}={x:1})");
     // Issue #533
     check("({ foo } = bar)");
@@ -362,7 +368,7 @@ describe("parens", function () {
 
   it("regression test for issue #327", function () {
     const expr = "(function(){}())";
-    check(expr);
+    check_(expr);
 
     const ast = parse(expr);
     const callExpression = ast.program.body[0].expression;
@@ -379,7 +385,7 @@ describe("parens", function () {
 
   it("regression test for issue #366", function () {
     const code = "typeof a ? b : c";
-    check(code);
+    check_(code);
 
     const ast = parse(code);
     const exprStmt = ast.program.body[0];

From 348d646fc35b5dac3d25f6a00f340bf3702fccc6 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 12:26:27 -0700
Subject: [PATCH 02/12] test/parens: More-informative failure messages

Printing the entire source code for each side of the comparison,
rather than just the first node path where they differ, makes it
easier to see what went wrong in printing.
---
 test/parens.ts | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/test/parens.ts b/test/parens.ts
index 47df6f30..49b7f32e 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -22,7 +22,14 @@ function check_(expr: string) {
 
   const expressionAst = parseExpression(expr);
   const generic = printer.printGenerically(expressionAst).code;
-  types.astNodesAreEquivalent.assert(expressionAst, parseExpression(generic));
+
+  if (!types.astNodesAreEquivalent(expressionAst, parseExpression(generic))) {
+    throw new assert.AssertionError({
+      message: "Expected values to parse to equivalent ASTs",
+      actual: generic,
+      expected: expr,
+    });
+  }
 }
 
 function check(expr: string) {

From 6ace97115bc430a0ed4b9cc4aaba489defc80566 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 12:53:39 -0700
Subject: [PATCH 03/12] parens: Greatly simplify hasParens

I don't quite see why the justNeedsOpeningParen logic would be
necessary.  If having just an opening parenthesis is good enough for
those nodes (and indeed it seems like it is), then it seems like it
should be good enough for any node.  I.e., we could treat
justNeedsOpeningParen as always true.

And if we do so, all the tests still pass.

So I'm now pretty confident that we can in fact cut that logic out.
This code gets quite a bit simpler when we do, so that's nice!

Then in particular, cutting out this pair of calls to firstInStatement
and canBeFirstInStatement leaves just one remaining call site for each
of them.  That makes things a lot simpler for us when we go to make
changes to how those functions work -- which we're about to do, in
order to fix some bugs.
---
 lib/fast-path.ts | 48 +++---------------------------------------------
 1 file changed, 3 insertions(+), 45 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index 925cbaac..f79f9a36 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -225,54 +225,12 @@ FPp.map = function map(callback /*, name1, name2, ... */) {
   return result;
 };
 
-// Returns true if the node at the tip of the path is wrapped with
-// parentheses, OR if the only reason the node needed parentheses was that
-// it couldn't be the first expression in the enclosing statement (see
-// FastPath#canBeFirstInStatement), and it has an opening `(` character.
-// For example, the FunctionExpression in `(function(){}())` appears to
-// need parentheses only because it's the first expression in the AST, but
-// since it happens to be preceded by a `(` (which is not apparent from
-// the AST but can be determined using FastPath#getPrevToken), there is no
-// ambiguity about how to parse it, so it counts as having parentheses,
-// even though it is not immediately followed by a `)`.
+// Returns true if the node at the tip of the path is preceded by an
+// open-paren token `(`.
 FPp.hasParens = function () {
   const node = this.getNode();
-
   const prevToken = this.getPrevToken(node);
-  if (!prevToken) {
-    return false;
-  }
-
-  const nextToken = this.getNextToken(node);
-  if (!nextToken) {
-    return false;
-  }
-
-  if (prevToken.value === "(") {
-    if (nextToken.value === ")") {
-      // If the node preceded by a `(` token and followed by a `)` token,
-      // then of course it has parentheses.
-      return true;
-    }
-
-    // If this is one of the few Expression types that can't come first in
-    // the enclosing statement because of parsing ambiguities (namely,
-    // FunctionExpression, ObjectExpression, and ClassExpression) and
-    // this.firstInStatement() returns true, and the node would not need
-    // parentheses in an expression context because this.needsParens(true)
-    // returns false, then it just needs an opening parenthesis to resolve
-    // the parsing ambiguity that made it appear to need parentheses.
-    const justNeedsOpeningParen =
-      !this.canBeFirstInStatement() &&
-      this.firstInStatement() &&
-      !this.needsParens(true);
-
-    if (justNeedsOpeningParen) {
-      return true;
-    }
-  }
-
-  return false;
+  return prevToken && prevToken.value === "(";
 };
 
 FPp.getPrevToken = function (node) {

From 2bd9d2668806f79b2188f0b003ca369629a41609 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 12:56:26 -0700
Subject: [PATCH 04/12] parens [nfc]: Cut now-disused assumeExpressionContext
 flag

We just cut the only call site that passed this flag, in hasParens.
---
 lib/fast-path.ts | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index f79f9a36..e620f79c 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -41,7 +41,7 @@ interface FastPathType {
   hasParens(): any;
   getPrevToken(node: any): any;
   getNextToken(node: any): any;
-  needsParens(assumeExpressionContext?: boolean): any;
+  needsParens(): any;
   canBeFirstInStatement(): any;
   firstInStatement(): any;
 }
@@ -269,7 +269,7 @@ FPp.getNextToken = function (node) {
 
 // Inspired by require("ast-types").NodePath.prototype.needsParens, but
 // more efficient because we're iterating backwards through a stack.
-FPp.needsParens = function (assumeExpressionContext) {
+FPp.needsParens = function () {
   const node = this.getNode();
 
   // This needs to come before `if (!parent) { return false }` because
@@ -499,11 +499,7 @@ FPp.needsParens = function (assumeExpressionContext) {
     return containsCallExpression(node);
   }
 
-  if (
-    assumeExpressionContext !== true &&
-    !this.canBeFirstInStatement() &&
-    this.firstInStatement()
-  ) {
+  if (!this.canBeFirstInStatement() && this.firstInStatement()) {
     return true;
   }
 

From 785e9bdcaf30d1e234378712eb9fbda65481cbf2 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 15:18:06 -0700
Subject: [PATCH 05/12] test/parens [nfc]: Cut duplicate
 ArrowFunctionExpression cases

There's another copy of these a few lines above.
---
 test/parens.ts | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/test/parens.ts b/test/parens.ts
index 49b7f32e..39a1b9b2 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -183,16 +183,6 @@ describe("parens", function () {
     check("function* test () { yield yield foo }");
   });
 
-  describe("ArrowFunctionExpression", () => {
-    check("(() => {})()");
-    check("test(() => {})");
-
-    check("(() => {}).test");
-    check("test[() => {}]");
-
-    check("(() => {}) + (() => {})");
-  });
-
   it("ReprintedParens", function () {
     const code = "a(function g(){}.call(this));";
     const ast1 = parse(code);

From b285ce30fa9274436ed0d7805abfb1e557fa069a Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 15:20:19 -0700
Subject: [PATCH 06/12] test/parens: Add test for ClassExpression
 first-in-statement case

Previously, no tests would fail if you commented out the check for
ClassExpression in canBeFirstInStatement.  As these new test cases
show, that check is indeed necessary.
---
 test/parens.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/parens.ts b/test/parens.ts
index 39a1b9b2..506ae590 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -183,6 +183,11 @@ describe("parens", function () {
     check("function* test () { yield yield foo }");
   });
 
+  describe("ClassExpression", function () {
+    check("(class { static f() { return 3; } }).f()");
+    check("const x = class { static f() { return 3; } }.f()");
+  });
+
   it("ReprintedParens", function () {
     const code = "a(function g(){}.call(this));";
     const ast1 = parse(code);

From 6138fc493c74c383c62f3724fd868fe0af91f0f5 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 14:22:19 -0700
Subject: [PATCH 07/12] parens [nfc]: Simplify firstInStatement a bit

As it was, if there was a non-`n.Node` value in the middle of the
stack (is that even possible?), then we'd recheck the same parent
and child as we did on the last iteration.  That can't do anything
useful; instead, just have each iteration always check either the
current parent and child, or nothing.
---
 lib/fast-path.ts | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index e620f79c..b859caf5 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -563,13 +563,14 @@ FPp.firstInStatement = function () {
   let childName, child;
 
   for (let i = s.length - 1; i >= 0; i -= 2) {
-    if (n.Node.check(s[i])) {
-      childName = parentName;
-      child = parent;
-      parentName = s[i - 1];
-      parent = s[i];
+    if (!n.Node.check(s[i])) {
+      continue;
     }
 
+    childName = parentName;
+    child = parent;
+    parentName = s[i - 1];
+    parent = s[i];
     if (!parent || !child) {
       continue;
     }

From ec2e3a3e6415f211be6af97473e409f34f5f0369 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 14:48:51 -0700
Subject: [PATCH 08/12] parens [nfc]: Cut impossible BlockStatement case from
 firstInStatement

To get here, the child would have to already be a statement,
so we'd have stopped at the previous step.

All tests still pass.  In fact, all tests still pass if instead
of deleting this one replaces the body with `assert.fail()`.
So that adds empirical evidence that this case is impossible.
---
 lib/fast-path.ts | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index b859caf5..3dda017f 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -575,15 +575,6 @@ FPp.firstInStatement = function () {
       continue;
     }
 
-    if (
-      n.BlockStatement.check(parent) &&
-      parentName === "body" &&
-      childName === 0
-    ) {
-      assert.strictEqual(parent.body[0], child);
-      return true;
-    }
-
     if (n.ExpressionStatement.check(parent) && childName === "expression") {
       assert.strictEqual(parent.expression, child);
       return true;

From 3c795aee9091aae13ff669471e4fbc3c70168c0f Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 14:51:06 -0700
Subject: [PATCH 09/12] parens [nfc]: Handle assignments just like other exprs
 in firstInStatement

These are fundamentally in exactly the same situation as a
CallExpression, a BinaryExpression, etc.
---
 lib/fast-path.ts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index 3dda017f..53d7469a 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -580,16 +580,16 @@ FPp.firstInStatement = function () {
       return true;
     }
 
-    if (n.AssignmentExpression.check(parent) && childName === "left") {
-      assert.strictEqual(parent.left, child);
-      return true;
-    }
-
     if (n.ArrowFunctionExpression.check(parent) && childName === "body") {
       assert.strictEqual(parent.body, child);
       return true;
     }
 
+    if (n.AssignmentExpression.check(parent) && childName === "left") {
+      assert.strictEqual(parent.left, child);
+      continue;
+    }
+
     // s[i + 1] and s[i + 2] represent the array between the parent
     // SequenceExpression node and its child nodes
     if (

From bde3724f814bd82f36dddbcdf8fb0be0f39dfd23 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 14:42:53 -0700
Subject: [PATCH 10/12] parens [nfc]: Inline canBeFirstInStatement at only call
 site

We're going to change how this works, to fix some bugs.
Putting the logic here will simplify that.
---
 lib/fast-path.ts | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index 53d7469a..5be687ba 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -42,7 +42,6 @@ interface FastPathType {
   getPrevToken(node: any): any;
   getNextToken(node: any): any;
   needsParens(): any;
-  canBeFirstInStatement(): any;
   firstInStatement(): any;
 }
 
@@ -499,11 +498,14 @@ FPp.needsParens = function () {
     return containsCallExpression(node);
   }
 
-  if (!this.canBeFirstInStatement() && this.firstInStatement()) {
-    return true;
+  switch (node.type) {
+    case "FunctionExpression":
+    case "ObjectExpression":
+    case "ClassExpression":
+      return this.firstInStatement();
+    default:
+      return false;
   }
-
-  return false;
 };
 
 function isBinary(node: any) {
@@ -539,24 +541,6 @@ function containsCallExpression(node: any): any {
   return false;
 }
 
-FPp.canBeFirstInStatement = function () {
-  const node = this.getNode();
-
-  if (n.FunctionExpression.check(node)) {
-    return false;
-  }
-
-  if (n.ObjectExpression.check(node)) {
-    return false;
-  }
-
-  if (n.ClassExpression.check(node)) {
-    return false;
-  }
-
-  return true;
-};
-
 FPp.firstInStatement = function () {
   const s = this.stack;
   let parentName, parent;

From d83b22eff4b5f7fac0d1a68d83818e1283df7555 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 15:01:43 -0700
Subject: [PATCH 11/12] parens: Fix arrow-function bodies getting excess parens

We were inserting parentheses unnecessarily in the bodies of some
arrow functions.  In particular, `() => function(){}` would become
`() => (function(){})`, and `() => class {}.foo.bar` would become
`() => (class {}).foo.bar`.

The cause turns out to be that we were taking the logic that
applies if you have an *expression statement* starting with a
FunctionExpression or ClassExpression -- which does indeed need
parens, to avoid getting parsed as a FunctionDeclaration or
ClassDeclaration respectively -- and applying it the same way if
one of those expressions is instead at the start of a braceless
arrow-function body, aka an ExpressionBody.

In fact, while an ExpressionBody does call for similar intervention
if it starts with a `{` token (so with an ObjectExpression node),
that is the only case where it does.

In the ES spec, this is expressed as lookahead constraints, on the
one hand at ExpressionStatement:
  https://tc39.es/ecma262/#prod-ExpressionStatement

and on the other hand at the two productions where ExpressionBody
appears:
  https://tc39.es/ecma262/#prod-ConciseBody
  https://tc39.es/ecma262/#prod-AsyncConciseBody

Adjust the logic accordingly to distinguish the two situations,
and add tests.

The ExpressionStatement lookahead constraints also point at one
more edge case which we don't yet correctly handle.  We'll fix that
in the next commit for the sake of making the logic comprehensive,
along with comments explaining how it corresponds to the spec.

Fixes: #914
Fixes: #1082
---
 lib/fast-path.ts | 20 +++++++++++++++-----
 test/parens.ts   | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index 5be687ba..4179ac7e 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -42,7 +42,10 @@ interface FastPathType {
   getPrevToken(node: any): any;
   getNextToken(node: any): any;
   needsParens(): any;
-  firstInStatement(): any;
+  firstInExpressionStatement(): boolean;
+  firstInExpressionStatementOrExpressionBody(
+    onlyExpressionStatement?: boolean,
+  ): boolean;
 }
 
 interface FastPathConstructor {
@@ -499,10 +502,11 @@ FPp.needsParens = function () {
   }
 
   switch (node.type) {
-    case "FunctionExpression":
     case "ObjectExpression":
+      return this.firstInExpressionStatementOrExpressionBody();
+    case "FunctionExpression":
     case "ClassExpression":
-      return this.firstInStatement();
+      return this.firstInExpressionStatement();
     default:
       return false;
   }
@@ -541,7 +545,13 @@ function containsCallExpression(node: any): any {
   return false;
 }
 
-FPp.firstInStatement = function () {
+FPp.firstInExpressionStatement = function () {
+  return this.firstInExpressionStatementOrExpressionBody(true);
+};
+
+FPp.firstInExpressionStatementOrExpressionBody = function (
+  onlyExpressionStatement: boolean = false,
+) {
   const s = this.stack;
   let parentName, parent;
   let childName, child;
@@ -566,7 +576,7 @@ FPp.firstInStatement = function () {
 
     if (n.ArrowFunctionExpression.check(parent) && childName === "body") {
       assert.strictEqual(parent.body, child);
-      return true;
+      return !onlyExpressionStatement;
     }
 
     if (n.AssignmentExpression.check(parent) && childName === "left") {
diff --git a/test/parens.ts b/test/parens.ts
index 506ae590..10c3623b 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -188,6 +188,25 @@ describe("parens", function () {
     check("const x = class { static f() { return 3; } }.f()");
   });
 
+  describe("ExpressionBody", () => {
+    // We need parens if an ExpressionBody starts with an ObjectExpression:
+    check("() => ({ x: 1 })"); // ExpressionBody, with an object literal
+    check("() => { x: 1 }"); // FunctionBody, with statement labelled "x"
+
+    // But we don't for other kinds of nodes -- even some that *would* need
+    // parens if they were the start of an ExpressionStatement.
+
+    // Issue #914: FunctionExpression at start of ExpressionBody
+    check("const e = () => function(g, h) { return i; };");
+    check("() => function(){}");
+    check("() => function(){}.call(this)");
+
+    // Issue #1082: ClassExpression at start of ExpressionBody
+    check("const D = (C) => class extends C {};");
+    check("() => class {}");
+    check("() => class {}.prototype");
+  });
+
   it("ReprintedParens", function () {
     const code = "a(function g(){}.call(this));";
     const ast1 = parse(code);

From 8a5584a08c38b87af6d316eb48b082bbbb00ad99 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Sat, 11 Jun 2022 16:40:18 -0700
Subject: [PATCH 12/12] parens: Fix on `let [`; nail down first-in-statement
 logic to spec

This is a fun edge case I discovered from reading the ES spec,
in particular here:
  https://tc39.es/ecma262/#prod-ExpressionStatement

while working out whether there were cases we were missing in this
logic, beyond the three we had.

I believe this turns out to be the only such case.  Add comments
explaining why, with references to the language spec.
---
 lib/fast-path.ts | 33 +++++++++++++++++++++++++++++++++
 test/parens.ts   | 19 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/lib/fast-path.ts b/lib/fast-path.ts
index 4179ac7e..621d9648 100644
--- a/lib/fast-path.ts
+++ b/lib/fast-path.ts
@@ -501,13 +501,46 @@ FPp.needsParens = function () {
     return containsCallExpression(node);
   }
 
+  // The ExpressionStatement production, and the two productions that
+  // contain ExpressionBody, have lookahead constraints that forbid some
+  // possibilities for their next node or two:
+  //   https://tc39.es/ecma262/#prod-ExpressionStatement
+  //   https://tc39.es/ecma262/#prod-ConciseBody
+  //   https://tc39.es/ecma262/#prod-AsyncConciseBody
+  //
+  // The effect of these is that if we have an expression that appears in
+  // one of those and would start with a forbidden token sequence, we need
+  // to insert parens so that the first token is `(` instead.
+  //
+  // We choose to do this on the smallest subexpression we can.
   switch (node.type) {
     case "ObjectExpression":
+      // Will start with `{`.  Therefore can't be the start of an
+      // ExpressionStatement or (either use of) an ExpressionBody.
       return this.firstInExpressionStatementOrExpressionBody();
+
     case "FunctionExpression":
     case "ClassExpression":
+      // Will start with the token `function`, tokens `async function`, or
+      // token `class`.  Therefore can't start an ExpressionStatement.
       return this.firstInExpressionStatement();
+
+    case "MemberExpression":
+      if (
+        n.Identifier.check(node.object) &&
+        node.object.name === "let" &&
+        node.computed
+      ) {
+        // Will start with the tokens `let [`.  Therefore can't start an
+        // ExpressionStatement.
+        return this.firstInExpressionStatement();
+      }
+      return false;
+
     default:
+      // Will not start with any of the above sequences of tokens, unless it
+      // starts with a child node that does.  If so, that child will take
+      // care of it (possibly by letting its own child take care of it, etc.)
       return false;
   }
 };
diff --git a/test/parens.ts b/test/parens.ts
index 10c3623b..aedca393 100644
--- a/test/parens.ts
+++ b/test/parens.ts
@@ -188,6 +188,21 @@ describe("parens", function () {
     check("const x = class { static f() { return 3; } }.f()");
   });
 
+  describe("let-bracket ambiguity", () => {
+    // Test the `let [` lookahead constraint of:
+    //   https://tc39.es/ecma262/#prod-ExpressionStatement
+
+    // This prints "[5]" (try it!).  A MemberExpression `let [x]` appears twice.
+    check("{var let = [[3]], x = 0; (let [x] = [5]); console.log(let [x])}");
+
+    // This prints "[3]".  The first `let [x]` starts a LexicalDeclaration.
+    check("{var let = [[3]], x = 0; {let [x] = [5];} console.log(let [x])}");
+
+    // Focussing in on the key statements:
+    check("(let [x] = [5])"); // ExpressionStatement, with MemberExpression
+    check("let [x] = [5]"); // LexicalDeclaration
+  });
+
   describe("ExpressionBody", () => {
     // We need parens if an ExpressionBody starts with an ObjectExpression:
     check("() => ({ x: 1 })"); // ExpressionBody, with an object literal
@@ -205,6 +220,10 @@ describe("parens", function () {
     check("const D = (C) => class extends C {};");
     check("() => class {}");
     check("() => class {}.prototype");
+
+    // `let [` starting an ExpressionBody
+    check("() => let[x]");
+    check("() => let [x] = [5]");
   });
 
   it("ReprintedParens", function () {