diff --git a/src/ast/g.rs b/src/ast/g.rs index 92cd31a4712b..e43881f20633 100644 --- a/src/ast/g.rs +++ b/src/ast/g.rs @@ -101,7 +101,12 @@ impl Class { return false; } - if property.kind == PropertyKind::Normal && f.contains(flags::Property::IsStatic) { + // Static auto-accessor initializers run at class-definition time + // just like static fields, so they get the same side-effect check. + if (property.kind == PropertyKind::Normal + || property.kind == PropertyKind::AutoAccessor) + && f.contains(flags::Property::IsStatic) + { for val in [property.value, property.initializer].into_iter().flatten() { match val.data { ExprData::EArrow(..) | ExprData::EFunction(..) => {} diff --git a/src/js_parser/lower/lower_decorators.rs b/src/js_parser/lower/lower_decorators.rs index 77c2620dac5e..93518f2a0ae2 100644 --- a/src/js_parser/lower/lower_decorators.rs +++ b/src/js_parser/lower/lower_decorators.rs @@ -1266,9 +1266,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O ); pre_eval_stmts.push(p.var_decl(dec_ref, Some(arr), loc)); } + // Auto-accessors duplicate the key across the synthesized get/set + // pair, so their computed keys must be hoisted to evaluate once. if prop.flags.contains(Flags::Property::IsComputed) && prop.key.is_some() - && prop.ts_decorators.len_u32() > 0 + && (prop.ts_decorators.len_u32() > 0 || prop.kind == PropertyKind::AutoAccessor) { computed_key_counter += 1; let key_name: &'a [u8] = if computed_key_counter == 1 { diff --git a/src/js_parser/parse/mod.rs b/src/js_parser/parse/mod.rs index 7b3519a9ba8a..08bff5e0d6b4 100644 --- a/src/js_parser/parse/mod.rs +++ b/src/js_parser/parse/mod.rs @@ -262,6 +262,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O p.lexer.expect(T::TCloseBrace)?; let has_any_decorators = has_decorators || class_opts.ts_decorators.len() > 0; + + // Auto-accessor classes lower through the standard-decorator path, which + // would silently misapply any legacy TS decorators in the same class. + if has_auto_accessor && !p.options.features.standard_decorators && has_any_decorators { + p.log().add_error( + Some(p.source), + class_keyword.loc, + b"Cannot mix the `accessor` keyword with `experimentalDecorators: true` in the same class. Use standard decorators instead.", + ); + } + // `Expr: Copy` — safe arena-slice → owned Vec (one memcpy, no double-drop). let ts_decorators = ExprNodeList::from_arena_slice(class_opts.ts_decorators); Ok(G::Class { @@ -273,8 +284,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O body_loc, properties: bun_ast::StoreSlice::new_mut(properties.into_bump_slice_mut()), has_decorators: has_any_decorators, - should_lower_standard_decorators: p.options.features.standard_decorators - && (has_any_decorators || has_auto_accessor), + should_lower_standard_decorators: has_auto_accessor + || (p.options.features.standard_decorators && has_any_decorators), }) } diff --git a/src/js_parser/parse/parse_property.rs b/src/js_parser/parse/parse_property.rs index 81fa6a8f0224..19e194ca9bf1 100644 --- a/src/js_parser/parse/parse_property.rs +++ b/src/js_parser/parse/parse_property.rs @@ -463,10 +463,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } PropertyModifierKeyword::PAccessor => { - // "accessor" keyword for auto-accessor fields (TC39 standard decorators) + // `accessor [no LineTerminator here] ClassElementName`, + // valid under either decorator mode (TC39 / TS 4.9+). if opts.is_class && !p.lexer.has_newline_before - && p.options.features.standard_decorators && PropertyModifierKeyword::find(raw) == Some(PropertyModifierKeyword::PAccessor) { diff --git a/test/regression/issue/29197.test.ts b/test/regression/issue/29197.test.ts new file mode 100644 index 000000000000..92d59c66da78 --- /dev/null +++ b/test/regression/issue/29197.test.ts @@ -0,0 +1,259 @@ +// https://github.com/oven-sh/bun/issues/29197 (and #27335) +// +// The `accessor` keyword (TC39 auto-accessors / TS 4.9+) was rejected as a +// syntax error when a project's tsconfig.json had `experimentalDecorators: true`. +// The keyword should be accepted under either decorator mode. JSC doesn't +// parse `accessor` natively, so any class with auto-accessors is routed +// through the standard-decorator lowering (WeakMap + getter/setter) +// regardless of mode. Mixing `accessor` with legacy TS decorators errors +// clearly instead of silently rerouting decorators through the standard +// runtime. + +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +async function runBun(cwd: string, ...args: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: bunEnv, + cwd, + stderr: "pipe", + stdout: "pipe", + }); + return await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); +} + +test.concurrent("accessor with various modifiers under experimentalDecorators: true", async () => { + using dir = tempDir("issue-29197-modifiers", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: true }, + }), + "main.ts": `class Foo { + accessor a = 1; + public accessor b = 2; + private accessor c = 3; + protected accessor d = 4; + static accessor e = 5; + readonly accessor f = 6; + getC() { return this.c; } + getD() { return this.d; } +} +const f = new Foo(); +console.log(f.a, f.b, f.getC(), f.getD(), Foo.e, f.f); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("1 2 3 4 5 6\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("accessor without tsconfig (TS file, no decorator flags)", async () => { + using dir = tempDir("issue-29197-plain", { + "main.ts": `class Foo { + accessor x: number = 42; +} +console.log(new Foo().x); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("42\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("accessor still works under standard decorators mode", async () => { + using dir = tempDir("issue-29197-std", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: false }, + }), + "main.ts": `function dec(value: any, context: any) { + console.log("dec", context.name, context.kind); +} +class Foo { + @dec accessor x: number = 7; +} +console.log(new Foo().x); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("dec x accessor\n7\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent( + "mixing accessor with experimentalDecorators legacy @dec is a clear error, not silent wrong semantics", + async () => { + using dir = tempDir("issue-29197-mixed", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: true }, + }), + "main.ts": `function legacyDec(target: any, key: string) {} + +class Foo { + @legacyDec + doThing() {} + + accessor x: number = 0; +} +`, + }); + + const [, stderr, exitCode] = await runBun(String(dir), "main.ts"); + expect(stderr).toContain("Cannot mix the `accessor` keyword with `experimentalDecorators: true`"); + expect(exitCode).not.toBe(0); + }, +); + +test.concurrent("static accessor field: direct access works; subclass access throws (TC39 spec)", async () => { + // The standard-decorator lowering stores static accessor state in a + // WeakMap keyed on the declaring class. `Counter.count` round-trips; a + // subclass access (`Sub.count`) invokes the inherited getter with + // `this === Sub`, which is not in the WeakMap — matches TC39's static + // private-field brand-check semantics (TypeError at the key lookup). + using dir = tempDir("issue-29197-subclass", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: false }, + }), + "main.ts": `class Counter { static accessor count = 10; } +class Sub extends Counter {} +console.log(Counter.count); +Counter.count = 99; +console.log(Counter.count); +try { + console.log("Sub.count=", Sub.count); +} catch (e) { + console.log("Sub caught:", (e as any).name); +} +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("10\n99\nSub caught: TypeError\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("accessor field in a class expression", async () => { + using dir = tempDir("issue-29197-expr", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: false }, + }), + "main.ts": `const Foo = class { accessor x = 1; }; +const f = new Foo(); +console.log(f.x); +f.x = 2; +console.log(f.x); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("1\n2\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent( + "newline between `accessor` and the name triggers ASI (two fields, not one auto-accessor)", + async () => { + // TC39 grammar: `accessor [no LineTerminator here] ClassElementName`. + // With a newline, `accessor` must be parsed as a plain field name + // terminated by ASI, and the following `y = 1` becomes a second + // data field — NOT a single auto-accessor `y`. Matches tsc/esbuild. + using dir = tempDir("issue-29197-asi", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: true }, + }), + "main.ts": `class C { + accessor + y = 1 +} +const c = new C() as any; +console.log("keys:", Object.getOwnPropertyNames(c).sort().join(",")); +console.log("accessor:", c.accessor); +console.log("y:", c.y); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("keys: accessor,y\naccessor: undefined\ny: 1\n"); + expect(exitCode).toBe(0); + }, +); + +test.concurrent( + "`static accessor` with a side-effecting initializer is not hoisted past preceding statements", + async () => { + // Non-bundle tree-shaking calls `G::Class::can_be_moved()` on the + // pre-visit AST. `can_be_moved` used to only inspect `.Normal` static + // initializers, so a class with `static accessor x = sideEffect()` was + // (incorrectly) treated as movable and hoisted ahead of preceding + // statements, inverting evaluation order. + using dir = tempDir("issue-29197-hoist", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: false }, + }), + "main.ts": + 'console.log("first");\n' + + "export class Foo {\n" + + ' static accessor x = (console.log("second"), 42);\n' + + "}\n" + + 'console.log("third");\n', + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("first\nsecond\nthird\n"); + expect(exitCode).toBe(0); + }, +); + +test.concurrent("anonymous `export default class` with a static accessor round-trips", async () => { + // Regression: `export default class { static accessor x = 1 }` used + // to trip a null-ref panic in `lower_standard_decorators_stmt` because + // `class.class_name` was only injected from `default_name` when the + // class had decorators. Auto-accessors go through the same lowering, + // so the name injection now also runs when any property is an + // `AutoAccessor`. + using dir = tempDir("issue-29197-default-export", { + "tsconfig.json": JSON.stringify({ + compilerOptions: { experimentalDecorators: false }, + }), + "base.ts": "export default class { static accessor x = 1; }\n", + "main.ts": + "import Base from './base';\n" + + "console.log('x=', Base.x);\n" + + "Base.x = 42;\n" + + "console.log('x=', Base.x);\n", + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("x= 1\nx= 42\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("accessor with a computed key evaluates the key exactly once", async () => { + // TC39 auto-accessor spec requires the PropertyName to be evaluated + // once. An undecorated `accessor [k()] = 1` lowers through + // `lower_decorators` into a `get [k()]` / `set [k()]` pair that shares + // `prop.key`; without the computed-key hoist (gated in older code on + // `ts_decorators.len > 0`), `k()` runs twice — breaking the spec and + // installing the getter/setter under different keys for a non-idempotent + // key. Widened the hoist gate to include `AutoAccessor`. + using dir = tempDir("issue-29197-computed-key", { + "main.ts": `let calls = 0; +const k = () => (calls++, "x"); +class C { + accessor [k()] = 42; +} +const c = new C() as any; +console.log("calls=", calls); +console.log("x=", c.x); +c.x = 99; +console.log("x=", c.x); +console.log("calls=", calls); +`, + }); + + const [stdout, , exitCode] = await runBun(String(dir), "main.ts"); + expect(stdout).toBe("calls= 1\nx= 42\nx= 99\ncalls= 1\n"); + expect(exitCode).toBe(0); +});