Skip to content
Open
234 changes: 197 additions & 37 deletions src/js_parser/lower/lower_decorators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
enum StaticElementKind {
Block,
FieldOrAccessor,
/// Undecorated public static field moved out of the body of a class that
/// has class decorators (index into `relocated_static_fields`).
PlainField,
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -139,6 +142,41 @@
&& !is_eval_or_arguments(name)
}

/// `static x = 1` / `static x;` with no decorators. When the class itself is
/// decorated these are initialized after the class decorators run (on whatever
/// class they returned), so they cannot stay in the class body.
#[inline]
fn is_plain_static_field(prop: &Property) -> bool {
prop.kind == PropertyKind::Normal
&& prop.flags.contains(Flags::Property::IsStatic)
&& !prop.flags.contains(Flags::Property::IsMethod)
&& prop.ts_decorators.len_u32() == 0
&& prop.key.is_some()
&& !has_private_key(prop)
}

#[inline]
fn has_private_key(prop: &Property) -> bool {
matches!(prop.key, Some(key) if matches!(key.data, js_ast::ExprData::EPrivateIdentifier(_)))
}

/// Whether a class statement with class decorators gets a binding of its own
/// for the body's references to its name (see `declare_inner_class_binding`).
///
/// Private static members stay installed on the class as written, so a body
/// that reaches them through the class name (`Foo.#count`) has to keep naming
/// that class; pointing the name at a replacement returned by a decorator would
/// turn every such access into a brand-check TypeError.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fn wants_inner_class_binding(class: &G::Class) -> bool {
class.should_lower_standard_decorators
&& class.ts_decorators.len_u32() > 0
&& !class
.properties
.slice()
.iter()
.any(|prop| prop.flags.contains(Flags::Property::IsStatic) && has_private_key(prop))
}

// ── impl P ───────────────────────────────────────────────────────────────────

impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_ONLY> {
Expand Down Expand Up @@ -172,6 +210,22 @@
ref_
}

/// `_Foo`: what `class Foo`'s body refers to by its own name once lowered.
/// The name binding a class declaration creates for its body is immutable,
/// so after `Foo = __decorateElement(...)` it would still hold the
/// undecorated class; `lower_impl` instead declares this symbol (`let _Foo;`)
/// in the scope the class statement is in, which must be the current scope
/// here, and reassigns it along with `Foo`. Element decorator and computed
/// key expressions are evaluated before that declaration and so hit the TDZ
/// the spec gives them.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fn declare_inner_class_binding(&mut self, class_name_ref: Ref) -> Ref {
let class_name: &'a [u8] = self.symbols[class_name_ref.inner_index() as usize]
.original_name
.slice();
let name = self.bump_name2(b"_", class_name);
self.new_sym(js_ast::symbol::Kind::Other, name)
}

