Skip to content

Commit

Permalink
fix #701: parser bug with arrow functions
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jan 21, 2021
1 parent 49e0ae8 commit 0baaa8b
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 40 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@

## Unreleased

* Fix a parser bug about suffix expressions after an arrow function body ([#701](https://github.com/evanw/esbuild/issues/701))

The JavaScript parser incorrectly handled suffix expressions after a non-expression arrow function body. In practice, this came up when a semicolon was omitted from the end of an expression statement and the following expression could be considered a suffix expression:

```js
x = () => {}
(y)
```

This was incorrectly parsed as `(x = () => {})(y);` instead of `x = () => {}; y;`. With this release, this edge case should now be parsed correctly.

* Add new `neutral` platform to help text ([#695](https://github.com/evanw/esbuild/pull/695))

The new `--platform=neutral` API option that was added in the previous release was incorrectly not listed in the CLI help text for the platform feature. This omission has been fixed. The fix was contributed by [@hardfist](https://github.com/hardfist).
Expand Down
7 changes: 3 additions & 4 deletions internal/js_ast/js_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,9 @@ type EArrow struct {
Args []Arg
Body FnBody

IsAsync bool
HasRestArg bool
IsParenthesized bool
PreferExpr bool // Use shorthand if true and "Body" is a single return statement
IsAsync bool
HasRestArg bool
PreferExpr bool // Use shorthand if true and "Body" is a single return statement
}

type EFunction struct{ Fn Fn }
Expand Down
63 changes: 27 additions & 36 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,24 @@ type parser struct {
// makes it easier to quickly scan the top-level statements for "var" locals
// with the guarantee that all will be found.
relocatedTopLevelVars []js_ast.LocRef

// ArrowFunction is a special case in the grammar. Although it appears to be
// a PrimaryExpression, it's actually an AssigmentExpression. This means if
// a AssigmentExpression ends up producing an ArrowFunction then nothing can
// come after it other than the comma operator, since the comma operator is
// the only thing above AssignmentExpression under the Expression rule:
//
// AssignmentExpression:
// ArrowFunction
// ConditionalExpression
// LeftHandSideExpression = AssignmentExpression
// LeftHandSideExpression AssignmentOperator AssignmentExpression
//
// Expression:
// AssignmentExpression
// Expression , AssignmentExpression
//
afterArrowBodyLoc logger.Loc
}

// This is used as part of an incremental build cache key. Some of these values
Expand Down Expand Up @@ -1846,10 +1864,9 @@ func (p *parser) parseArrowBody(args []js_ast.Arg, data fnOrArrowDataParse) *js_
data.allowSuperCall = p.fnOrArrowDataParse.allowSuperCall

if p.lexer.Token == js_lexer.TOpenBrace {
return &js_ast.EArrow{
Args: args,
Body: p.parseFnBody(data),
}
body := p.parseFnBody(data)
p.afterArrowBodyLoc = p.lexer.Loc()
return &js_ast.EArrow{Args: args, Body: body}
}

p.pushScopeForParsePass(js_ast.ScopeFunctionBody, arrowLoc)
Expand Down Expand Up @@ -2138,22 +2155,14 @@ func (p *parser) parseParenExpr(loc logger.Loc, opts parenExprOpts) js_ast.Expr
p.log.AddRangeError(&p.source, spreadRange, "Unexpected \"...\"")
panic(js_lexer.LexerPanic{})
}
value := js_ast.JoinAllWithComma(items)
markExprAsParenthesized(value)
return value
return js_ast.JoinAllWithComma(items)
}

// Indicate that we expected an arrow function
p.lexer.Expected(js_lexer.TEqualsGreaterThan)
return js_ast.Expr{}
}

func markExprAsParenthesized(value js_ast.Expr) {
if e, ok := value.Data.(*js_ast.EArrow); ok {
e.IsParenthesized = true
}
}

func (p *parser) convertExprToBindingAndInitializer(expr js_ast.Expr, invalidLog []logger.Loc) (js_ast.Binding, *js_ast.Expr, []logger.Loc) {
var initializer *js_ast.Expr
if assign, ok := expr.Data.(*js_ast.EBinary); ok && assign.Op == js_ast.BinOpAssign {
Expand Down Expand Up @@ -2317,7 +2326,6 @@ func (p *parser) parsePrefix(level js_ast.L, errors *deferredErrors, flags exprF
p.allowIn = true

value := p.parseExpr(js_ast.LLowest)
markExprAsParenthesized(value)
p.lexer.Expect(js_lexer.TCloseParen)

p.allowIn = oldAllowIn
Expand Down Expand Up @@ -2963,24 +2971,10 @@ func (p *parser) parseExprCommon(level js_ast.L, errors *deferredErrors, flags e
}

func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredErrors, flags exprFlag) js_ast.Expr {
// ArrowFunction is a special case in the grammar. Although it appears to be
// a PrimaryExpression, it's actually an AssigmentExpression. This means if
// a AssigmentExpression ends up producing an ArrowFunction then nothing can
// come after it other than the comma operator, since the comma operator is
// the only thing above AssignmentExpression under the Expression rule:
//
// AssignmentExpression:
// ArrowFunction
// ConditionalExpression
// LeftHandSideExpression = AssignmentExpression
// LeftHandSideExpression AssignmentOperator AssignmentExpression
//
// Expression:
// AssignmentExpression
// Expression , AssignmentExpression
//
if level < js_ast.LAssign {
if arrow, ok := left.Data.(*js_ast.EArrow); ok && !arrow.IsParenthesized {
optionalChain := js_ast.OptionalChainNone

for {
if p.lexer.Loc() == p.afterArrowBodyLoc {
for {
switch p.lexer.Token {
case js_lexer.TComma:
Expand All @@ -2995,11 +2989,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE
}
}
}
}

optionalChain := js_ast.OptionalChainNone

for {
// Stop now if this token is forbidden to follow a TypeScript "as" cast
if p.lexer.Loc() == p.forbidSuffixAfterAsLoc {
return left
Expand Down Expand Up @@ -11610,6 +11600,7 @@ func newParser(log logger.Log, source logger.Source, lexer js_lexer.Lexer, optio
fnOrArrowDataParse: fnOrArrowDataParse{isOutsideFn: true},
runtimeImports: make(map[string]js_ast.Ref),
promiseRef: js_ast.InvalidRef,
afterArrowBodyLoc: logger.Loc{Start: -1},

// For lowering private methods
weakMapRef: js_ast.InvalidRef,
Expand Down
96 changes: 96 additions & 0 deletions internal/js_parser/js_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,102 @@ func TestArrow(t *testing.T) {
expectPrinted(t, "1 < (() => {})", "1 < (() => {\n});\n")
expectParseError(t, "1 < () => {}", "<stdin>: error: Unexpected \")\"\n")
expectParseError(t, "(...x = y) => {}", "<stdin>: error: A rest argument cannot have a default initializer\n")

expectParseError(t, "() => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "x => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "async () => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "async x => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "async (x) => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")

expectPrinted(t, "() => {}\n(0)", "() => {\n};\n0;\n")
expectPrinted(t, "x => {}\n(0)", "(x) => {\n};\n0;\n")
expectPrinted(t, "async () => {}\n(0)", "async () => {\n};\n0;\n")
expectPrinted(t, "async x => {}\n(0)", "async (x) => {\n};\n0;\n")
expectPrinted(t, "async (x) => {}\n(0)", "async (x) => {\n};\n0;\n")

expectPrinted(t, "() => {}\n,0", "() => {\n}, 0;\n")
expectPrinted(t, "x => {}\n,0", "(x) => {\n}, 0;\n")
expectPrinted(t, "async () => {}\n,0", "async () => {\n}, 0;\n")
expectPrinted(t, "async x => {}\n,0", "async (x) => {\n}, 0;\n")
expectPrinted(t, "async (x) => {}\n,0", "async (x) => {\n}, 0;\n")

expectPrinted(t, "(() => {})\n(0)", "(() => {\n})(0);\n")
expectPrinted(t, "(x => {})\n(0)", "((x) => {\n})(0);\n")
expectPrinted(t, "(async () => {})\n(0)", "(async () => {\n})(0);\n")
expectPrinted(t, "(async x => {})\n(0)", "(async (x) => {\n})(0);\n")
expectPrinted(t, "(async (x) => {})\n(0)", "(async (x) => {\n})(0);\n")

expectParseError(t, "y = () => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "y = x => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "y = async () => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "y = async x => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")
expectParseError(t, "y = async (x) => {}(0)", "<stdin>: error: Expected \";\" but found \"(\"\n")

expectPrinted(t, "y = () => {}\n(0)", "y = () => {\n};\n0;\n")
expectPrinted(t, "y = x => {}\n(0)", "y = (x) => {\n};\n0;\n")
expectPrinted(t, "y = async () => {}\n(0)", "y = async () => {\n};\n0;\n")
expectPrinted(t, "y = async x => {}\n(0)", "y = async (x) => {\n};\n0;\n")
expectPrinted(t, "y = async (x) => {}\n(0)", "y = async (x) => {\n};\n0;\n")

expectPrinted(t, "y = () => {}\n,0", "y = () => {\n}, 0;\n")
expectPrinted(t, "y = x => {}\n,0", "y = (x) => {\n}, 0;\n")
expectPrinted(t, "y = async () => {}\n,0", "y = async () => {\n}, 0;\n")
expectPrinted(t, "y = async x => {}\n,0", "y = async (x) => {\n}, 0;\n")
expectPrinted(t, "y = async (x) => {}\n,0", "y = async (x) => {\n}, 0;\n")

expectPrinted(t, "y = (() => {})\n(0)", "y = (() => {\n})(0);\n")
expectPrinted(t, "y = (x => {})\n(0)", "y = ((x) => {\n})(0);\n")
expectPrinted(t, "y = (async () => {})\n(0)", "y = (async () => {\n})(0);\n")
expectPrinted(t, "y = (async x => {})\n(0)", "y = (async (x) => {\n})(0);\n")
expectPrinted(t, "y = (async (x) => {})\n(0)", "y = (async (x) => {\n})(0);\n")

expectParseError(t, "(() => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(x => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async () => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async x => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async (x) => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")

expectParseError(t, "(() => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(x => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async () => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async x => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "(async (x) => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")

expectPrinted(t, "(() => {}\n,0)", "() => {\n}, 0;\n")
expectPrinted(t, "(x => {}\n,0)", "(x) => {\n}, 0;\n")
expectPrinted(t, "(async () => {}\n,0)", "async () => {\n}, 0;\n")
expectPrinted(t, "(async x => {}\n,0)", "async (x) => {\n}, 0;\n")
expectPrinted(t, "(async (x) => {}\n,0)", "async (x) => {\n}, 0;\n")

expectPrinted(t, "((() => {})\n(0))", "(() => {\n})(0);\n")
expectPrinted(t, "((x => {})\n(0))", "((x) => {\n})(0);\n")
expectPrinted(t, "((async () => {})\n(0))", "(async () => {\n})(0);\n")
expectPrinted(t, "((async x => {})\n(0))", "(async (x) => {\n})(0);\n")
expectPrinted(t, "((async (x) => {})\n(0))", "(async (x) => {\n})(0);\n")

expectParseError(t, "y = (() => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (x => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async () => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async x => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async (x) => {}(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")

expectParseError(t, "y = (() => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (x => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async () => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async x => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")
expectParseError(t, "y = (async (x) => {}\n(0))", "<stdin>: error: Expected \")\" but found \"(\"\n")

expectPrinted(t, "y = (() => {}\n,0)", "y = (() => {\n}, 0);\n")
expectPrinted(t, "y = (x => {}\n,0)", "y = ((x) => {\n}, 0);\n")
expectPrinted(t, "y = (async () => {}\n,0)", "y = (async () => {\n}, 0);\n")
expectPrinted(t, "y = (async x => {}\n,0)", "y = (async (x) => {\n}, 0);\n")
expectPrinted(t, "y = (async (x) => {}\n,0)", "y = (async (x) => {\n}, 0);\n")

expectPrinted(t, "y = ((() => {})\n(0))", "y = (() => {\n})(0);\n")
expectPrinted(t, "y = ((x => {})\n(0))", "y = ((x) => {\n})(0);\n")
expectPrinted(t, "y = ((async () => {})\n(0))", "y = (async () => {\n})(0);\n")
expectPrinted(t, "y = ((async x => {})\n(0))", "y = (async (x) => {\n})(0);\n")
expectPrinted(t, "y = ((async (x) => {})\n(0))", "y = (async (x) => {\n})(0);\n")
}

func TestTemplate(t *testing.T) {
Expand Down

0 comments on commit 0baaa8b

Please sign in to comment.