Keep required parens around a new expression callee

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 &current.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")");

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);
+  });
+});