From d6bb94f8bea11c316a48872ea64f83b1b13fbf1a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 4 Jun 2026 17:01:21 +0000 Subject: [PATCH 1/6] Keep required parens around a new expression callee The JS printer dropped the parentheses required around the callee of a `new` expression when the callee was a tagged template whose tag contains a call, or an optional chain. This changed the meaning of the transpiled program: new (foo()`bar`)() -> new foo()`bar` // reparses as (new foo())`bar` new (baz()?.qux)() -> new baz()?.qux // new in optional chain: SyntaxError Only ECall consulted `Level::New` / `ForbidCall` to wrap itself as a `new` callee; EDot, EIndex, and tagged ETemplate forwarded ForbidCall to their target instead, so the inner call wrapped (parens on the wrong node) or, for templates, the flag was dropped entirely. Wrap the whole member/index/tagged-template chain in parens when it is a `new` callee and contains a call or optional chain that `new` would otherwise capture, and stop forwarding ForbidCall once wrapped so the inner call isn't parenthesized on its own. Plain member chains (`new (a.b.c)()`) still drop the redundant parens. --- src/js_printer/lib.rs | 81 ++++++++++++++++++++-- test/bundler/transpiler/transpiler.test.js | 57 ++++++++++++++- 2 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index f85dde30ad1..a9a684e0c21 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1469,6 +1469,45 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> } } +/// When a member access, index, or tagged template is the direct callee of a +/// `new` expression, the surrounding parentheses are only required when the +/// chain contains a call or optional chain that `new` would otherwise capture. +/// +/// `new a.b.c()` is identical to `new (a.b.c)()` — a plain member chain binds +/// into the `new` target — so those parens may be dropped. But `new (a().b)()` +/// must stay parenthesized: unwrapped, `new a().b()` reparses as +/// `(new a()).b()`. Likewise an optional chain (`new (a?.b)()`) is a syntax +/// error without the parens (`new` may not appear in an optional chain). +/// +/// Walks the `EDot`/`EIndex`/`ETemplate` (tagged) chain and returns `true` when +/// a descendant `ECall` or optional-chain node makes the parens load-bearing. +fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { + use js_ast::ExprData; + let mut current = expr; + loop { + match ¤t.data { + ExprData::ECall(_) => return true, + ExprData::EDot(e) => { + if e.optional_chain.is_some() { + return true; + } + current = &e.target; + } + ExprData::EIndex(e) => { + if e.optional_chain.is_some() { + return true; + } + current = &e.target; + } + ExprData::ETemplate(e) => match &e.tag { + Some(tag) => current = tag, + None => return false, + }, + _ => return false, + } + } +} + pub enum PrintResult { Result(PrintResultSuccess), Err(bun_core::Error), @@ -3670,7 +3709,17 @@ pub mod __gated_printer { ExprData::EDot(e) => { let is_optional_chain = e.optional_chain == Some(js_ast::OptionalChain::Start); - let mut wrap = false; + // As the callee of `new`, wrap the whole member chain in parens when it + // contains a call or optional chain; `new (a().b)()` must not reparse as + // `(new a()).b()`. The parens then protect the inner call, so drop + // `ForbidCall` before descending to avoid wrapping it a second time. + let mut wrap = (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) + && new_callee_needs_parens(&expr); + if wrap { + self.print(b"("); + flags.remove(ExprFlag::ForbidCall); + } + if e.optional_chain.is_none() { flags.insert(ExprFlag::HasNonOptionalChainParent); @@ -3682,7 +3731,7 @@ pub mod __gated_printer { return; } } else { - if flags.contains(ExprFlag::HasNonOptionalChainParent) { + if !wrap && flags.contains(ExprFlag::HasNonOptionalChainParent) { wrap = true; self.print(b"("); } @@ -3720,7 +3769,16 @@ pub mod __gated_printer { } } ExprData::EIndex(e) => { - let mut wrap = false; + // See `EDot`: wrap the whole chain in parens as a `new` callee when it + // contains a call or optional chain, and drop `ForbidCall` so the inner + // call isn't parenthesized on its own. + let mut wrap = (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) + && new_callee_needs_parens(&expr); + if wrap { + self.print(b"("); + flags.remove(ExprFlag::ForbidCall); + } + if e.optional_chain.is_none() { flags.insert(ExprFlag::HasNonOptionalChainParent); @@ -3736,7 +3794,7 @@ pub mod __gated_printer { } } } else { - if flags.contains(ExprFlag::HasNonOptionalChainParent) { + if !wrap && flags.contains(ExprFlag::HasNonOptionalChainParent) { wrap = true; self.print(b"("); } @@ -4154,6 +4212,17 @@ pub mod __gated_printer { } } + // As the callee of `new`, a tagged template whose tag contains a call + // must be wrapped as a whole: `new (a()`b`)()` must not reparse as + // `(new a())`b``. A plain tag (`new (a`b`)()`) needs no parens since a + // tagged template is itself a member expression. + let new_wrap = e.tag.is_some() + && (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) + && new_callee_needs_parens(&expr); + if new_wrap { + self.print(b"("); + } + if let Some(tag) = &e.tag { self.add_source_mapping(expr.loc); // Optional chains are forbidden in template tags @@ -4206,6 +4275,10 @@ pub mod __gated_printer { } } self.print(b"`"); + + if new_wrap { + self.print(b")"); + } } ExprData::ERegExp(e) => { self.add_source_mapping(expr.loc); diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index ecb35f50c5b..32936f072d3 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -1,4 +1,4 @@ -import { describe, expect, it } from "bun:test"; +import { describe, expect, it, test } from "bun:test"; import { bunEnv, bunExe, hideFromStackTrace, tempDir } from "harness"; import { join } from "path"; @@ -981,6 +981,34 @@ function foo() {} ); }); + // https://github.com/oven-sh/bun/issues/31812 + it("keeps required parens around a new expression callee", () => { + const exp = ts.expectPrinted_; + + // Member/index/tagged-template/optional-chain callees that contain a call or + // optional chain must stay parenthesized: without the parens, `new`'s + // arguments and the trailing member/template/call would reparse against the + // wrong subexpression (e.g. `new (a().b)()` -> `(new a()).b()`). + exp("new (a().b)();", "new (a().b)"); + exp("new (a()[b])();", "new (a()[b])"); + exp("new (a().b.c)();", "new (a().b.c)"); + exp("new (a()`bar`)();", "new (a()`bar`)"); + exp("new (a()?.b)();", "new (a()?.b)"); + exp("new (a.b()?.c)();", "new (a.b()?.c)"); + exp("new (a().b())();", "new (a().b())"); + + // Optional chains can never be a bare `new` callee (`new a?.b` is a syntax + // error), so the parens are required even without a call in the chain. + exp("new (a?.b)();", "new (a?.b)"); + exp("new (a?.b.c)();", "new (a?.b.c)"); + + // A plain member chain or tag binds into the `new` target, so the parens are + // redundant and must not be re-emitted (no over-wrapping). + exp("new (a.b.c)();", "new a.b.c"); + exp("new (a`bar`)();", "new a`bar`"); + exp("new (a.b.c.d)();", "new a.b.c.d"); + }); + it("modifiers", () => { const exp = ts.expectPrinted_; @@ -4764,3 +4792,30 @@ describe("multi-line comment scanning", () => { expectParseError(`/*${pad600}🦊`, message); }); }); + +describe("new expression callee parenthesization", () => { + // https://github.com/oven-sh/bun/issues/31812 + // The printer used to drop the parens required around a `new` callee that was + // a tagged template whose tag contains a call, or an optional chain. Both + // changed the meaning of the transpiled program: the tagged-template form + // reparsed as `(new foo())`bar`` and the optional-chain form became an illegal + // `new` inside an optional chain (a syntax error). Run the transpiled program + // end-to-end and confirm it still builds the class instance. + test.each([ + ["tagged template tag with a call", "const foo = () => (args) => class A {}; console.log(new (foo()`bar`)());"], + ["optional chain", "const baz = () => ({ qux: class A {} }); console.log(new (baz()?.qux)());"], + ["optional chain with a call", "const a = () => ({ b: class A {} }); console.log(new (a()?.b)());"], + ["optional chain without a call", "const o = { b: class A {} }; console.log(new (o?.b)());"], + ])("evaluates %s correctly", async (_name, code) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", code], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("A {}\n"); + expect(exitCode).toBe(0); + }); +}); From 1cec0f9daf5d620dadb8e82290efcc7f85bcbe17 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 4 Jun 2026 18:04:52 +0000 Subject: [PATCH 2/6] Address review: avoid over-wrapping and fix optional-chain template tag Two follow-ups in the same printer code path: - `new_callee_needs_parens` no longer forces the outer `new`-callee parens for an optional chain that is already isolated below a non-optional access. `new ((a?.b).c)()` is printed as `new (a?.b).c` (the existing HasNonOptionalChainParent handling already wraps the `(a?.b)`), not the redundant `new ((a?.b).c)`. A call is still always wrapped. - The tagged-template printer's optional-chain check matched on the template node instead of the tag, so it was dead code: `(a?.b)`tpl`` printed as the invalid `a?.b`tpl`` (an optional chain may not directly tag a template literal). Check `tag.is_optional_chain()` so the tag is parenthesized. Tests extended for both, and the spawning test.each is now concurrent. --- src/js_printer/lib.rs | 39 +++++++++++++--------- test/bundler/transpiler/transpiler.test.js | 18 +++++++++- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index a9a684e0c21..c2197ae4ba5 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1476,27 +1476,41 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> /// `new a.b.c()` is identical to `new (a.b.c)()` — a plain member chain binds /// into the `new` target — so those parens may be dropped. But `new (a().b)()` /// must stay parenthesized: unwrapped, `new a().b()` reparses as -/// `(new a()).b()`. Likewise an optional chain (`new (a?.b)()`) is a syntax -/// error without the parens (`new` may not appear in an optional chain). +/// `(new a()).b()`. Likewise an optional chain that reaches the `new` +/// (`new (a?.b)()`) is a syntax error without the parens (`new` may not appear +/// in an optional chain). /// -/// Walks the `EDot`/`EIndex`/`ETemplate` (tagged) chain and returns `true` when -/// a descendant `ECall` or optional-chain node makes the parens load-bearing. +/// Walks the `EDot`/`EIndex`/`ETemplate` (tagged) chain toward its base: +/// +/// - a `ECall` anywhere always needs the parens — nothing else isolates a call +/// from the `new` (`new a().b()` → `(new a()).b()`); +/// - an optional-chain link needs the parens only while it still reaches the top +/// of the chain. Once a non-optional access sits above it, the printer's +/// `HasNonOptionalChainParent` handling already wraps that optional chain +/// (`(a?.b).c`), and everything below it is inside those parens, so no outer +/// wrap is needed and the walk can stop. fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { use js_ast::ExprData; let mut current = expr; + // Tracks whether a non-optional member/index access sits above `current`, + // which is exactly when `HasNonOptionalChainParent` isolates a nested + // optional chain for us. + let mut crossed_non_optional_access = false; loop { match ¤t.data { ExprData::ECall(_) => return true, ExprData::EDot(e) => { if e.optional_chain.is_some() { - return true; + return !crossed_non_optional_access; } + crossed_non_optional_access = true; current = &e.target; } ExprData::EIndex(e) => { if e.optional_chain.is_some() { - return true; + return !crossed_non_optional_access; } + crossed_non_optional_access = true; current = &e.target; } ExprData::ETemplate(e) => match &e.tag { @@ -4225,15 +4239,10 @@ pub mod __gated_printer { if let Some(tag) = &e.tag { self.add_source_mapping(expr.loc); - // Optional chains are forbidden in template tags - // `Expr::is_optional_chain` is gated upstream; inline its body. - let is_optional_chain = match &expr.data { - ExprData::EDot(d) => d.optional_chain.is_some(), - ExprData::EIndex(i) => i.optional_chain.is_some(), - ExprData::ECall(c) => c.optional_chain.is_some(), - _ => false, - }; - if is_optional_chain { + // An optional chain may not directly tag a template literal + // (`a?.b`t`` is a SyntaxError), so wrap the tag in parens. + // The check is on the tag, not on this `ETemplate` node. + if tag.is_optional_chain() { self.print(b"("); self.print_expr(*tag, Level::Lowest, ExprFlag::none()); self.print(b")"); diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 32936f072d3..0d8fa4b2c9b 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -1007,6 +1007,22 @@ function foo() {} exp("new (a.b.c)();", "new a.b.c"); exp("new (a`bar`)();", "new a`bar`"); exp("new (a.b.c.d)();", "new a.b.c.d"); + + // An optional chain that is already isolated below a non-optional access is + // wrapped by the printer's existing optional-chain handling, so the `new` + // callee needs no second pair of parens around the whole chain. + exp("new ((a?.b).c)();", "new (a?.b).c"); + exp("new ((a?.b).c.d)();", "new (a?.b).c.d"); + exp("new ((a?.b)[c])();", "new (a?.b)[c]"); + exp("new ((a()?.b).c)();", "new (a()?.b).c"); + }); + + it("wraps an optional-chain tag of a template literal", () => { + // `a?.b`tpl`` is a SyntaxError (an optional chain may not directly tag a + // template literal), so the tag must be parenthesized when printed. + ts.expectPrinted_("(a?.b)`tpl`;", "(a?.b)`tpl`"); + ts.expectPrinted_("(a?.b())`tpl`;", "(a?.b())`tpl`"); + ts.expectPrinted_("(a?.[b])`tpl`;", "(a?.[b])`tpl`"); }); it("modifiers", () => { @@ -4801,7 +4817,7 @@ describe("new expression callee parenthesization", () => { // reparsed as `(new foo())`bar`` and the optional-chain form became an illegal // `new` inside an optional chain (a syntax error). Run the transpiled program // end-to-end and confirm it still builds the class instance. - test.each([ + test.concurrent.each([ ["tagged template tag with a call", "const foo = () => (args) => class A {}; console.log(new (foo()`bar`)());"], ["optional chain", "const baz = () => ({ qux: class A {} }); console.log(new (baz()?.qux)());"], ["optional chain with a call", "const a = () => ({ b: class A {} }); console.log(new (a()?.b)());"], From b0516000d31ea8b3ff88fca3bb565c5b7f317c37 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 4 Jun 2026 18:51:10 +0000 Subject: [PATCH 3/6] Address review: don't over-wrap a new callee with an optional-chain template tag `new_callee_needs_parens` now also marks the chain as having crossed a non-optional boundary when descending through a tagged template. The tag of a template is independently parenthesized when it's an optional chain (`(a?.b)`t``), which already makes the whole template a member expression that `new` accepts, so the outer wrap is redundant: new ((a?.b)`tpl`)() now prints new (a?.b)`tpl` (was new ((a?.b)`tpl`)) new ((a?.b)`tpl`.c)() now prints new (a?.b)`tpl`.c A call in the tag still forces the parens (`ECall` returns true unconditionally), so `new (a()`tpl`)()` stays wrapped. Added assertions for all three. --- src/js_printer/lib.rs | 17 ++++++++++++----- test/bundler/transpiler/transpiler.test.js | 7 +++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index c2197ae4ba5..671a4a5d2cc 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1485,10 +1485,10 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> /// - a `ECall` anywhere always needs the parens — nothing else isolates a call /// from the `new` (`new a().b()` → `(new a()).b()`); /// - an optional-chain link needs the parens only while it still reaches the top -/// of the chain. Once a non-optional access sits above it, the printer's -/// `HasNonOptionalChainParent` handling already wraps that optional chain -/// (`(a?.b).c`), and everything below it is inside those parens, so no outer -/// wrap is needed and the walk can stop. +/// of the chain. Once a non-optional access (or a tagged template) sits above +/// it, the inner chain is already parenthesized on its own — by the printer's +/// `HasNonOptionalChainParent` handling (`(a?.b).c`) or the template-tag wrap +/// (`(a?.b)`t``) — so it is inside those parens and no outer wrap is needed. fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { use js_ast::ExprData; let mut current = expr; @@ -1514,7 +1514,14 @@ fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { current = &e.target; } ExprData::ETemplate(e) => match &e.tag { - Some(tag) => current = tag, + // A tagged template isolates an optional-chain tag the same way a + // non-optional access does: the tag is independently parenthesized + // when printed (`(a?.b)`t``), making the whole template a member + // expression `new` accepts, so no outer wrap is needed below it. + Some(tag) => { + crossed_non_optional_access = true; + current = tag; + } None => return false, }, _ => return false, diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 0d8fa4b2c9b..1f119e66f90 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -1015,6 +1015,13 @@ function foo() {} exp("new ((a?.b).c.d)();", "new (a?.b).c.d"); exp("new ((a?.b)[c])();", "new (a?.b)[c]"); exp("new ((a()?.b).c)();", "new (a()?.b).c"); + + // A tagged template likewise isolates an optional-chain tag (it gets its own + // parens), so the whole template is a member expression and no outer parens + // are needed — but a call in the tag still forces them. + exp("new ((a?.b)`tpl`)();", "new (a?.b)`tpl`"); + exp("new ((a?.b)`tpl`.c)();", "new (a?.b)`tpl`.c"); + exp("new (a()`tpl`)();", "new (a()`tpl`)"); }); it("wraps an optional-chain tag of a template literal", () => { From 7a355c14a59940e62abe8edfb7c0d6ea879e4d68 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 4 Jun 2026 19:44:12 +0000 Subject: [PATCH 4/6] Address review: handle import/require and optional-chain calls as new callees Two more cases in `new_callee_needs_parens`, both reached through a tagged template tag (which prints the tag with ForbidCall dropped): - `import(...)`, `require(...)`, and `require.resolve(...)` print as calls and are captured by `new` exactly like `ECall`, but the helper fell through to `false` for them, so a template-tagged one printed unwrapped: `new (import(x)`tpl`)()` -> `new import(x)`tpl`` (SyntaxError) and bundled `new (require("m").f`tpl`)()` -> `new require("m").f`tpl`` (wrong bind). Treat them like `ECall`. - An optional-chain call below a crossed non-optional access/template is already isolated by the printer, so it no longer forces a redundant outer wrap: `new ((a?.()).b)()` -> `new (a?.()).b`. A non-optional call still always wraps. Tests added for both; the bundled require case round-trips to `{"ok":1}`. --- src/js_printer/lib.rs | 32 ++++++++++++++++------ test/bundler/transpiler/transpiler.test.js | 10 +++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index 671a4a5d2cc..412f5525dc6 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1482,13 +1482,15 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> /// /// Walks the `EDot`/`EIndex`/`ETemplate` (tagged) chain toward its base: /// -/// - a `ECall` anywhere always needs the parens — nothing else isolates a call -/// from the `new` (`new a().b()` → `(new a()).b()`); -/// - an optional-chain link needs the parens only while it still reaches the top -/// of the chain. Once a non-optional access (or a tagged template) sits above -/// it, the inner chain is already parenthesized on its own — by the printer's -/// `HasNonOptionalChainParent` handling (`(a?.b).c`) or the template-tag wrap -/// (`(a?.b)`t``) — so it is inside those parens and no outer wrap is needed. +/// - a non-optional call (`ECall`, `import(...)`, `require(...)`) always needs +/// the parens — nothing isolates a plain call from the `new` +/// (`new a().b()` → `(new a()).b()`); +/// - an optional-chain link (including an optional call) needs the parens only +/// while it still reaches the top of the chain. Once a non-optional access (or +/// a tagged template) sits above it, the inner chain is already parenthesized +/// on its own — by the printer's `HasNonOptionalChainParent` handling +/// (`(a?.b).c`) or the template-tag wrap (`(a?.b)`t``) — so it is inside those +/// parens and no outer wrap is needed. fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { use js_ast::ExprData; let mut current = expr; @@ -1498,7 +1500,21 @@ fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { let mut crossed_non_optional_access = false; loop { match ¤t.data { - ExprData::ECall(_) => return true, + ExprData::ECall(e) => { + // An optional-chain call below a crossed boundary is isolated the + // same way an optional `EDot`/`EIndex` is; a non-optional call is + // never isolated, so it always needs the parens. + if e.optional_chain.is_some() { + return !crossed_non_optional_access; + } + return true; + } + // `import(...)`, `require(...)`, and `require.resolve(...)` also print + // as calls and are captured by `new` the same way `ECall` is; none of + // them can be an optional chain. + ExprData::EImport(_) + | ExprData::ERequireString(_) + | ExprData::ERequireResolveString(_) => return true, ExprData::EDot(e) => { if e.optional_chain.is_some() { return !crossed_non_optional_access; diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 1f119e66f90..83a465b0f1d 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -1022,6 +1022,16 @@ function foo() {} exp("new ((a?.b)`tpl`)();", "new (a?.b)`tpl`"); exp("new ((a?.b)`tpl`.c)();", "new (a?.b)`tpl`.c"); exp("new (a()`tpl`)();", "new (a()`tpl`)"); + + // An optional-chain call isolated below a non-optional access or template is + // wrapped on its own too, so no outer parens (a plain call still needs them). + exp("new ((a?.()).b)();", "new (a?.()).b"); + exp("new ((a?.b())`tpl`)();", "new (a?.b())`tpl`"); + + // `import(...)` and `require(...)` print as calls, so a template-tagged one + // must stay wrapped as a `new` callee (otherwise `new import(x)`tpl`` / + // `new require("m").f`tpl`` reparse against the wrong subexpression). + exp("new (import(x)`tpl`)();", "new (import(x)`tpl`)"); }); it("wraps an optional-chain tag of a template literal", () => { From 5bd14501d93689c84bc881a85ad0106637d13cbc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 4 Jun 2026 20:33:40 +0000 Subject: [PATCH 5/6] Fix regression: only hoist import/require new-callee wrap through template tags The previous commit made `new_callee_needs_parens` return true for `EImport`/`ERequireString`/`ERequireResolveString` anywhere in the chain, but through an `EDot`/`EIndex` chain those nodes already self-wrap via the forwarded `ForbidCall`. Forcing the outer wrap there changed the emitted parens for `new require("m").a.b(...)` from `new (require("m")).a.b(...)` to `new (require("m").a.b)(...)`, which broke node's test-buffer-indexof.js (it pins the exact `new (require("buffer")).Buffer.prototype.lastIndexOf(...)` serialization in an error-message assertion). Track whether the walk has passed through a template tag (the only node that drops `ForbidCall`) and hoist the wrap for these call-like leaves only then, so `new (require("m").f`tpl`)()` still wraps as a whole while the `EDot` chain case keeps wrapping just the call. --- src/js_printer/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index 412f5525dc6..e212431413b 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1498,6 +1498,11 @@ fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { // which is exactly when `HasNonOptionalChainParent` isolates a nested // optional chain for us. let mut crossed_non_optional_access = false; + // `EDot`/`EIndex` forward `ForbidCall` to their target, so a call at the base + // of such a chain wraps itself (`new (require("m")).f`). A tagged template + // does not — it prints its tag with `ForbidCall` dropped — so a call reached + // only through template tags must be hoisted and wrapped here instead. + let mut crossed_template = false; loop { match ¤t.data { ExprData::ECall(e) => { @@ -1510,11 +1515,12 @@ fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { return true; } // `import(...)`, `require(...)`, and `require.resolve(...)` also print - // as calls and are captured by `new` the same way `ECall` is; none of - // them can be an optional chain. + // as calls. Through an `EDot`/`EIndex` chain the forwarded `ForbidCall` + // already wraps them, so only hoist the wrap when they are reached + // through a template tag (none of them can be an optional chain). ExprData::EImport(_) | ExprData::ERequireString(_) - | ExprData::ERequireResolveString(_) => return true, + | ExprData::ERequireResolveString(_) => return crossed_template, ExprData::EDot(e) => { if e.optional_chain.is_some() { return !crossed_non_optional_access; @@ -1536,6 +1542,7 @@ fn new_callee_needs_parens(expr: &js_ast::Expr) -> bool { // expression `new` accepts, so no outer wrap is needed below it. Some(tag) => { crossed_non_optional_access = true; + crossed_template = true; current = tag; } None => return false, From fe824ddef82b0fd72270009ae7f33bc9a4c848eb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 6 Jun 2026 02:16:07 +0000 Subject: [PATCH 6/6] Decide new-callee parens once at ENew instead of in every member arm Per review: the previous shape called new_callee_needs_parens from the EDot/EIndex/ETemplate arms, and since ForbidCall keeps forwarding down a plain chain, every level re-walked the remainder - O(n^2) walks for `new a.b.c.d()`. ENew is the only producer of ForbidCall, so make the decision there: one O(depth) walk per `new` expression, and zero added work on the member arms (they revert to their original logic). When the callee needs the parens, ENew prints them and the target at Level::Lowest without ForbidCall; otherwise the original New + ForbidCall path is unchanged, so a call at the base of a member chain still wraps just itself (`new (require("m")).f`). The one other consumer change is O(1): when the HasNonOptionalChainParent parens open around an isolated optional chain, drop ForbidCall since those parens already protect the call inside (`new (a()?.b).c`, not `new ((a())?.b).c`). Output is byte-identical across the test matrix; no test changes. --- src/js_printer/lib.rs | 78 ++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 46 deletions(-) diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index e212431413b..6380e2e73df 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1469,9 +1469,9 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> } } -/// When a member access, index, or tagged template is the direct callee of a -/// `new` expression, the surrounding parentheses are only required when the -/// chain contains a call or optional chain that `new` would otherwise capture. +/// Decides, once per `ENew`, whether its callee must be printed inside +/// parentheses: only when the callee chain contains a call or optional chain +/// that `new` would otherwise capture. /// /// `new a.b.c()` is identical to `new (a.b.c)()` — a plain member chain binds /// into the `new` target — so those parens may be dropped. But `new (a().b)()` @@ -1480,11 +1480,14 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> /// (`new (a?.b)()`) is a syntax error without the parens (`new` may not appear /// in an optional chain). /// -/// Walks the `EDot`/`EIndex`/`ETemplate` (tagged) chain toward its base: +/// Returning `false` routes the callee through the `Level::New` + `ForbidCall` +/// path unchanged, where a call at the base of a member chain still wraps just +/// itself (`new (require("m")).f`). Walks the `EDot`/`EIndex`/`ETemplate` +/// (tagged) chain toward its base: /// -/// - a non-optional call (`ECall`, `import(...)`, `require(...)`) always needs -/// the parens — nothing isolates a plain call from the `new` -/// (`new a().b()` → `(new a()).b()`); +/// - a non-optional `ECall`, or `import(...)`/`require(...)` reached through a +/// template tag (which drops `ForbidCall`), always needs the parens — nothing +/// else isolates a plain call from the `new` (`new a().b()` → `(new a()).b()`); /// - an optional-chain link (including an optional call) needs the parens only /// while it still reaches the top of the chain. Once a non-optional access (or /// a tagged template) sits above it, the inner chain is already parenthesized @@ -3540,7 +3543,16 @@ pub mod __gated_printer { self.add_source_mapping(expr.loc); self.print(b"new"); self.print_space(); - self.print_expr(e.target, Level::New, ExprFlag::forbid_call()); + // Deciding the callee's parens once here keeps it a single + // O(depth) walk per `new`; leaving it to the member arms would + // re-walk the chain at every level while `ForbidCall` forwards. + if new_callee_needs_parens(&e.target) { + self.print(b"("); + self.print_expr(e.target, Level::Lowest, ExprFlag::none()); + self.print(b")"); + } else { + self.print_expr(e.target, Level::New, ExprFlag::forbid_call()); + } let args = e.args.slice(); if !args.is_empty() || level.gte(Level::Postfix) { self.print(b"("); @@ -3753,17 +3765,7 @@ pub mod __gated_printer { ExprData::EDot(e) => { let is_optional_chain = e.optional_chain == Some(js_ast::OptionalChain::Start); - // As the callee of `new`, wrap the whole member chain in parens when it - // contains a call or optional chain; `new (a().b)()` must not reparse as - // `(new a()).b()`. The parens then protect the inner call, so drop - // `ForbidCall` before descending to avoid wrapping it a second time. - let mut wrap = (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) - && new_callee_needs_parens(&expr); - if wrap { - self.print(b"("); - flags.remove(ExprFlag::ForbidCall); - } - + let mut wrap = false; if e.optional_chain.is_none() { flags.insert(ExprFlag::HasNonOptionalChainParent); @@ -3775,9 +3777,13 @@ pub mod __gated_printer { return; } } else { - if !wrap && flags.contains(ExprFlag::HasNonOptionalChainParent) { + if flags.contains(ExprFlag::HasNonOptionalChainParent) { wrap = true; self.print(b"("); + // These parens isolate the whole optional chain, so a + // call inside it no longer needs its own parens as a + // `new` callee. + flags.remove(ExprFlag::ForbidCall); } flags.remove(ExprFlag::HasNonOptionalChainParent); } @@ -3813,16 +3819,7 @@ pub mod __gated_printer { } } ExprData::EIndex(e) => { - // See `EDot`: wrap the whole chain in parens as a `new` callee when it - // contains a call or optional chain, and drop `ForbidCall` so the inner - // call isn't parenthesized on its own. - let mut wrap = (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) - && new_callee_needs_parens(&expr); - if wrap { - self.print(b"("); - flags.remove(ExprFlag::ForbidCall); - } - + let mut wrap = false; if e.optional_chain.is_none() { flags.insert(ExprFlag::HasNonOptionalChainParent); @@ -3838,9 +3835,13 @@ pub mod __gated_printer { } } } else { - if !wrap && flags.contains(ExprFlag::HasNonOptionalChainParent) { + if flags.contains(ExprFlag::HasNonOptionalChainParent) { wrap = true; self.print(b"("); + // These parens isolate the whole optional chain, so a + // call inside it no longer needs its own parens as a + // `new` callee. + flags.remove(ExprFlag::ForbidCall); } flags.remove(ExprFlag::HasNonOptionalChainParent); } @@ -4256,17 +4257,6 @@ pub mod __gated_printer { } } - // As the callee of `new`, a tagged template whose tag contains a call - // must be wrapped as a whole: `new (a()`b`)()` must not reparse as - // `(new a())`b``. A plain tag (`new (a`b`)()`) needs no parens since a - // tagged template is itself a member expression. - let new_wrap = e.tag.is_some() - && (level.gte(Level::New) || flags.contains(ExprFlag::ForbidCall)) - && new_callee_needs_parens(&expr); - if new_wrap { - self.print(b"("); - } - if let Some(tag) = &e.tag { self.add_source_mapping(expr.loc); // An optional chain may not directly tag a template literal @@ -4314,10 +4304,6 @@ pub mod __gated_printer { } } self.print(b"`"); - - if new_wrap { - self.print(b")"); - } } ExprData::ERegExp(e) => { self.add_source_mapping(expr.loc);