feat(transformer/typescript): debug_assert that enum_eval ran in semantic#22252
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
enum_eval ran in semantic
Merging this PR will not alter performance
Comparing Footnotes
|
9ba0666 to
4f83cc1
Compare
d8f4d70 to
603d1e5
Compare
#22253) Pre-requisite for #22252. ## Why #22252 adds a `debug_assert!` in the transformer that requires `Scoping` to be built with `SemanticBuilder::with_enum_eval(true)` whenever a `TSEnumDeclaration` reaches the transform pass. The internal `Compiler` (`crates/oxc/src/compiler.rs`) and the playground already enable it; the CI run on #22252 surfaced a few internal callers that don't. Land that fix on its own first, so the assert PR doesn't trip CI on these tasks. ## Sites updated Every `SemanticBuilder` site that feeds `Transformer::build_with_scoping`: - `tasks/track_memory_allocations/src/lib.rs` — both call sites (`cargo allocs`). - `tasks/benchmark/benches/transformer.rs` - `tasks/benchmark/benches/pipeline.rs` - `tasks/benchmark/benches/minifier.rs` - `tasks/benchmark/benches/codegen.rs` Other semantic builders in these files that feed only the minifier / codegen / compressor are left alone — they don't go through enum lowering. ## Test plan - [x] `cargo check -p oxc_track_memory_allocations` — clean - [x] `cargo check --benches -p oxc_benchmark` — clean
603d1e5 to
5b20d12
Compare
b8ca72b to
6749b02
Compare
5b20d12 to
6fc3230
Compare
Merge activity
|
…mantic (#22252) Closes #21667. Stacked on #22253 (which lands the prerequisite `with_enum_eval(true)` calls in `cargo allocs` and the four transformer benches that this assert would otherwise trip). ## Problem The TS enum lowering reads pre-computed member values from `Scoping`. Those values are only populated when `SemanticBuilder::with_enum_eval(true)` is set; without it, every `get_enum_member_value` call returns `None`, so string-valued members are treated as opaque expressions and the transform emits incorrect reverse mappings. Example from #21667: ```ts enum Theme { Light = "Light", Dark = "Dark", Default = Theme.Light, } ``` When the transformer runs on a `Scoping` built without `enum_eval`, it does not recognise `Theme.Light` as a string and produces: ```js Theme[Theme["Default"] = Theme.Light] = "Default"; ``` — i.e. a reverse-mapping assignment for a string member, which is wrong (and runtime-observable). ## Fix Add a `debug_assert!` at the entry of `transform_ts_enum` (after the `decl.declare` early return) verifying that the enum's body scope is registered in `Scoping`. The check uses `get_enum_body_scopes` as the signal: `add_enum_body_scope` has exactly one caller (`evaluate_enum_members`), which is gated entirely by `enum_eval`. No new state on `Scoping` / `EnumData`. The internal `Compiler` (`crates/oxc/src/compiler.rs`) and the playground (`napi/playground/src/lib.rs`) already enable `enum_eval`. The benches and `cargo allocs` are fixed in the parent PR. This guard catches the misuse for any other external caller of `Transformer::build_with_scoping`. ## Test plan - [x] `cargo test -p oxc_transformer` — new integration tests in `tests/integrations/enum_eval.rs` cover both the positive case and the `#[should_panic]` case (debug-only) - [x] `cargo run -p oxc_transform_conformance` — no snapshot changes (the conformance runner goes through `CompilerInterface`, which already enables `enum_eval`) - [x] `cargo clippy -p oxc_transformer --tests` — clean
6fc3230 to
66c9b01
Compare
### 🚀 Features - 66c9b01 transformer/typescript: Debug_assert that `enum_eval` ran in semantic (#22252) (Dunqing) - ffe6475 minifier: Fold `Array` constructor with safe spreads (#22215) (camc314) ### 🐛 Bug Fixes - d3d0b18 traverse: Handle `ChainElement::TSNonNullExpression` in `GatherNodeParts` (#22247) (leaysgur) - 4e880de transformer/object-rest-spread: Declare temp vars for computed keys (#22284) (camc314) - a7c3e22 semantic: Clear member write target for computed keys (#22302) (camc314) - 6a8852d codegen: Emit newline after legal-comment orphan flush (#22304) (Dunqing) - 5da9fda transformer/explicit-resource-management: Preserve class names (#22306) (Dunqing) - b5d970f transformer/explicit-resource-management: Preserve class names (#22290) (camc314) - bc54fd4 minifier: Keep function / class names if direct eval is present in the scope (#22241) (sapphi-red) - 7a810c0 minifier: Refresh direct eval flags after DCE (#21787) (Dunqing) - dd88726 transformer/legacy-decorator: Preserve accessor type annotation for emitDecoratorMetadata (#21966) (Dunqing) - 29a3cd7 codegen: Swap mapping/indent order for top-level decls (#22206) (Dunqing) - 73b4f40 minifier: Preserve catch binding with direct eval (#22221) (camc314) - 0e13d17 minifier: Preserve optional chain base side effects (#22219) (camc314) - 0c7c01c transformer/typescript: Inline optional-chain enum member access (#21834) (Dunqing) - a6aff7e codegen: Emit block/array/object end mapping at close char (#22200) (Dunqing) - a099b03 codegen: Emit call end mapping at `)` position, not past it (#22199) (Dunqing) - 5753774 minifier: Cap if-return ternary collapse for firefox (#21841) (Gurupungav Narayanan) - 2493bdd codegen: Correct sourcemap end mappings for closing delimiters (#22001) (Mark Dalgleish) - 3b385e2 minifier: Bail optimizing `Array` with unknown arg count (#22188) (camc314) - 9fa2122 parser: Parse array computed class keys (#22159) (camc314) ### 📚 Documentation - a4a6892 napi/parser: Correct code comment (#22278) (overlookmotel) - 9305373 oxc: Update README (#22178) (camc314) Co-authored-by: Cameron <cameron.clark@hey.com>
…ansform (#9325) Related: oxc-project/oxc#22252 ## Summary Fixes the bug where `transformSync`, the vite TS transform plugin, and the bundler (when `define` triggers a program mutation) emitted a corrupt reverse-mapping line for string-enum alias members. For input ```ts enum Theme { Light = "Light", Dark = "Dark", Default = Theme.Light, } ``` rolldown produced ```js Theme[Theme["Default"] = Theme.Light] = "Default"; ``` which overwrites `Theme["Light"]`'s reverse mapping with `"Default"`. Expected (matching `tsc`): ```js Theme["Default"] = "Light"; ``` ## Cause The oxc TS transformer's enum lowering looks up each member's constant value via `Scoping::get_enum_member_value` (`crates/oxc_transformer/src/typescript/enum.rs:310`). That table is only populated when the `SemanticBuilder` is built with `with_enum_eval(true)`. When it isn't, every member returns `None`, so the transformer takes the non-string-init branch and emits the `Foo[Foo["x"] = init] = "x"` reverse-mapping form regardless of whether `init` is statically a string. The bundler's initial build (`crates/rolldown/src/utils/pre_process_ecma_ast.rs:66`) already enables `enum_eval`, but four other call sites that hand a `Scoping` to `Transformer::build_with_scoping` did not: - `transformSync` (`crates/rolldown_common/src/utils/enhanced_transform.rs`) — initial build and the fallback used after `ReplaceGlobalDefines` consumes the scoping. - The vite TS transform plugin (`crates/rolldown_plugin_vite_transform/src/lib.rs`). - The bundler's `recreate_scoping` (`crates/rolldown/src/utils/pre_process_ecma_ast.rs:238`), which feeds the transformer when the define plugin has consumed the original scoping. ## Fix Add `.with_enum_eval(true)` to all four call sites: - `crates/rolldown_common/src/utils/enhanced_transform.rs` — initial build before transformer. - `crates/rolldown_common/src/utils/enhanced_transform.rs` — fallback rebuild when `ReplaceGlobalDefines` consumed the original scoping. - `crates/rolldown_plugin_vite_transform/src/lib.rs` — vite TS transform plugin. - `crates/rolldown/src/utils/pre_process_ecma_ast.rs` — `recreate_scoping`, the fallback used by step 3 (transformer) when define has consumed scoping. Later callers of `recreate_scoping` (inject / DCE / PreProcessor) run after enums have been lowered to IIFEs, so the eval pass is a no-op there. Closes #9312 ## Test plan - [x] Added two `transformSync` regression tests in `packages/rolldown/tests/utils/transform.test.ts` (string-enum alias forward-only, numeric-enum alias reverse-mapping retained). - [x] Added a bundler fixture under `crates/rolldown/tests/rolldown/issues/9312/` covering define + string-enum-alias. - [x] All 24 existing enum-related Rust integration tests pass. - [x] All 38 define-related tests pass. - [x] `cargo clippy -p rolldown_common -p rolldown_plugin_vite_transform -- -D warnings` clean. - [x] Verified the issue's exact `transformSync` repro (string, numeric, mixed enums) matches `tsc` output.
…ansform (#9325) Related: oxc-project/oxc#22252 ## Summary Fixes the bug where `transformSync`, the vite TS transform plugin, and the bundler (when `define` triggers a program mutation) emitted a corrupt reverse-mapping line for string-enum alias members. For input ```ts enum Theme { Light = "Light", Dark = "Dark", Default = Theme.Light, } ``` rolldown produced ```js Theme[Theme["Default"] = Theme.Light] = "Default"; ``` which overwrites `Theme["Light"]`'s reverse mapping with `"Default"`. Expected (matching `tsc`): ```js Theme["Default"] = "Light"; ``` ## Cause The oxc TS transformer's enum lowering looks up each member's constant value via `Scoping::get_enum_member_value` (`crates/oxc_transformer/src/typescript/enum.rs:310`). That table is only populated when the `SemanticBuilder` is built with `with_enum_eval(true)`. When it isn't, every member returns `None`, so the transformer takes the non-string-init branch and emits the `Foo[Foo["x"] = init] = "x"` reverse-mapping form regardless of whether `init` is statically a string. The bundler's initial build (`crates/rolldown/src/utils/pre_process_ecma_ast.rs:66`) already enables `enum_eval`, but four other call sites that hand a `Scoping` to `Transformer::build_with_scoping` did not: - `transformSync` (`crates/rolldown_common/src/utils/enhanced_transform.rs`) — initial build and the fallback used after `ReplaceGlobalDefines` consumes the scoping. - The vite TS transform plugin (`crates/rolldown_plugin_vite_transform/src/lib.rs`). - The bundler's `recreate_scoping` (`crates/rolldown/src/utils/pre_process_ecma_ast.rs:238`), which feeds the transformer when the define plugin has consumed the original scoping. ## Fix Add `.with_enum_eval(true)` to all four call sites: - `crates/rolldown_common/src/utils/enhanced_transform.rs` — initial build before transformer. - `crates/rolldown_common/src/utils/enhanced_transform.rs` — fallback rebuild when `ReplaceGlobalDefines` consumed the original scoping. - `crates/rolldown_plugin_vite_transform/src/lib.rs` — vite TS transform plugin. - `crates/rolldown/src/utils/pre_process_ecma_ast.rs` — `recreate_scoping`, the fallback used by step 3 (transformer) when define has consumed scoping. Later callers of `recreate_scoping` (inject / DCE / PreProcessor) run after enums have been lowered to IIFEs, so the eval pass is a no-op there. Closes #9312 ## Test plan - [x] Added two `transformSync` regression tests in `packages/rolldown/tests/utils/transform.test.ts` (string-enum alias forward-only, numeric-enum alias reverse-mapping retained). - [x] Added a bundler fixture under `crates/rolldown/tests/rolldown/issues/9312/` covering define + string-enum-alias. - [x] All 24 existing enum-related Rust integration tests pass. - [x] All 38 define-related tests pass. - [x] `cargo clippy -p rolldown_common -p rolldown_plugin_vite_transform -- -D warnings` clean. - [x] Verified the issue's exact `transformSync` repro (string, numeric, mixed enums) matches `tsc` output.

Closes #21667.
Stacked on #22253 (which lands the prerequisite
with_enum_eval(true)calls incargo allocsand the four transformer benches that this assert would otherwise trip).Problem
The TS enum lowering reads pre-computed member values from
Scoping. Those values are only populated whenSemanticBuilder::with_enum_eval(true)is set; without it, everyget_enum_member_valuecall returnsNone, so string-valued members are treated as opaque expressions and the transform emits incorrect reverse mappings.Example from #21667:
When the transformer runs on a
Scopingbuilt withoutenum_eval, it does not recogniseTheme.Lightas a string and produces:— i.e. a reverse-mapping assignment for a string member, which is wrong (and runtime-observable).
Fix
Add a
debug_assert!at the entry oftransform_ts_enum(after thedecl.declareearly return) verifying that the enum's body scope is registered inScoping.The check uses
get_enum_body_scopesas the signal:add_enum_body_scopehas exactly one caller (evaluate_enum_members), which is gated entirely byenum_eval. No new state onScoping/EnumData.The internal
Compiler(crates/oxc/src/compiler.rs) and the playground (napi/playground/src/lib.rs) already enableenum_eval. The benches andcargo allocsare fixed in the parent PR. This guard catches the misuse for any other external caller ofTransformer::build_with_scoping.Test plan
cargo test -p oxc_transformer— new integration tests intests/integrations/enum_eval.rscover both the positive case and the#[should_panic]case (debug-only)cargo run -p oxc_transform_conformance— no snapshot changes (the conformance runner goes throughCompilerInterface, which already enablesenum_eval)cargo clippy -p oxc_transformer --tests— clean