js_parser: name an anonymous decorated class after the parameter or for loop variable it initializes

[robobun]

Problem

• With standard (TC39) decorators, an anonymous class expression used directly as the initializer of a parameter or of a for loop declaration ends up named "_class":

  function dec() {}
  function f(K = class { @dec m() {} }) { return K.name; }
  console.log(f()); // bun 1.4.0: "_class"    node, decorators removed: "K"
  for (const L = class { @dec m() {} }; ; ) { console.log(L.name); break; } // "_class", node: "L"

  The parameter form misbehaves the same way in function expressions, arrows, methods, setters and constructors (including TypeScript parameter properties), with a class decorator on the default (K.name and the decorator's context.name are both ""), and for an accessor-only class with no decorators at all, which goes through the same lowering. Destructuring defaults (function f({ K = class { @dec m() {} } } = {})), const K = ..., K = ..., object keys and class fields were already right.
• Cause: the lowering rewrites the expression to _class = class {}, __decorateElement(...), _class (lower_impl, src/js_parser/lower/lower_decorators.rs:1145), which on its own would name the class after the temporary, so it relies on the visitor having recorded the name the source position gives the class in p.decorator_class_name. visit_decls, visit_binding (destructuring defaults), visit_binary, visit_class and the export default path set it. Two sites that visit a binding's initializer did not: visit_args (parameter defaults) and the SLocal arm of visit_for_loop_init, which iterates a for (;;) head's declarations itself instead of going through visit_decls.

Fix

• The check that visit_decls and the two destructuring arms of visit_binding each had inline becomes decorator_class_name_from_binding(binding, value) -> Option<&[u8]>: the binding's name when the binding is a plain identifier and the value is an anonymous class that will be lowered, None otherwise. Those three sites and the two missing ones assign its result to decorator_class_name around the visit and restore the previous value afterwards. It returns the name rather than doing the visit itself so that visit_decls, which visits through visit_expr_in_out, can use it too, and so it has the same shape as the decorator_class_name_from_key helper js_parser: name lowered anonymous classes after numeric, non-ASCII and private property keys #38787 adds for property keys.
• This is correct because a binding's initializer is one of the positions where the language names an anonymous class after the binding (see Background), and that applies to a parameter or a for head declaration exactly as it does to the const and destructuring cases that already worked; so the binding's name is the name the class has without lowering, which is what node prints. The guard keeps everything else unchanged: a named class, a class that is not lowered, or a class nested inside some other initializer (K = [class { @dec m() {} }][0]) gets None, and e_class clears the field before visiting the class body, so it cannot leak into nested classes. At the three converted sites the conditions are the ones that were inline before; the only difference is that a non-matching value now stores None instead of leaving the field alone, and every writer of the field only sets it immediately before visiting the class that consumes it, so it is already None whenever these sites run. Every parameter list (functions, arrows, methods, accessors, constructors) is visited through visit_args, so that one call covers all of them.
• Merge order: the lowering applies the recorded name by giving the class expression a binding of that name, which shadows the variable inside the class body. At the two new positions this PR therefore reproduces what const K = ... does today: a body that refers to the parameter while a class decorator replaces the class, or after the parameter is reassigned, sees the original class, and bun build renames the binding (K2). js_parser: keep the inferred name of lowered anonymous decorated class expressions #38757 replaces that binding with a __name call for every context and still reads decorator_class_name, so this propagation is needed with or without it; merged on top of this branch locally, the probes for those shapes match node (details below). Landing js_parser: keep the inferred name of lowered anonymous decorated class expressions #38757 first avoids the intermediate state, if convenient.
• Not touched here: ??=, ||= and &&= have the same gap at the assignment site in visit_binary (js_parser: name an anonymous decorated class assigned with ??=, ||= or &&= after the target #38924), and the remaining inline copies that derive the name from an assignment target or export default are a separate cleanup.
• Verification, test/bundler/transpiler/es-decorators.test.ts, new anonymous decorated class as a binding initializer block: every kind of parameter list plus object and array destructuring defaults (the converted visit_binding sites), const/let/var/destructuring declarations in a for head, a class decorator's context.name, an accessor-only class, a TypeScript parameter property, and a guard test (explicit class name kept, nested class not renamed, explicitly passed argument unaffected). Expected values match node 26 running the same code with the decorators removed. Five of the six tests fail on the unfixed build ("_class" or ""); with the fix all 65 tests in the file pass. The converted visit_decls site is covered by the existing const X = class { @dec ... } tests in this file and es-decorators-esbuild.test.ts.
  • Also run with the fix: es-decorators-esbuild.test.ts, decorators.test.ts, decorator-metadata.test.ts, bundler_decorator_metadata.test.ts, regression/issue/27575.test.ts, transpiler.test.js, esbuild/ts.test.ts, esbuild/default.test.ts, esbuild/lower.test.ts, bundler_naming.test.ts: 603 pass, 0 fail.
  • cargo clippy -p bun_js_parser is clean.

Background

• Name inference: an anonymous function or class expression takes its .name from the position it appears in, provided it appears there directly. The positions are a variable initializer (const K = class {}, including in a for head), an assignment (K = class {}), a property value ({ K: class {} }), export default, and the initializer of a binding named K, whether that binding is a destructuring element or a plain parameter. Wrapping the class in something else (an array literal, an assignment to a temporary) gives it that context's name instead, or none.
• Standard decorator lowering: a class with decorators or accessor members (should_lower_standard_decorators, decided at parse time) is rewritten into _class = class { ... } followed by __decorateElement(...) calls. Since the class is now assigned to _class, the visitor records the source position's name in p.decorator_class_name before visiting the class expression; e_class in visit_expr.rs takes it (and clears the field) and passes it to the lowering as name_from_context, which uses it for the class's name and for the class decorator's context.name.
• Open PRs on the same mechanism: js_parser: keep the inferred name of lowered anonymous decorated class expressions #38757 changes how the recorded name is applied (see merge order above), js_parser: name an anonymous decorated export default class "default" #38758 covers export default, js_parser: name lowered anonymous classes after numeric, non-ASCII and private property keys #38787 adds the key-derived counterpart of this helper, js_parser: name an anonymous decorated class assigned with ??=, ||= or &&= after the target #38924 covers logical assignment, and js_parser: declare decorator lowering temporaries per evaluation in parameter defaults and field initializers #38904 adds bookkeeping around the loop in visit_args. None of them changes the sites edited here; js_parser: name lowered anonymous classes after numeric, non-ASCII and private property keys #38787 and this PR merge cleanly apart from both appending tests at the same spot. The "_class" fallback for a class with no naming context at all (the nested case above; node prints "") belongs to js_parser: keep the inferred name of lowered anonymous decorated class expressions #38757 and is not changed here.

Emitted code for the repro, before and after

Before:

function f(K = (_dec = [dec], _init = __decoratorStart(undefined), _class = class {
  constructor() { __runInitializers(_init, 5, this); }
  m() {}
}, __decorateElement(_init, 1, "m", _dec, _class), __decoratorMetadata(_init, _class), _class)) {
  return K.name;
}

After:

function f(K = (_dec = [dec], _init = __decoratorStart(undefined), _class = class K {
  constructor() { __runInitializers(_init, 5, this); }
  m() {}
}, __decorateElement(_init, 1, "m", _dec, _class), __decoratorMetadata(_init, _class), _class)) {
  return K.name;
}

This is the same shape destructuring defaults and const K = class { @dec m() {} } already produce.

Shadowing probe, with and without #38757

function dec() {}
function replace(cls) { return class Replacement extends cls {}; }
function a(K = class { @dec m() { return K; } }) { return new K().m() === K; }
function b(K = @replace class { m() { return K; } }) { return new K().m() === K; }
function c(K = class { @dec m() { return K; } }) { const C = K; K = 1; return new C().m(); }
// d: the same three shapes written as `const K = ...` / `let K = ...` in a block

	a	b	c	d (const/let contexts)
node (decorators removed)	true	true	1	true, true, 1
bun 1.4.0	true	true	1	true, false, class
this PR	true	false	class	true, false, class
this PR + #38757 merged locally	true	true	1	true, true, 1

With #38757 applied on top, the combined es-decorators.test.ts (this PR's tests and its own) passes, 73 tests.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1c20f4f8-c253-4b13-80af-096379813927

📥 Commits

Reviewing files that changed from the base of the PR and between 7d276b9 and 89adfb8.

📒 Files selected for processing (2)

• src/js_parser/visit/mod.rs
• test/bundler/transpiler/es-decorators.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Reproduced on bun 1.4.0 and on a debug build of main: function f(K = class { @dec m() {} }) { return K.name } returns "_class" (node, with the decorator removed, returns "K"); the same for arrows, methods, constructors, setters, a class decorator on the default, an accessor-only class, and, as pointed out in review, for (const K = class { @dec m() {} }; ;).

Fix: decorator_class_name_from_binding in src/js_parser/visit/mod.rs, used by visit_args, visit_for_loop_init, visit_decls and the two destructuring sites in visit_binding. Tests: the anonymous decorated class as a binding initializer block in test/bundler/transpiler/es-decorators.test.ts (5 of the 6 fail without the fix). Details, including the merge order relative to #38757, are in the PR description.

[claude]

LGTM — the visit_args change mirrors the exact save/set/restore pattern already used for decorator_class_name in visit_decls, visit_binding (BArray/BObject), and visit_class, and the tests cover the full parameter-list matrix plus the negative cases. The inline nit about visit_for_loop_init is a real sibling site but obscure enough to defer or note as out of scope.

Extended reasoning...

Overview

Ten lines added to visit_args in src/js_parser/visit/mod.rs so that when a plain parameter's default value is an anonymous class that will go through standard-decorator lowering, p.decorator_class_name is set to the parameter's identifier around the visit_expr call and restored afterwards. This is the fifth copy of an identical pattern already present in the same file for visit_decls, the BArray and BObject arms of visit_binding, and two places in visit_class. The guard (is_anonymous_named ∧ EClass ∧ should_lower_standard_decorators ∧ BIdentifier) is byte-for-byte the same as visit_binding uses for destructuring defaults, and e_class in visit_expr.rs consumes and clears the field before visiting the class body, so nothing can leak into nested expressions.

The test additions (~110 lines in es-decorators.test.ts) exercise every kind of parameter list — function declaration/expression, arrow, second parameter, constructor, instance/static method, object method, setter, destructuring — plus a class-decorator context.name check, an accessor-only class, a TypeScript parameter property, and a guard test confirming an explicit class name is kept, a class nested inside [...] is not renamed, and an explicitly passed argument is unaffected. Tests use test.concurrent and the file's existing runDecorator helper.

Security risks

None. This is a compile-time name-inference fix in the JS visitor with no untrusted-input parsing, no allocation, and no runtime-facing code path.

Level of scrutiny

Low-to-medium. The Rust change is mechanical — it copies an established local pattern into one more site, with the same guard and restore discipline. The behavioral change is narrowly scoped by the guard to exactly the case being fixed (anonymous lowered class directly in a plain-identifier parameter default). The PR description traces the mechanism to lower_decorators.rs and cross-references the four related open PRs on the same field, none of which conflict with this site.

Other factors

The bug-hunting pass surfaced one nit: visit_for_loop_init's SLocal arm is a sibling decl-value visit that also lacks this propagation, so for (const K = class { @dec m() {} }; ; ) still prints "_class". That's a valid same-class observation per REVIEW.md, but it's an extremely obscure input and doesn't affect the correctness of what's here — worth either the same eight-line fix or a note that it's deferred. Not a blocker.

I also confirmed the save/restore is unconditional (restore runs even when the guard doesn't match), so a None from an outer context can't be accidentally overwritten, and that decorator_class_name is Copy (Option<&[u8]>) so the plain assignment save/restore is sound.

[robobun]

^{Updated 12:05 PM PT - Aug 15th, 2026}

❌ @robobun, your commit 89adfb8 has some failures in Build #98136 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38906

That installs a local version of the PR into your bun-38906 executable, so you can run:

bun-38906 --bun

[claude]

LGTM — the follow-up commits addressed the visit_for_loop_init sibling site and the comment-cop feedback.

What was reviewed:

• visit_binding_initializer is a straight extraction of the save/set/restore logic that already existed inline at the two visit_binding destructuring sites; conditions are identical, so those two call sites are behavior-preserving.
• The two new call sites (visit_args, visit_for_loop_init's SLocal arm) only diverge from a plain visit_expr when the initializer is directly an anonymous EClass with should_lower_standard_decorators — every other initializer is unchanged.
• Confirmed e_class in visit_expr.rs takes and clears decorator_class_name before visiting the class body, so the name can't leak into a nested class (and the guard test covers this).
• The shadowing trade-off in the description is the existing behavior of the decorator_class_name mechanism (already exhibited by const K = ...), not new to this PR.

Extended reasoning...

Overview

Two-file change: src/js_parser/visit/mod.rs (net −8 lines) extracts a visit_binding_initializer helper that saves/sets/restores self.decorator_class_name around visit_expr, and applies it at four sites — the two destructuring-default sites in visit_binding (which already had this logic inlined verbatim) plus two sites that were missing it: visit_args (parameter defaults) and visit_for_loop_init's SLocal arm. test/bundler/transpiler/es-decorators.test.ts gains a six-test describe block covering every parameter-list kind, for-head declarations, class-decorator context.name, an accessor-only class, a TS parameter property, and a guard test for named/nested/passed classes.

Security risks

None. This is transpiler output-shape logic for standard-decorator lowering; no untrusted-input parsing, allocation, syscalls, or FFI is touched. The guard is purely additive on top of an existing visit_expr call.

Level of scrutiny

Medium. The parser visitor is a hot path that runs on every file, but the helper's guard (is_anonymous_named() && EClass && should_lower_standard_decorators && BIdentifier) means the only code that sees any behavior change is an anonymous decorated/accessor class used directly as a parameter default or for-head initializer. For every other initializer the helper is save field; visit_expr; restore field on an Option<&[u8]> that was already None. The two refactored destructuring sites had byte-identical conditions before; I diffed the old inline blocks against the helper and the only differences are let-chain vs nested-if syntax and dropping a redundant .get() (the field is reachable via Deref).

Other factors

• My prior review flagged visit_for_loop_init as a missed sibling; 288315c fixed it, added a for-head test, and moved the shared logic into the helper. The two comment-cop notes were addressed by trimming the doc comment to one line in e0b3cdd/54bad43.
• No CODEOWNERS entry covers src/js_parser/.
• The PR description documents a shadowing edge case (a class body referring to the parameter after a replacing class decorator or reassignment now sees the original class). I checked and this is the pre-existing behavior of every other decorator_class_name site (const K = ... already produces it, per the description's own probe table), and #38757 is the tracked fix for the whole mechanism — this PR just brings two more sites in line with the rest.
• Tests follow harness conventions (test.concurrent, tempDir, concurrent pipe drain, stderr/stdout asserted before exit code) and cover the negative contract (explicit name kept, nested class not renamed, explicit argument unaffected). The author reports 603 passes across the wider decorator/transpiler suites and clean clippy.

[claude]

LGTM — the visit_for_loop_init sibling and the doc-comment length are both addressed; the shared helper is a clean dedup of what visit_decls and the two destructuring arms already did.

What was reviewed:

• Verified the helper's guards (EClass, class_name.is_none(), should_lower_standard_decorators, BIdentifier) are exactly the old inline conditions — is_anonymous_named() for EClass is class_name.is_none(), so dropping the outer was_anonymous_named_expr gate is behavior-preserving.
• Checked the one difference (unconditional assign vs. leave-prev-when-no-match): e_class in visit_expr.rs:2631-2632 reads-and-clears the field, so prev is always None at these sites; the new form is defensively safer for a stale value anyway.
• Confirmed visit_args covers every parameter-list kind (functions, arrows, methods, constructors) via visit_func, matching the test matrix.

Extended reasoning...

Overview

Two files: src/js_parser/visit/mod.rs adds decorator_class_name_from_binding and wires it into four binding-initializer visit sites (visit_args, visit_for_loop_init's SLocal arm, and the BArray/BObject arms of visit_binding), plus refactors visit_decls to use the same helper. test/bundler/transpiler/es-decorators.test.ts gains a six-test describe block covering every parameter-list kind, for-head declarations, class-decorator context.name, an accessor-only class, a TS parameter property, and a negative guard (explicit name kept, nested class not renamed, passed argument unaffected).

Security risks

None. This is name inference for a transpiler lowering; no untrusted input handling, no allocation, no unsafe. The helper only reads arena-owned data through &self.

Level of scrutiny

The visitor pass runs on every parsed file, so a mistake here would be visible — but the change is narrowly gated: the helper returns None unless the initializer is directly an anonymous EClass with should_lower_standard_decorators set and the binding is a plain identifier. Everything else (named classes, non-lowered classes, non-class initializers, destructuring/rest bindings) is unchanged. The three refactored sites (visit_decls, BArray, BObject) previously open-coded the identical checks; I compared them condition-by-condition against the helper and against Expr::is_anonymous_named (EClass → class_name.is_none()) and they match. The only semantic delta is that the field is now unconditionally assigned (to None when the guards fail) instead of left at its previous value; since e_class in visit_expr.rs consumes-and-clears the field on entry and every setter save/restores, the previous value is always None at these sites, so this is moot (and strictly safer if that invariant ever slipped).

Other factors

My prior review flagged the missing visit_for_loop_init sibling; that is now fixed with a dedicated test (const/let/var/destructuring in the for head). The comment-cop flags on the doc comment were resolved by trimming to one line. Tests are subprocess-based, use test.concurrent, drain both pipes, and assert exact JSON output verified against Node. The known shadowing trade-off is pre-existing (already applies to const K = ...) and is documented as fixed by the orthogonal #38757, which reads the same field this PR sets. The one deliberately excluded sibling (??=/||=/&&= in visit_binary) is called out in the description.

[claude]

LGTM — the visit_for_loop_init sibling I raised earlier is now covered, and the refactor into decorator_class_name_from_binding is behavior-preserving at the three converted sites.

What was reviewed:

• Verified the helper's guards match the old inline checks: is_anonymous_named() on an EClass is exactly class_name.is_none(), so the condition is identical.
• Checked the "unconditionally assign None on non-match" change: e_class in visit_expr.rs:2631 takes and clears the field before visiting the body, and every other writer saves/restores around its visit, so no name can be pending at these sites.
• Tests cover the variant matrix (all parameter list kinds, for-head const/let/var/destructuring, ctx.name, accessor-only, TS parameter property) plus a guard test for named/nested/passed.

Extended reasoning...

Overview

Two files: src/js_parser/visit/mod.rs and test/bundler/transpiler/es-decorators.test.ts. The Rust change extracts three copies of an inline decorator_class_name check into a helper (decorator_class_name_from_binding) and applies it at two additional binding-initializer visit sites that were missing it: visit_args (parameter defaults) and the SLocal arm of visit_for_loop_init. The test file gains a six-test describe block covering every kind of parameter list, for-head declarations, and negative guards.

Security risks

None. This affects only what .name a lowered decorated class expression receives; no untrusted-input parsing, allocation, or FFI surface is touched.

Level of scrutiny

js_parser is core, but the change is narrowly scoped to the decorator_class_name field, which is only read when should_lower_standard_decorators is true on the immediate EClass — a small blast radius. The refactor at the three converted sites is behavior-identical: for an EClass, is_anonymous_named() is defined as class_name.is_none() (ast/expr.rs:48), so the helper's guard is the same. The one semantic difference — non-matching values now store None instead of leaving the field alone — is safe because the consumer (e_class in visit_expr.rs:2631–2632) reads and clears the field, and every writer saves/restores around its own visit, so the field is already None on entry to these loops. The two new sites use the same save/set/visit/restore shape as the existing ones.

Other factors

• My earlier review comment about visit_for_loop_init was addressed in 288315c and a for-head test added; the comment-cop notes about the doc comment length were addressed in 54bad43.
• The PR description honestly documents that the two new positions inherit the same shadowing quirk const K = ... already has, and that #38757 fixes it for all positions. That is a pre-existing property of how the recorded name is applied, not of whether it is recorded, so it does not block this change.
• The description enumerates the sibling deliberately left out (logical assignment, tracked as #38924) and states the wider test suites (es-decorators-esbuild, decorators, transpiler, esbuild suites, bundler_naming) pass.
• Tests follow harness conventions (tempDir, bunEnv, drain both pipes concurrently, assert stderr/stdout before exit code, test.concurrent for the independent subprocess spawns).

[robobun]

Status of this PR against main after #40833 merged (a22b2aa, the rewritten lowering).

What changed on main: a class expression with only member decorators or accessor members now stays in place, so it gets its name from the parameter or the for head natively. Four of the six tests in the anonymous decorated class as a binding initializer block pass on main without this PR.

What this PR still fixes: a class expression with a class decorator. The lowering puts that class in the middle of a comma expression and passes the name to __decorateElement(_init, 0, name, ...). The name comes only from decorator_class_name, and visit_args and the SLocal arm of visit_for_loop_init do not set it. On a debug build of a22b2aa, with const dec = (v, c) => {}:

case	main a22b2aa	main + this PR	node, decorators removed
((k = @dec class {}) => k)().name	""	"k"	"k"
(function (K = @dec class {}) { return K; })().name	""	"K"	"K"
class M { m(p = @dec class {}) { return p; } }	""	"p"	"p"
for (const L = @dec class {}; ; ), also let	""	"L"	"L"
context.name seen by the decorator in (k = @dec class {}) => k	""	"k"

The main + this PR column is the same under bun run and bun build --target=bun. The arrow, function and for (const ...) rows also hold under bun build --minify, because the name is a string literal.

Merge state: this branch merges into a22b2aa without conflicts, and cargo check -p bun_js_parser passes on the result. With the merged build, test/bundler/transpiler/es-decorators.test.ts gives 423 pass, 1 fail.

The one failure is a guard in parameter name does not override an explicit class name or reach a nested class:

function nested(K = [class { @dec m() {} }][0]) { return K.name; }
expect(nested).not.toBe("K");

It fails on a22b2aa with and without this PR. The class expression now stays in place, the constant folder inlines [class {}][0], and the class then takes the name K. The same line without the decorator also gives "K" on bun (node: ""). That is the case #35593 covers, not this change. A wrapper that the folder leaves alone keeps the guard valid: K = id(class { @dec m() {} }) gives "" on a22b2aa. K = (0, class { @dec m() {} }) does not work as a guard, because the parser drops the 0, and the class takes the name K.