diff --git a/src/ast/symbol.rs b/src/ast/symbol.rs index 699462bae261..400f50f441e3 100644 --- a/src/ast/symbol.rs +++ b/src/ast/symbol.rs @@ -317,7 +317,7 @@ pub enum Kind { impl Kind { #[inline] - pub(crate) fn is_private(self) -> bool { + pub fn is_private(self) -> bool { (self as u8) >= (Kind::PrivateField as u8) && (self as u8) <= (Kind::PrivateStaticGetSetPair as u8) } diff --git a/src/js_parser/lower/lower_decorators.rs b/src/js_parser/lower/lower_decorators.rs index 77c2620dac5e..8eb840b265ef 100644 --- a/src/js_parser/lower/lower_decorators.rs +++ b/src/js_parser/lower/lower_decorators.rs @@ -374,6 +374,81 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } + /// Push the `get`/`set` property pair that replaces a lowered + /// auto-accessor. `get_return` is the expression the getter returns; + /// `set_expr` is the setter's body expression, which reads the value + /// from `setter_param_ref`. + fn push_accessor_get_set_pair( + &mut self, + props: &mut BumpVec<'a, Property>, + accessor_flags: Flags::PropertySet, + getter_key: Option, + setter_key: Option, + get_return: Expr, + setter_param_ref: Ref, + set_expr: Expr, + loc: bun_ast::Loc, + ) { + let bump = self.arena; + + let get_body = bump.alloc_slice_copy(&[self.s( + S::Return { + value: Some(get_return), + }, + loc, + )]); + let get_fn = G::Fn { + body: G::FnBody { + stmts: bun_ast::StoreSlice::new_mut(get_body), + loc, + }, + ..Default::default() + }; + + let set_body = bump.alloc_slice_copy(&[self.s( + S::SExpr { + value: set_expr, + ..Default::default() + }, + loc, + )]); + let setter_binding = self.b( + B::Identifier { + r#ref: setter_param_ref, + }, + loc, + ); + let setter_fn_args = bump.alloc(G::Arg { + binding: setter_binding, + ..Default::default() + }); + let set_fn = G::Fn { + args: bun_ast::StoreSlice::new_mut(core::slice::from_mut(setter_fn_args)), + body: G::FnBody { + stmts: bun_ast::StoreSlice::new_mut(set_body), + loc, + }, + ..Default::default() + }; + + let mut flags = accessor_flags; + flags.insert(Flags::Property::IsMethod); + props.push(Property { + key: getter_key, + value: Some(self.new_expr(E::Function { func: get_fn }, loc)), + kind: PropertyKind::Get, + flags, + ..Default::default() + }); + props.push(Property { + key: setter_key, + value: Some(self.new_expr(E::Function { func: set_fn }, loc)), + kind: PropertyKind::Set, + flags, + ..Default::default() + }); + } + /// Get the method kind code (1=method, 2=getter, 3=setter). fn method_kind(prop: &Property) -> u8 { match prop.kind { @@ -1099,6 +1174,236 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } + // ── Accessor-only lowering (no decorators) ─────────── + + /// Lower the auto-accessors of a class that has no decorators at all. + /// + /// `should_lower_standard_decorators` is also set for classes that merely + /// contain `accessor` members. Without decorators nothing has to run after + /// class creation, so none of the relocation machinery in `lower_impl` + /// applies; relocating would carry native `#name` references out of the + /// class body and lose the source order of static initializers and static + /// blocks. Instead, each accessor is replaced in place by a private + /// backing field plus a getter/setter pair (the same shape esbuild + /// emits), and every other class element is left untouched: + /// + /// ```js + /// accessor a = init; + /// // becomes + /// #a = init; + /// get a() { return this.#a; } + /// set a(v) { this.#a = v; } + /// ``` + fn lower_auto_accessors_in_place( + &mut self, + class: &mut G::Class, + loc: bun_ast::Loc, + is_expr: bool, + original_stmt: Option, + out: &mut BumpVec<'a, Stmt>, + ) { + let p = self; + let bump = p.arena; + class.should_lower_standard_decorators = false; + + if class + .properties + .slice() + .iter() + .any(|prop| prop.kind == PropertyKind::AutoAccessor) + { + // Backing names must not duplicate a private name declared in this + // class and must not shadow a private name that code inside this + // class body references (which resolves to an enclosing class). + // Every such name exists in the symbol table by now — private + // names are declared at parse time — so treating all of them as + // taken over-approximates both sets. The set is built once per + // parse and extended with each generated name (`p.taken_private_names`), + // keeping repeated lowerings distinct without rescanning. + let mut taken_private_names = match p.taken_private_names.take() { + Some(set) => set, + None => { + let mut set: HashMap<&'a [u8], ()> = HashMap::default(); + for sym in p.symbols.iter() { + if sym.kind.is_private() { + // SAFETY: original_name is arena-owned, valid for 'a. + let name: &'a [u8] = sym.original_name.slice(); + set.insert(name, ()); + } + } + set + } + }; + + let mut new_properties = BumpVec::::new_in(bump); + let mut computed_key_decls = BumpVec::::new_in(bump); + let mut computed_key_counter: usize = 0; + let mut accessor_storage_counter: usize = 0; + + for prop in class.properties.slice().iter() { + if prop.kind != PropertyKind::AutoAccessor { + new_properties.push(prop_full_copy(prop)); + continue; + } + + let key_expr = prop.key.expect("infallible: prop has key"); + let is_computed = prop.flags.contains(Flags::Property::IsComputed); + + let base_name: &'a [u8] = 'base: { + if !is_computed { + match &key_expr.data { + js_ast::ExprData::EString(s) + if !s.is_utf16 + && s.next.is_none() + && js_lexer::is_identifier(&s.data) + && !s.eql_comptime(b"constructor") => + { + break 'base p.bump_name2(b"#", &s.data); + } + js_ast::ExprData::EPrivateIdentifier(pi) => { + // SAFETY: original_name is arena-owned, valid for 'a. + let orig: &'a [u8] = p.symbols[pi.ref_.inner_index() as usize] + .original_name + .slice(); + // `accessor #p` keeps its `get #p`/`set #p` + // pair, so the backing field needs a fresh + // name: `#_p`. + break 'base p.bump_name2(b"#_", &orig[1..]); + } + _ => {} + } + } + let name = p.bump_name(b"#_accessor_storage", Some(accessor_storage_counter)); + accessor_storage_counter += 1; + name + }; + let mut backing_name = base_name; + let mut suffix: usize = 2; + while taken_private_names.contains_key(backing_name) { + backing_name = p.bump_name(base_name, Some(suffix)); + suffix += 1; + } + taken_private_names.insert(backing_name, ()); + + let backing_kind = if prop.flags.contains(Flags::Property::IsStatic) { + js_ast::symbol::Kind::PrivateStaticField + } else { + js_ast::symbol::Kind::PrivateField + }; + let backing_ref = p.new_sym(backing_kind, backing_name); + + // #backing = init; + let mut backing_flags = prop.flags; + backing_flags.remove(Flags::Property::IsComputed); + new_properties.push(Property { + kind: PropertyKind::Normal, + flags: backing_flags, + key: Some(p.new_expr(E::PrivateIdentifier { ref_: backing_ref }, key_expr.loc)), + initializer: prop.initializer, + ..Default::default() + }); + + // Computed keys must evaluate exactly once: assign the key to + // a temporary in the getter's key position and reuse it for + // the setter. + let (getter_key, setter_key) = if is_computed { + computed_key_counter += 1; + let key_name: &'a [u8] = if computed_key_counter == 1 { + b"_computedKey" + } else { + p.bump_name(b"_computedKey", Some(computed_key_counter)) + }; + let key_ref = p.new_sym(js_ast::symbol::Kind::Other, key_name); + let binding = p.b(B::Identifier { r#ref: key_ref }, key_expr.loc); + computed_key_decls.push(G::Decl { + binding, + value: None, + }); + ( + Some(p.assign_to(key_ref, key_expr, key_expr.loc)), + Some(p.use_ref(key_ref, key_expr.loc)), + ) + } else { + (prop.key, prop.key) + }; + + // get key() { return this.#backing; } + p.record_usage(backing_ref); + let this_e = p.new_expr(E::This {}, loc); + let priv_e = p.new_expr(E::PrivateIdentifier { ref_: backing_ref }, loc); + let get_return = p.new_expr( + E::Index { + target: this_e, + index: priv_e, + optional_chain: None, + }, + loc, + ); + + // set key(v) { this.#backing = v; } + let setter_param_ref = p.new_sym(js_ast::symbol::Kind::Other, b"v"); + p.record_usage(backing_ref); + let this_e2 = p.new_expr(E::This {}, loc); + let priv_e2 = p.new_expr(E::PrivateIdentifier { ref_: backing_ref }, loc); + let set_target = p.new_expr( + E::Index { + target: this_e2, + index: priv_e2, + optional_chain: None, + }, + loc, + ); + let v_e = p.use_ref(setter_param_ref, loc); + let set_expr = Expr::assign(set_target, v_e); + + p.push_accessor_get_set_pair( + &mut new_properties, + prop.flags, + getter_key, + setter_key, + get_return, + setter_param_ref, + set_expr, + loc, + ); + } + + class.properties = bun_ast::StoreSlice::new_mut(new_properties.into_bump_slice_mut()); + p.taken_private_names = Some(taken_private_names); + + if !computed_key_decls.is_empty() { + let decls = DeclList::from_bump_vec(computed_key_decls); + let decl_stmt = p.s( + S::Local { + decls, + ..Default::default() + }, + loc, + ); + if is_expr { + if let Some(stmt_list) = p.nearest_stmt_list_mut() { + stmt_list.push(decl_stmt); + } + } else { + out.push(decl_stmt); + } + } + } + + if is_expr { + let class_expr = p.new_expr(class_copy(class), loc); + out.push(p.s( + S::SExpr { + value: class_expr, + ..Default::default() + }, + loc, + )); + } else { + out.push(original_stmt.expect("infallible: statement mode")); + } + } + // ── Core lowering ──────────────────────────────────── #[allow(clippy::too_many_lines)] @@ -1114,6 +1419,19 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let p = self; let bump = p.arena; + // Classes whose only lowering-relevant feature is `accessor` members + // take the in-place path; the machinery below is for decorators. + if class.ts_decorators.len_u32() == 0 + && class + .properties + .slice() + .iter() + .all(|prop| prop.ts_decorators.len_u32() == 0) + { + p.lower_auto_accessors_in_place(class, loc, is_expr, original_stmt, out); + return; + } + // Receiver-capture temporaries created by `rewrite_private_accesses_in_expr` // land in `temp_refs_to_declare`; everything pushed past this point is // declared in a `var` statement alongside the other lowering variables @@ -1558,72 +1876,51 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let wme = p.new_weak_map_expr(loc); prefix_stmts.push(p.var_decl(wm_ref, Some(wme), loc)); - // Getter: get foo() { return __privateGet(this, _foo); } + // Computed keys must evaluate exactly once: assign the + // key to a temporary in the getter's key position and + // reuse it for the setter. + let (getter_key, setter_key) = + if prop.flags.contains(Flags::Property::IsComputed) + && let Some(key_expr) = prop.key + { + computed_key_counter += 1; + let key_name: &'a [u8] = if computed_key_counter == 1 { + b"_computedKey" + } else { + p.bump_name(b"_computedKey", Some(computed_key_counter)) + }; + let key_ref = p.new_sym(js_ast::symbol::Kind::Other, key_name); + prefix_stmts.push(p.var_decl(key_ref, None, key_expr.loc)); + ( + Some(p.assign_to(key_ref, key_expr, key_expr.loc)), + Some(p.use_ref(key_ref, key_expr.loc)), + ) + } else { + (prop.key, prop.key) + }; + + // get foo() { return __privateGet(this, _foo); } let this_e = p.new_expr(E::This {}, loc); let wm_e = p.use_ref(wm_ref, loc); let get_ret = p.call_rt(loc, b"__privateGet", &[this_e, wm_e]); - let get_body = bump.alloc_slice_copy(&[p.s( - S::Return { - value: Some(get_ret), - }, - loc, - )]); - let get_fn = G::Fn { - body: G::FnBody { - stmts: bun_ast::StoreSlice::new_mut(get_body), - loc, - }, - ..Default::default() - }; - // Setter: set foo(v) { __privateSet(this, _foo, v); } + // set foo(v) { __privateSet(this, _foo, v); } let setter_param_ref = p.new_sym(js_ast::symbol::Kind::Other, b"v"); let this_e2 = p.new_expr(E::This {}, loc); let wm_e2 = p.use_ref(wm_ref, loc); let v_e = p.use_ref(setter_param_ref, loc); let set_call = p.call_rt(loc, b"__privateSet", &[this_e2, wm_e2, v_e]); - let set_body = bump.alloc_slice_copy(&[p.s( - S::SExpr { - value: set_call, - ..Default::default() - }, - loc, - )]); - let setter_binding = p.b( - B::Identifier { - r#ref: setter_param_ref, - }, + + p.push_accessor_get_set_pair( + &mut new_properties, + prop.flags, + getter_key, + setter_key, + get_ret, + setter_param_ref, + set_call, loc, ); - let setter_fn_args = bump.alloc(G::Arg { - binding: setter_binding, - ..Default::default() - }); - let set_fn = G::Fn { - args: bun_ast::StoreSlice::new_mut(core::slice::from_mut(setter_fn_args)), - body: G::FnBody { - stmts: bun_ast::StoreSlice::new_mut(set_body), - loc, - }, - ..Default::default() - }; - - let mut getter_flags = prop.flags; - getter_flags.insert(Flags::Property::IsMethod); - new_properties.push(Property { - key: prop.key, - value: Some(p.new_expr(E::Function { func: get_fn }, loc)), - kind: PropertyKind::Get, - flags: getter_flags, - ..Default::default() - }); - new_properties.push(Property { - key: prop.key, - value: Some(p.new_expr(E::Function { func: set_fn }, loc)), - kind: PropertyKind::Set, - flags: getter_flags, - ..Default::default() - }); let init_val = prop .initializer diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index e14684e6a2f2..37d080eb99a2 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -604,6 +604,12 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { /// Name from assignment context for anonymous decorated class expressions. /// Set before visitExpr, consumed by lowerStandardDecoratorsImpl. pub(crate) decorator_class_name: Option<&'a [u8]>, + + /// Every private name in use, built lazily by `lower_auto_accessors_in_place` + /// on its first call (all parse-time private declarations exist in `symbols` + /// by then) and extended with each generated backing name, so later classes + /// reuse it instead of rescanning the symbol table. + pub(crate) taken_private_names: Option>, } // `binding::ToExprWrapper` type-erases `*P` (which is generic over @@ -8847,6 +8853,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O will_wrap_module_in_try_catch_for_using: false, nearest_stmt_list: None, decorator_class_name: None, + taken_private_names: None, jsx_transform, diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index dfce8bec1923..d0d427ed9f6e 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -812,12 +812,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } // We only inject a name into classes when decorator lowering - // needs one: legacy TS decorators (`has_decorators`) or - // standard decorator lowering, which also covers classes with - // only auto-accessor fields and no decorators. - if class.class.has_decorators - || class.class.should_lower_standard_decorators - { + // needs one, i.e. when the class actually has decorators + // (legacy TS or standard). Classes with only auto-accessor + // fields and no decorators are lowered in place, stay + // anonymous, and keep `.name === "default"`. + if class.class.has_decorators { if class.class.class_name.is_none() || class.class.class_name.unwrap().ref_.is_empty() { diff --git a/test/bundler/transpiler/es-decorators.test.ts b/test/bundler/transpiler/es-decorators.test.ts index bdc61e68c48d..364e714eb285 100644 --- a/test/bundler/transpiler/es-decorators.test.ts +++ b/test/bundler/transpiler/es-decorators.test.ts @@ -1210,4 +1210,274 @@ describe("ES Decorators", () => { expect(exitCode).toBe(0); }); }); + + // https://github.com/oven-sh/bun/issues/31921 + // Classes with accessor members but no decorators are lowered in place + // (private backing field + getter/setter); nothing is relocated out of the + // class body, so native #names stay in scope and static elements keep + // their source order. + describe.concurrent("auto-accessor without decorators", () => { + test("static accessor initializer can reference a private member", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + const C = class Foo { + static #m = function (tag) { return { tag }; }; + static accessor a = Foo.#m("a").tag; + }; + console.log(C.a); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("a\n"); + expect(exitCode).toBe(0); + }); + + test("static block can reference a private member", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + const C = class Foo { + static accessor a = 1; + static #m = 5; + static { console.log(this.#m); } + }; + console.log(C.a); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("5\n1\n"); + expect(exitCode).toBe(0); + }); + + test("static accessor initializers and static blocks run in source order", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + const order = []; + const C = class { + static accessor a = (order.push("a"), 1); + static { order.push("block"); } + static accessor b = (order.push("b"), 2); + }; + console.log(order.join(","), C.a, C.b); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("a,block,b 1 2\n"); + expect(exitCode).toBe(0); + }); + + test("static accessor initializers run in source order with static fields", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + const order = []; + class C { + static a = (order.push("a"), 1); + static accessor b = (order.push("b"), 2); + static c = (order.push("c"), C.b); + } + console.log(order.join(","), C.a, C.b, C.c); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("a,b,c 1 2 2\n"); + expect(exitCode).toBe(0); + }); + + test("instance accessor initializers run in source order with plain fields", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + const order = []; + class C { + accessor a = (order.push("a"), 1); + b = (order.push("b"), this.a + 1); + } + const c = new C(); + console.log(order.join(","), c.a, c.b); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("a,b 1 2\n"); + expect(exitCode).toBe(0); + }); + + test("instance accessor initializer can reference a private field", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class D { + #p = 5; + accessor a = this.#p + 1; + } + console.log(new D().a); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("6\n"); + expect(exitCode).toBe(0); + }); + + test("private auto-accessor", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class Bar { + static accessor #p = 7; + static read() { return Bar.#p; } + static bump() { Bar.#p++; } + } + console.log(Bar.read()); + Bar.bump(); + console.log(Bar.read()); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("7\n8\n"); + expect(exitCode).toBe(0); + }); + + test("instance private auto-accessor", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class C { + accessor #z = 5; + get z() { return this.#z; } + set z(v) { this.#z = v; } + } + const c = new C(); + console.log(c.z); + c.z = 50; + console.log(c.z); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("5\n50\n"); + expect(exitCode).toBe(0); + }); + + test("instance and static accessors may share a name", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class C { + accessor x = 1; + static accessor x = 2; + } + const c = new C(); + c.x = 10; + C.x = 20; + console.log(c.x, C.x); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("10 20\n"); + expect(exitCode).toBe(0); + }); + + test("accessor lowers to a private backing field inside the class body", () => { + const transpiler = new Bun.Transpiler({ loader: "js", target: "bun" }); + expect(transpiler.transformSync(`class C { accessor x = 1; }`)).toMatchInlineSnapshot(` + "class C { + #x = 1; + get x() { + return this.#x; + } + set x(v) { + this.#x = v; + } + } + " + `); + }); + + test("backing field name avoids the class's own private names", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class Foo { + #a = 1; + accessor a = 2; + readPriv() { return this.#a; } + } + const f = new Foo(); + f.a = 3; + console.log(f.a, f.readPriv()); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("3 1\n"); + expect(exitCode).toBe(0); + }); + + // https://github.com/oven-sh/bun/issues/29837 + test("subclass can override an accessor of the same name", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class A { + accessor name = "A"; + } + class B extends A { + accessor name = "B"; + logName() { + console.log(this.name); + console.log(super.name); + } + } + new B().logName(); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("B\nA\n"); + expect(exitCode).toBe(0); + }); + + test("backing field name does not capture an enclosing class's private name", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + class Outer { + static #a = 5; + static Inner = class { + static accessor a = Outer.#a; + }; + } + console.log(Outer.Inner.a); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("5\n"); + expect(exitCode).toBe(0); + }); + + test("computed accessor keys evaluate once", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + let n = 0; + const key = () => (n++, "k" + n); + class Baz { + accessor [key()] = 4; + } + const b = new Baz(); + console.log(n, b.k1); + b.k1 = 8; + console.log(b.k1); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("1 4\n8\n"); + expect(exitCode).toBe(0); + }); + + test("computed accessor keys evaluate once in a decorated class", async () => { + const { stdout, stderr, exitCode } = await runDecorator(` + let n = 0; + const key = () => (n++, "k" + n); + function dec(v, ctx) {} + class Baz { + @dec m() {} + accessor [key()] = 4; + } + const b = new Baz(); + console.log(n, b.k1); + b.k1 = 8; + console.log(b.k1); + `); + expect(stderr).toBe(""); + expect(stdout).toBe("1 4\n8\n"); + expect(exitCode).toBe(0); + }); + + test("export default accessor-only class keeps name 'default'", async () => { + using dir = tempDir("es-dec-accessor-default-name", { + "entry.js": ` + import Cls from "./mod.js"; + console.log(Cls.name, Cls.a); + `, + "mod.js": ` + export default class { + static accessor a = 1; + } + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, rawStderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(filterStderr(rawStderr)).toBe(""); + expect(stdout).toBe("default 1\n"); + expect(exitCode).toBe(0); + }); + }); });