/// Single var declaration statement.
fn var_decl(&mut self, ref_: Ref, value: Option<Expr>, l: bun_ast::Loc) -> Stmt {
let binding = self.b(B::Identifier { r#ref: ref_ }, l);
Expand Down Expand Up @@ -1064,9 +1118,13 @@

// ── Public API ───────────────────────────────────────

/// `inner_class_ref` is the binding `visit_class` resolved the body's
/// references to the class name to (see `declare_inner_class_binding`), or
/// `Ref::NONE` when it left them on the class name itself.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fn lower_standard_decorators_stmt(
&mut self,
stmt: Stmt,
inner_class_ref: Ref,
out: &mut BumpVec<'a, Stmt>,
) {
// Every call site is the visitStmt `s_class` branch. `Stmt` and the
Expand All @@ -1078,7 +1136,15 @@
js_ast::StmtData::SClass(c) => c,
_ => unreachable!(),
};
self.lower_impl(&mut s_class.class, stmt.loc, None, false, Some(stmt), out);
self.lower_impl(
&mut s_class.class,
stmt.loc,
None,
false,
Some(stmt),
inner_class_ref,
out,
);
}

pub(crate) fn lower_standard_decorators_expr(
Expand All @@ -1089,7 +1155,15 @@
) -> Expr {
let bump = self.arena;
let mut out = BumpVec::<Stmt>::new_in(bump);
self.lower_impl(class, loc, name_from_context, true, None, &mut out);
self.lower_impl(
class,
loc,
name_from_context,
true,
None,
Ref::NONE,
&mut out,
);
if out.is_empty() {
return self.new_expr(E::Missing {}, loc);
}
Expand All @@ -1109,6 +1183,7 @@
name_from_context: Option<&'a [u8]>,
is_expr: bool,
original_stmt: Option<Stmt>,
visited_inner_class_ref: Ref,
out: &mut BumpVec<'a, Stmt>,
) {
let p = self;
Expand Down Expand Up @@ -1156,15 +1231,15 @@
class_name_loc = class.class_name.as_ref().unwrap().loc;
}

let mut inner_class_ref: Ref = class_name_ref;
if !is_expr {
// SAFETY: original_name is arena-owned for 'a.
let cns: &'a [u8] = p.symbols[class_name_ref.inner_index() as usize]
.original_name
.slice();
let name = p.bump_name2(b"_", cns);
inner_class_ref = p.new_sym(js_ast::symbol::Kind::Other, name);
}
// Statement mode only; class expressions keep their own (per-evaluation)
// name binding and redirect relocated code to `_class` instead.
Comment thread
robobun marked this conversation as resolved.
Outdated
let inner_class_ref: Ref = if is_expr {
class_name_ref
} else if visited_inner_class_ref.is_symbol() {
visited_inner_class_ref
} else {
p.declare_inner_class_binding(class_name_ref)
};

// `ExprNodeList = Vec<Expr>` owns its
// buffer, so this MUST be a real ownership transfer; the previous
Expand All @@ -1175,6 +1250,12 @@
bun_alloc::AstAlloc::take(&mut class.ts_decorators);
let class_decorators_len = class_decorators.len_u32() as usize;

// Relocated initializers are printed outside the class body, where a
// `#name` access would be a syntax error, so a class with private
// members keeps its static fields in place.
let has_private_members = class.properties.slice().iter().any(has_private_key);
let relocate_static_fields = class_decorators_len > 0 && !has_private_members;

let init_ref = p.new_sym(js_ast::symbol::Kind::Other, b"_init");
if is_expr {
let binding = p.b(B::Identifier { r#ref: init_ref }, loc);
Expand Down Expand Up @@ -1266,10 +1347,11 @@
);
pre_eval_stmts.push(p.var_decl(dec_ref, Some(arr), loc));
}
if prop.flags.contains(Flags::Property::IsComputed)
&& prop.key.is_some()
&& prop.ts_decorators.len_u32() > 0
&& (prop.ts_decorators.len_u32() > 0
|| (relocate_static_fields && is_plain_static_field(prop)))
{

Check failure on line 1354 in src/js_parser/lower/lower_decorators.rs

View check run for this annotation

Claude / Claude Code Review

Hoisted computed keys of relocated static fields break source-order evaluation vs. in-body computed keys

Widening the pre-eval condition to hoist computed keys of relocated undecorated static fields breaks source-order evaluation when they're interleaved with computed keys of members that stay in the body (undecorated instance fields, methods, getters/setters). `@wrap class Foo { static [key("s1")] = 1; [key("i")] = 2; static [key("s2")] = 3; }` now evaluates keys as `["s1","s2","i"]` instead of the spec-correct `["s1","i","s2"]` that bun 1.4.0 produced — the PR's own "in source order" test only in
Comment thread
claude[bot] marked this conversation as resolved.
computed_key_counter += 1;
let key_name: &'a [u8] = if computed_key_counter == 1 {
b"_computedKey"
Expand Down Expand Up @@ -1315,8 +1397,22 @@
}
}

// For named class expressions: swap to expr_class_ref for suffix ops
// Must be read before the lowering itself starts referencing the binding.
let inner_binding_used =
!is_expr && p.symbols[inner_class_ref.inner_index() as usize].use_count_estimate > 0;
if !is_expr && !inner_binding_used {
// Either declared below or merged into the class name, so the symbol
// never prints as an undeclared `_Foo`.
Comment thread
robobun marked this conversation as resolved.
Outdated
p.symbols[inner_class_ref.inner_index() as usize]
.link
.set(class_name_ref);
}

// For named class expressions: swap to expr_class_ref for suffix ops.
// Code relocated out of the body can no longer see the expression's own
// name binding, so it is redirected to `_class` as well.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut original_class_name_for_decorator: Option<&'a [u8]> = None;
let mut relocated_name_rewrite: Option<RewriteKind> = None;
if is_expr
&& !expr_class_is_anonymous
&& let Some(ecr) = expr_class_ref
Expand All @@ -1327,6 +1423,10 @@
.original_name
.slice(),
);
relocated_name_rewrite = Some(RewriteKind::ReplaceRef {
old: class_name_ref,
new: ecr,
});
class_name_ref = ecr;
class_name_loc = loc;
}
Expand Down Expand Up @@ -1385,6 +1485,7 @@
let mut static_init_entries = BumpVec::<FieldInitEntry>::new_in(bump);
let mut instance_init_entries = BumpVec::<FieldInitEntry>::new_in(bump);
let mut static_element_order = BumpVec::<StaticElement>::new_in(bump);
let mut relocated_static_fields = BumpVec::<Property>::new_in(bump);
let mut extracted_static_blocks =
BumpVec::<js_ast::StoreRef<G::ClassStaticBlock>>::new_in(bump);
let mut prefix_stmts = BumpVec::<Stmt>::new_in(bump);
Expand Down Expand Up @@ -1661,6 +1762,14 @@
}
continue;
}
if relocate_static_fields && is_plain_static_field(prop) {
static_element_order.push(StaticElement {
kind: StaticElementKind::PlainField,
index: relocated_static_fields.len(),
});
relocated_static_fields.push(prop_copy(prop));
continue;
}

