Skip to content

Commit

Permalink
parens: Fix arrow-function bodies getting excess parens
Browse files Browse the repository at this point in the history
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: benjamn#914
Fixes: benjamn#1082
  • Loading branch information
gnprice committed Jun 12, 2022
1 parent bde3724 commit d83b22e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
20 changes: 15 additions & 5 deletions lib/fast-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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") {
Expand Down
19 changes: 19 additions & 0 deletions test/parens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d83b22e

Please sign in to comment.