diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index f85dde30ad1..6380e2e73df 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1469,6 +1469,92 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) -> } } +/// 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)()` +/// must stay parenthesized: unwrapped, `new a().b()` reparses as +/// `(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). +/// +/// 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 `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 +/// 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; + // 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; + // `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) => { + // 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. 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 crossed_template, + ExprData::EDot(e) => { + if e.optional_chain.is_some() { + return !crossed_non_optional_access; + } + crossed_non_optional_access = true; + current = &e.target; + } + ExprData::EIndex(e) => { + if e.optional_chain.is_some() { + return !crossed_non_optional_access; + } + crossed_non_optional_access = true; + current = &e.target; + } + ExprData::ETemplate(e) => match &e.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; + crossed_template = true; + current = tag; + } + None => return false, + }, + _ => return false, + } + } +} + pub enum PrintResult { Result(PrintResultSuccess), Err(bun_core::Error), @@ -3457,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"("); @@ -3685,6 +3780,10 @@ pub mod __gated_printer { 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); } @@ -3739,6 +3838,10 @@ pub mod __gated_printer { 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); } @@ -4156,15 +4259,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 ecb35f50c5b..83a465b0f1d 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,67 @@ 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"); + + // 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"); + + // 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`)"); + + // 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", () => { + // `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", () => { const exp = ts.expectPrinted_; @@ -4764,3 +4825,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.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)());"], + ["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); + }); +});