Check failure on line 1772 in src/js_parser/lower/lower_decorators.rs

View check run for this annotation

Claude / Claude Code Review

Relocated static fields containing `super` / `new.target` become a SyntaxError

Relocating an undecorated static field whose initializer uses `super` (or `new.target`) now emits it verbatim as a `__publicField(...)` argument at the enclosing scope, which is a hard SyntaxError. Before this PR such fields stayed in the class body and ran; `is_plain_static_field` needs a bail-out (like the `has_private_members` one) when the initializer contains `super`/`new.target`, since `rewrite_expr` has no way to rewrite those.
Comment thread
claude[bot] marked this conversation as resolved.
new_properties.push(prop_full_copy(prop));
continue;
}
Expand Down Expand Up @@ -2086,7 +2195,13 @@

let cls_dec_list = ExprNodeList::from_bump_vec(cls_dec_args);
let dec_call = p.call_runtime(loc, b"__decorateElement", cls_dec_list);
suffix_exprs.push(p.assign_to(class_name_ref, dec_call, class_name_loc));
// `Foo = _Foo = __decorateElement(...)`
let decorated = if inner_binding_used {
p.assign_to(inner_class_ref, dec_call, class_name_loc)
} else {
dec_call
};
suffix_exprs.push(p.assign_to(class_name_ref, decorated, class_name_loc));
}

// 6: Static method extra initializers
Expand Down Expand Up @@ -2197,6 +2312,41 @@
let c_e = p.use_ref(class_name_ref, class_name_loc);
suffix_exprs.push(p.call_rt(loc, b"__runInitializers", &[i_e, n_e, c_e]));
}
StaticElementKind::PlainField => {
// `__publicField(Foo, "x", init)` keeps the field syntax's
// [[Define]] semantics; a plain assignment would invoke
// inherited setters.
Comment thread
robobun marked this conversation as resolved.
Outdated
let field = &mut relocated_static_fields[elem.index];
let key = field.key.expect("infallible: prop has key");
let target = p.use_ref(class_name_ref, class_name_loc);
match field.initializer.as_mut() {
Some(init) => {
p.rewrite_expr(
init,
RewriteKind::ReplaceThis {
ref_: class_name_ref,
loc: class_name_loc,
},
);
if let Some(rewrite) = relocated_name_rewrite {

Check failure on line 2331 in src/js_parser/lower/lower_decorators.rs

View check run for this annotation

Claude / Claude Code Review

rewrite_expr misses `this` in arrow defaults and object computed keys of relocated static fields

`rewrite_expr` doesn't recurse into arrow parameter defaults (`EArrow` walks only `body.stmts`, not `args`) or object computed keys (`EObject` walks `value`/`initializer`, not `key`), so `@wrap class Foo { static make = (x = this) => x; static t = { [this.name]: 1 } }` now emits `__publicField(Foo, "make", (x = this) => x)` at statement scope where `this === undefined` — `Foo.make()` returns `undefined` and `[this.name]` throws, whereas before this PR the field stayed in the body and `this` was
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
p.rewrite_expr(init, rewrite);
}
let init = *init;
suffix_exprs.push(p.call_rt(
key.loc,
b"__publicField",
&[target, key, init],
));
}
None => {
suffix_exprs.push(p.call_rt(
key.loc,
b"__publicField",
&[target, key],
));
}
}
}
}
}
}
Expand Down Expand Up @@ -2422,6 +2572,16 @@
new_properties = merged;
}

