diff --git a/src/bundler/cache.rs b/src/bundler/cache.rs index 72f90118c3c..34661b3ce88 100644 --- a/src/bundler/cache.rs +++ b/src/bundler/cache.rs @@ -28,7 +28,7 @@ use bun_ast::RuntimeTranspilerCache; /// Bump when the cache wire format or parser output changes. Mirrors /// `expected_version` in src/jsc/RuntimeTranspilerCache.zig. -pub const RUNTIME_TRANSPILER_CACHE_VERSION: u32 = 20; +pub const RUNTIME_TRANSPILER_CACHE_VERSION: u32 = 21; /// Mirrors the Zig `pub var is_disabled` mutable global — written by T6 /// (src/runtime/cli/Arguments.zig:1603, src/jsc/VirtualMachine.zig:1383) and diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index 0e3e0ff6ce8..c69201f5a23 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -1606,8 +1606,22 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // Ignore declarations if the scope is shadowed by a direct "eval" call. // The eval'd code may indirectly reference this symbol and the actual // use count may be greater than 1. + // + // Ignore declarations inside a switch case body: every case in a switch + // shares one lexical scope but we visit one case at a time, so + // `use_count_estimate` for a decl in case 0 has not yet seen references + // from later cases and may spuriously read as 1 — inlining then would + // delete the decl out from under those later references (issue #30932). + // `is_inside_switch` resets at function boundaries (it lives on + // `FnOrArrowDataVisit`), so nested functions inside a case body still + // get the optimization. Unlike `StmtsKind::SwitchStmt`, using this + // flag keeps the using-lowering path at L1488 independent from the + // case-body guard here. // SAFETY: current_scope is a valid arena ptr for the parse. - if p.current_scope != p.module_scope && !p.current_scope().contains_direct_eval { + if !p.fn_or_arrow_data_visit.is_inside_switch + && p.current_scope != p.module_scope + && !p.current_scope().contains_direct_eval + { // Keep inlining variables until a failure or until there are none left. // That handles cases like this: // diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index 0c900fa2fda..7b176a5b092 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -2153,6 +2153,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O { p.push_scope_for_visit_pass(js_ast::scope::Kind::Block, data.body_loc) .expect("unreachable"); + // Every case body shares one lexical scope but is entered conditionally, + // so a `const` declared in one case does not dominate references to that + // name in other cases. Disable the const-local-prefix inlining for the + // entire switch body — otherwise `case 'a': console.log(X)` could be + // rewritten to use the value from `case '*': const X = 2;` above it and + // silently bypass the TDZ error the spec requires (issue #30932). + p.cur_scope().is_after_const_local_prefix = true; let old_is_inside_switch = p.fn_or_arrow_data_visit.is_inside_switch; p.fn_or_arrow_data_visit.is_inside_switch = true; let cases = data.cases.slice_mut(); @@ -2164,6 +2171,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // p.warnAboutTypeofAndString(s.Test, *c.Value) } let mut _stmts = stmts_to_list(p.arena, cases[i].body); + // Passes `StmtsKind::None` (not `SwitchStmt`) so the using-lowering + // path in `visit_stmts` stays enabled for each case body. The + // single-use-substitution pass is instead gated on + // `fn_or_arrow_data_visit.is_inside_switch`, which is set above. p.visit_stmts(&mut _stmts, StmtsKind::None) .expect("unreachable"); cases[i].body = list_to_stmts(_stmts); diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 6c60627bd25..ebcfa7fa70e 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -39,7 +39,8 @@ bun_core::declare_scope!(cache, visible); /// Version 18: Include ESM record (module info) with an ES Module, see #15758 /// Version 19: Sourcemap blob is InternalSourceMap (varint stream + sync points), not VLQ. /// Version 20: InternalSourceMap stream is bit-packed windows. -const EXPECTED_VERSION: u32 = 20; +/// Version 21: Parser no longer inlines const declarations across switch case bodies (#30932). +const EXPECTED_VERSION: u32 = 21; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/test/regression/issue/30932.test.ts b/test/regression/issue/30932.test.ts new file mode 100644 index 00000000000..12d664ce871 --- /dev/null +++ b/test/regression/issue/30932.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from "bun:test"; + +// https://github.com/oven-sh/bun/issues/30932 (and #18477) +// +// Every case in a switch shares one lexical scope but each case is entered +// conditionally, so a `const` declared in one case does not dominate references +// to the same name in sibling cases. Two inlining passes in the parser used to +// ignore this and produced code that silently bypassed the spec-required TDZ +// `ReferenceError`: +// +// 1. const-local-prefix inlining — `const X = literal` in the first case was +// recorded and every later reference to `X` in the switch body was +// rewritten to `literal`. +// 2. single-use substitution — `visit_stmts` ran once per case body but +// `use_count_estimate` is global, so when visiting `case "*"` the +// counter had only seen `case "*"`s references. `const X = expr` + +// `return X` read as single-use and the decl was deleted, leaving +// sibling-case references dangling. +describe("switch-case const does not leak across cases", () => { + test("literal-initialized const is not inlined into sibling case", () => { + function test(action: string) { + switch (action) { + case "*": + const CONSTANT = 2; + return "matched " + CONSTANT; + case "a": + return "a=" + CONSTANT; + } + } + expect(() => test("a")).toThrow(ReferenceError); + expect(test("*")).toBe("matched 2"); + }); + + test("non-foldable const is not substituted out of sibling case", () => { + function test(action: string) { + switch (action) { + case "*": + const X = Math.random(); + return "* " + X; + case "a": + return "a " + X; + } + } + // Post-fix JSC raises "Cannot access 'X' before initialization" (TDZ). + // Pre-fix, Bun deleted the `const X` decl entirely as a single-use + // substitution while visiting case "*", turning the case "a" reference + // into an unbound "X is not defined" — also a ReferenceError, but with + // different semantics — so match on name + message shape. + expect(() => test("a")).toThrow( + expect.objectContaining({ + name: "ReferenceError", + message: expect.stringContaining("before initialization"), + }), + ); + }); + + test("outer const is shadowed by inner const, not leaked through", () => { + const CONSTANT = 1; + function test(action: string) { + switch (action) { + case "*": + const CONSTANT = 2; + return "* " + CONSTANT; + case "a": + return "a " + CONSTANT; + } + return "outer=" + CONSTANT; + } + // The inner `const CONSTANT` hoists over the entire switch body and + // shadows the outer one, so touching it from `case "a"` before the + // `case "*"` declaration has evaluated is TDZ. + expect(() => test("a")).toThrow(ReferenceError); + expect(test("*")).toBe("* 2"); + // Outside the switch body the outer `CONSTANT` is still visible. + expect(test("other")).toBe("outer=1"); + }); + + test("declaration + use in the same case still works", () => { + function test() { + switch ("a") { + case "a": { + const X = 42; + return X; + } + } + } + expect(test()).toBe(42); + }); + + test("fall-through from declaring case still initializes binding", () => { + function test() { + switch ("a") { + case "a": + const X = 100; + case "b": + return "X=" + X; + } + } + expect(test()).toBe("X=100"); + }); +});