Skip to content

feat(transformer/typescript): debug_assert that enum_eval ran in semantic#22252

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix-transformer-enum-eval-debug-assert
May 10, 2026
Merged

feat(transformer/typescript): debug_assert that enum_eval ran in semantic#22252
graphite-app[bot] merged 1 commit into
mainfrom
fix-transformer-enum-eval-debug-assert

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented May 8, 2026

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:

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:

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

  • 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)
  • cargo run -p oxc_transform_conformance — no snapshot changes (the conformance runner goes through CompilerInterface, which already enables enum_eval)
  • cargo clippy -p oxc_transformer --tests — clean

@github-actions github-actions Bot added the A-transformer Area - Transformer / Transpiler label May 8, 2026
Copy link
Copy Markdown
Member Author

Dunqing commented May 8, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of 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.

@Dunqing Dunqing changed the title fix(minifier): refresh direct eval flags after DCE (#21787) feat(transformer/typescript): debug_assert that enum_eval ran in semantic May 8, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix-transformer-enum-eval-debug-assert (6fc3230) with main (b154559)2

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (6749b02) during the generation of this report, so b154559 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Dunqing Dunqing force-pushed the fix-transformer-enum-eval-debug-assert branch from 9ba0666 to 4f83cc1 Compare May 8, 2026 09:35
@Dunqing Dunqing changed the base branch from main to chore-tasks-enable-enum-eval-for-transformer May 8, 2026 09:35
@Dunqing Dunqing force-pushed the fix-transformer-enum-eval-debug-assert branch 2 times, most recently from d8f4d70 to 603d1e5 Compare May 8, 2026 10:11
@Dunqing Dunqing marked this pull request as ready for review May 8, 2026 10:15
@Dunqing Dunqing requested a review from overlookmotel as a code owner May 8, 2026 10:15
@Dunqing Dunqing requested a review from sapphi-red May 8, 2026 10:15
@graphite-app graphite-app Bot changed the base branch from chore-tasks-enable-enum-eval-for-transformer to graphite-base/22252 May 9, 2026 00:11
graphite-app Bot pushed a commit that referenced this pull request May 9, 2026
#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
@graphite-app graphite-app Bot force-pushed the fix-transformer-enum-eval-debug-assert branch from 603d1e5 to 5b20d12 Compare May 9, 2026 00:15
@graphite-app graphite-app Bot force-pushed the graphite-base/22252 branch from b8ca72b to 6749b02 Compare May 9, 2026 00:15
@graphite-app graphite-app Bot changed the base branch from graphite-base/22252 to main May 9, 2026 00:16
@graphite-app graphite-app Bot force-pushed the fix-transformer-enum-eval-debug-assert branch from 5b20d12 to 6fc3230 Compare May 9, 2026 00:16
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label May 10, 2026
Copy link
Copy Markdown
Member Author

Dunqing commented May 10, 2026

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
@graphite-app graphite-app Bot force-pushed the fix-transformer-enum-eval-debug-assert branch from 6fc3230 to 66c9b01 Compare May 10, 2026 09:36
@graphite-app graphite-app Bot merged commit 66c9b01 into main May 10, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 10, 2026
@graphite-app graphite-app Bot deleted the fix-transformer-enum-eval-debug-assert branch May 10, 2026 09:39
camc314 added a commit that referenced this pull request May 11, 2026
### 🚀 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>
graphite-app Bot pushed a commit to rolldown/rolldown that referenced this pull request May 12, 2026
…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.
IWANABETHATGUY pushed a commit to rolldown/rolldown that referenced this pull request May 18, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transformer: string enum with alias member produces incorrect reverse mapping

2 participants