// `static { _Foo = this }`: static initializers left in the body (see
// `relocate_static_fields`) read the binding while the class is still
// being defined, before the suffix can assign it.
if inner_binding_used {
let this_e = p.new_expr(E::This {}, class_name_loc);
let capture = p.assign_to(inner_class_ref, this_e, class_name_loc);
let block = p.make_static_block(capture, class_name_loc);
new_properties.insert(0, block);
}

class.properties = bun_ast::StoreSlice::new_mut(new_properties.into_bump_slice_mut());
class.has_decorators = false;
class.should_lower_standard_decorators = false;
Expand Down Expand Up @@ -2552,34 +2712,23 @@
}
out.extend_from_slice(&pre_eval_stmts);
out.extend_from_slice(&prefix_stmts);
out.push(init_decl_stmt);
out.push(original_stmt.unwrap());
for expr in suffix_exprs.iter() {
out.push(p.s(
S::SExpr {
value: *expr,
..Default::default()
},
expr.loc,
));
}
// Inner class binding: let _Foo = Foo
if !inner_class_ref.eql(class_name_ref) {
p.record_usage(class_name_ref);
// `let _Foo;` comes after the pre-evaluated element decorators and
// computed keys (which must still hit its TDZ) and before the class
// statement whose leading static block assigns it. `let` rather than
// `var` so that a class statement in a loop body or function gets a
// fresh binding per evaluation, like the class binding itself.
Comment thread
robobun marked this conversation as resolved.
Outdated
if inner_binding_used {
p.record_declared_symbol(inner_class_ref);
let binding = p.b(
B::Identifier {
r#ref: inner_class_ref,
},
loc,
);
let value = Some(p.new_expr(
E::Identifier {
ref_: class_name_ref,
..Default::default()
},
class_name_loc,
));
let decls = DeclList::from_slice(&[G::Decl { binding, value }]);
);
let decls = DeclList::from_slice(&[G::Decl {
binding,
value: None,
}]);
out.push(p.s(
S::Local {
kind: S::Kind::KLet,
Expand All @@ -2589,5 +2738,16 @@
loc,
));
}
out.push(init_decl_stmt);
out.push(original_stmt.unwrap());
for expr in suffix_exprs.iter() {
out.push(p.s(
S::SExpr {
value: *expr,
..Default::default()
},
expr.loc,
));
}
}
}
9 changes: 7 additions & 2 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6470,7 +6470,12 @@ fn path_package_name<'a>(path: &fs::Path<'a>) -> Option<&'a [u8]> {
}

impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_ONLY> {
pub(crate) fn lower_class(&mut self, stmtorexpr: js_ast::StmtOrExpr) -> &'a mut [Stmt] {
/// `inner_class_ref` is what `visit_class` returned for this class.
pub(crate) fn lower_class(
&mut self,
stmtorexpr: js_ast::StmtOrExpr,
inner_class_ref: Ref,
) -> &'a mut [Stmt] {
use js_ast::g::PropertyKind;
match stmtorexpr {
js_ast::StmtOrExpr::Stmt(stmt) => {
Expand All @@ -6485,7 +6490,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// `lower_standard_decorators_stmt` takes an out-param Vec; wrap to
// keep this function's slice contract.
let mut out = BumpVec::<Stmt>::new_in(self.arena);
self.lower_standard_decorators_stmt(stmt, &mut out);
self.lower_standard_decorators_stmt(stmt, inner_class_ref, &mut out);
return out.into_bump_slice_mut();
}

Expand Down
Loading