Skip to content

fix(transformer/async-to-generator): reparent parameter initializer scopes#22507

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-async-to-generator-parameter-scopes
May 17, 2026
Merged

fix(transformer/async-to-generator): reparent parameter initializer scopes#22507
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-async-to-generator-parameter-scopes

Conversation

@camc314

@camc314 camc314 commented May 17, 2026

Copy link
Copy Markdown
Contributor

Fixes a semantic mismatch in async_to_generator for async methods with parameter default initializers.

Before this change, the transform reused the original method scope as the inner generator scope and moved parameter bindings to the wrapper scope. Scopes created inside parameter defaults were left parented under the generator, even though the transformed AST places those defaults with the wrapper parameters.

For example:

class C {
  async method(value = function fallback() {}) {}
}

Conceptually this became:

class C {
  method(value = function fallback() {}) {
    // `value` belongs to this wrapper scope.

    return asyncToGenerator(function* () {
      // Before: the scope for `fallback` was still parented here,
      // because this generator reused the original async method scope.
    })();
  }
}

A fresh semantic rebuild from the transformed AST sees it differently:

class C {
  method(value = function fallback() {
    // After rebuild: `fallback` is created from the parameter default,
    // so its direct parent is the wrapper method scope.
  }) {
    return asyncToGenerator(function* () {
      // The generator should only own body scopes.
    })();
  }
}

This updates BindingMover so that when parameter bindings move to the wrapper, direct scopes created inside parameter-side expressions are reparented there too. It still stops at function/class scope boundaries, so nested body scopes are not traversed or moved.

@github-actions github-actions Bot added the A-transformer Area - Transformer / Transpiler label May 17, 2026
@camc314 camc314 marked this pull request as ready for review May 17, 2026 17:40
Copilot AI review requested due to automatic review settings May 17, 2026 17:40
@codspeed-hq

codspeed-hq Bot commented May 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing codex/fix-async-to-generator-parameter-scopes (d16bcb3) with main (ecfd3ca)

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes scope parenting in the ES2017 async-to-generator transform when formal parameters are moved to a wrapper function scope, by ensuring scopes created by parameter initializer expressions are reparented appropriately. This reduces semantic rebuild mismatches and improves conformance/coverage snapshot results.

Changes:

  • Extend BindingMover to also traverse parameter initializer expressions and reparent any direct function/arrow/class scopes found there to the target scope.
  • Ensure initializer expressions reached via assignment patterns and computed destructuring keys are visited (for scope reparenting), while still avoiding moving bindings declared inside those scopes.
  • Update transform conformance and test262 semantic coverage snapshots reflecting improved pass rates and fewer scope parent mismatch reports.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
crates/oxc_transformer/src/es2017/async_to_generator.rs Reparents scopes originating from parameter initializer expressions when moving parameter bindings to the wrapper scope.
tasks/transform_conformance/snapshots/oxc.snap.md Snapshot update showing babel-plugin-transform-async-to-generator now fully passing in the conformance set.
tasks/coverage/snapshots/semantic_test262.snap Snapshot update reflecting improved Positive Passed count and removal of many scope parent mismatch entries.

@camc314 camc314 self-assigned this May 17, 2026
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 17, 2026

camc314 commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

…copes (#22507)

Fixes a semantic mismatch in `async_to_generator` for async methods with parameter default initializers.

Before this change, the transform reused the original method scope as the inner generator scope and moved parameter bindings to the wrapper scope. Scopes created inside parameter defaults were left parented under the generator, even though the transformed AST places those defaults with the wrapper parameters.

For example:

```js
class C {
  async method(value = function fallback() {}) {}
}
```

Conceptually this became:

```js
class C {
  method(value = function fallback() {}) {
    // `value` belongs to this wrapper scope.

    return asyncToGenerator(function* () {
      // Before: the scope for `fallback` was still parented here,
      // because this generator reused the original async method scope.
    })();
  }
}
```

A fresh semantic rebuild from the transformed AST sees it differently:

```js
class C {
  method(value = function fallback() {
    // After rebuild: `fallback` is created from the parameter default,
    // so its direct parent is the wrapper method scope.
  }) {
    return asyncToGenerator(function* () {
      // The generator should only own body scopes.
    })();
  }
}
```

This updates `BindingMover` so that when parameter bindings move to the wrapper, direct scopes created inside parameter-side expressions are reparented there too. It still stops at function/class scope boundaries, so nested body scopes are not traversed or moved.
@graphite-app graphite-app Bot force-pushed the codex/fix-async-to-generator-parameter-scopes branch from d16bcb3 to c73c159 Compare May 17, 2026 19:36
@graphite-app graphite-app Bot merged commit c73c159 into main May 17, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 17, 2026
@graphite-app graphite-app Bot deleted the codex/fix-async-to-generator-parameter-scopes branch May 17, 2026 19:39
camc314 pushed a commit that referenced this pull request May 18, 2026
### 🐛 Bug Fixes

- 0f26de6 ecmascript: Resolve identifier value type via tracked
constants (#22234) (Alexander Lichter)
- c27a8cf minifier: Normalize `{ x: x }` shorthand so adjacent-if merge
is idempotent (#22401) (Dunqing)
- e431a0e parser: Break extends clause loop on fatal error (#22517)
(Boshen)
- e9ec7c6 minifier: Fold optional chains by base nullishness (#22236)
(Alexander Lichter)
- e6090e7 transformer: Keep enum IIFE when a non-inlinable value
reference remains (#22501) (Dunqing)
- 931b7d6 transformer: Inline const enum members through type-cast
wrappers (#22500) (Dunqing)
- b9615b2 codegen: Preserve string quotes in require() calls during
minification (#22475) (zennnnnnn11)
- c73c159 transformer/async-to-generator: Reparent parameter initializer
scopes (#22507) (camc314)
- ecfd3ca transformer/async-to-generator: Move only parameter bindings
(#22503) (camc314)
- 3ce3431 transformer/explicit-resource-managment: Preserve shadowed
for-head block (#22451) (camc314)

### ⚡ Performance

- ce92c6c semantic: `#[inline]` `Scoping::get_binding` (#22414)
(Dunqing)
- 98be95c regular_expression: Track regex flags via bitflags (#22427)
(Boshen)
- dbbc059 jsdoc: Skip should_attach_jsdoc when no remaining comments
(#22409) (Boshen)
- 217d7d8 minifier: Index `SymbolValues` by `SymbolId` (#22441)
(Dunqing)
- d782b78 minifier: Use BitSet for LiveUsageCollector live references
(#22425) (Boshen)
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.

2 participants