Skip to content

Commit

Permalink
fix #2805: parentheses for commented expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 8, 2023
1 parent 9df9a65 commit 7d89263
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 15 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Changelog

## Unreleased

* Fix a regression caused by comment preservation ([#2805](https://github.com/evanw/esbuild/issues/2805))

The new comment preservation behavior that was added in 0.16.14 introduced a regression where comments in certain locations could cause esbuild to omit certain necessary parentheses in the output. The outermost parentheses were incorrectly removed for the following syntax forms, which then introduced syntax errors:

```js
(/* comment */ { x: 0 }).x;
(/* comment */ function () { })();
(/* comment */ class { }).prototype;
```

This regression has been fixed.

## 0.16.15

* Add `format` to input files in the JSON metafile data
Expand Down
51 changes: 36 additions & 15 deletions internal/js_printer/js_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1627,6 +1627,9 @@ func (p *printer) printExprCommentsAtLoc(loc logger.Loc) {
return
}
if comments := p.exprComments[loc]; comments != nil && !p.printedExprComments[loc] {
wasStmtStart := p.stmtStart == len(p.js)
wasExportDefaultStart := p.exportDefaultStart == len(p.js)

// We must never generate a newline before certain expressions. For example,
// generating a newline before the expression in a "return" statement will
// cause a semicolon to be inserted, which would change the code's behavior.
Expand All @@ -1653,18 +1656,35 @@ func (p *printer) printExprCommentsAtLoc(loc logger.Loc) {

// Mark these comments as printed so we don't print them again
p.printedExprComments[loc] = true

if wasStmtStart {
p.stmtStart = len(p.js)
}
if wasExportDefaultStart {
p.exportDefaultStart = len(p.js)
}
}
}

func (p *printer) printExprCommentsAfterCloseTokenAtLoc(loc logger.Loc) {
if comments := p.exprComments[loc]; comments != nil && !p.printedExprComments[loc] {
wasStmtStart := p.stmtStart == len(p.js)
wasExportDefaultStart := p.exportDefaultStart == len(p.js)

for _, comment := range comments {
p.printIndent()
p.printIndentedComment(comment)
}

// Mark these comments as printed so we don't print them again
p.printedExprComments[loc] = true

if wasStmtStart {
p.stmtStart = len(p.js)
}
if wasExportDefaultStart {
p.exportDefaultStart = len(p.js)
}
}
}

Expand Down Expand Up @@ -3613,7 +3633,7 @@ func (p *printer) printStmt(stmt js_ast.Stmt, flags printStmtFlags) {
// Functions and classes must be wrapped to avoid confusion with their statement forms
p.exportDefaultStart = len(p.js)

p.printExpr(s2.Value, js_ast.LComma, 0)
p.printExprWithoutLeadingNewline(s2.Value, js_ast.LComma, 0)
p.printSemicolonAfterStatement()
return

Expand Down Expand Up @@ -4258,20 +4278,21 @@ type PrintResult struct {

func Print(tree js_ast.AST, symbols js_ast.SymbolMap, r renamer.Renamer, options Options) PrintResult {
p := &printer{
symbols: symbols,
renamer: r,
importRecords: tree.ImportRecords,
options: options,
moduleType: tree.ModuleTypeData.Type,
exprComments: tree.ExprComments,
stmtStart: -1,
exportDefaultStart: -1,
arrowExprStart: -1,
forOfInitStart: -1,
prevOpEnd: -1,
prevNumEnd: -1,
prevRegExpEnd: -1,
builder: sourcemap.MakeChunkBuilder(options.InputSourceMap, options.LineOffsetTables, options.ASCIIOnly),
symbols: symbols,
renamer: r,
importRecords: tree.ImportRecords,
options: options,
moduleType: tree.ModuleTypeData.Type,
exprComments: tree.ExprComments,
stmtStart: -1,
exportDefaultStart: -1,
arrowExprStart: -1,
forOfInitStart: -1,
prevOpEnd: -1,
prevNumEnd: -1,
prevRegExpEnd: -1,
noLeadingNewlineHere: -1,
builder: sourcemap.MakeChunkBuilder(options.InputSourceMap, options.LineOffsetTables, options.ASCIIOnly),
}

if p.exprComments != nil {
Expand Down
9 changes: 9 additions & 0 deletions internal/js_printer/js_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,15 @@ func TestFunction(t *testing.T) {
"function foo([, ,] = [, ,]) {\n}\n")
}

func TestCommentsAndParentheses(t *testing.T) {
expectPrinted(t, "(/* foo */ { x() { foo() } }.x());", "/* foo */\n({ x() {\n foo();\n} }).x();\n")
expectPrinted(t, "(/* foo */ function f() { foo(f) }());", "/* foo */\n(function f() {\n foo(f);\n})();\n")
expectPrinted(t, "(/* foo */ class x { static y() { foo(x) } }.y());", "/* foo */\n(class x {\n static y() {\n foo(x);\n }\n}).y();\n")
expectPrinted(t, "(/* @__PURE__ */ (() => foo())());", "/* @__PURE__ */ (() => foo())();\n")
expectPrinted(t, "export default (/* foo */ function f() {});", "export default (\n /* foo */\n function f() {\n }\n);\n")
expectPrinted(t, "export default (/* foo */ class x {});", "export default (\n /* foo */\n class x {\n }\n);\n")
}

func TestPureComment(t *testing.T) {
expectPrinted(t,
"(function() {})",
Expand Down
54 changes: 54 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2582,6 +2582,60 @@ tests.push(
}),
)

// Test comments inside expressions
tests.push(
test(['entry.js', '--outfile=node.js', '--target=es6'], {
'entry.js': `
let foo;
(
/* x */
{
y() {
foo = this.y.name
}
}
).y();
if (foo !== 'y') throw 'fail'
`,
}),

test(['entry.js', '--outfile=node.js', '--target=es6'], {
'entry.js': `
let foo;
(
/* x */
function y() {
foo = y.name
}
)();
if (foo !== 'y') throw 'fail'
`,
}),

test(['entry.js', '--outfile=node.js', '--target=es6'], {
'entry.js': `
let foo;
(
/* x */
class y {
static z() {
foo = y.name
}
}
).z();
if (foo !== 'y') throw 'fail'
`,
}),

test(['entry.js', '--outfile=node.js', '--target=es6'], {
'entry.js': `
let foo;
(/* @__PURE__ */ (() => foo = 'y')());
if (foo !== 'y') throw 'fail'
`,
}),
)

// Test certain minification transformations
for (const minify of [[], ['--minify-syntax']]) {
tests.push(
Expand Down

0 comments on commit 7d89263

Please sign in to comment.