feat(semantic): store `scope_id` in `Reference` by Dunqing · Pull Request #18053 · oxc-project/oxc

Dunqing · 2026-01-16T04:57:28Z

rolldown/rolldown#7936 relies on this. You can see how Rolldown uses this at description of that PR.

Additionally, it can also simplify the use case when you want to get the scope ID where the Reference occurs. See follow-up PR #18059

Dunqing · 2026-01-16T04:57:48Z

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 hot fixes, skip the queue and merge this PR next

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.

codspeed-hq · 2026-01-16T05:03:01Z

Merging this PR will not alter performance

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks¹

_{Comparing 01-16-feat_semantic_store_scope_id_in_reference (a4f011a) with main (c1c152b)}

3 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

Pull request overview

This pull request adds a scope_id field to the Reference struct to store the scope where each reference occurs, simplifying use cases that need to determine the scope of a reference. The change updates the core semantic analysis infrastructure and propagates the modification through all affected code paths and test snapshots.

Changes:

Added scope_id: ScopeId field to the Reference struct with corresponding getter method
Updated Reference::new() and Reference::new_with_symbol_id() constructors to accept scope_id parameter
Modified all call sites creating Reference instances to pass the current scope ID
Updated test snapshot generation to include scope_id in output
Regenerated all test snapshots to reflect the new field

Reviewed changes

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

Show a summary per file

File	Description
crates/oxc_syntax/src/reference.rs	Added `scope_id` field to `Reference` struct, updated constructors and added getter method
crates/oxc_traverse/src/context/scoping.rs	Updated `create_reference` and `create_unbound_reference` to pass `self.current_scope_id`
crates/oxc_semantic/src/builder.rs	Updated `reference_identifier` to pass `self.current_scope_id` when creating references
crates/oxc_semantic/tests/main.rs	Modified snapshot generation to include `scope_id` field in test output
crates/oxc_semantic/tests/fixtures/*/.snap	Regenerated all snapshot files to include `scope_id` for each reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Dunqing · 2026-01-19T02:48:35Z

I'd like to get this ship today to finish Rolldown's PR.

graphite-app · 2026-01-19T04:44:12Z

Merge activity

Jan 19, 4:44 AM UTC: Boshen added this pull request to the Graphite merge queue.
Jan 19, 5:00 AM UTC: Merged by the Graphite merge queue.

rolldown/rolldown#7936 relies on this. You can see how Rolldown uses this at description of that PR. Additionally, it can also simplify the use case when you want to get the scope ID where the `Reference` occurs. See follow-up PR #18059

…()` (#18059) Simplify the use cases mentioned in #18053. After that PR, we can access the `scope_id` directly from the `Reference`, eliminating the need for an intermediate step to get the node first. 😅: I didn't expect there would be a lot of performance improvement in the Mangler.

Note: waiting for oxc-project/oxc#18053 to merge to unblock this (Edit: merged, and already released) ## Summary Fixes: #6586 This PR refines the symbol renaming (deconflict) logic to correctly handle both **top-level symbol conflicts** and **nested scope shadowing**. The key insight is that these are two distinct problems requiring different solutions. Related fixes: - #7859 - #7867 - #7899 This PR essentially reverts all the logic implemented in the previous fixes while preserving the performance improvements introduced in #7867 (4%). --- ## 1. Top-Level Symbol Renaming When bundling multiple modules into a single chunk, symbols with the same name need unique identifiers. **Example:** ```js // lib.js export const value = 1; // other.js export const value = 2; // main.js import { value } from './lib.js'; import { value as otherValue } from './other.js'; console.log(value, otherValue); ``` **Output:** ```js const value = 1; // lib.js - keeps original name (entry module priority) const value$1 = 2; // other.js - renamed to avoid conflict console.log(value, value$1); ``` **Rules:** - Entry module symbols get naming priority - Subsequent conflicting symbols get suffixed: `$1`, `$2`, etc. - Reserved names (keywords, globals like `Object`, `Promise`) are always avoided --- ## 2. Nested Scope Symbol Renaming This handles cases where a nested binding (function parameter, catch clause, block-scoped variable) interacts with top-level symbols. ### Case A: NO renaming needed - Intentional shadowing When a nested binding shadows an import that **kept its original name**, no renaming is needed because JavaScript's scoping rules handle it correctly. **Example (`preserve_shadowing_param_name`):** ```js // lib.js export const Client = []; // main.js import { Client } from './lib.js'; // This param shadows the imported `Client`, but should NOT be renamed // since shadowing is intentional and doesn't cause conflicts at runtime. const Config = (Client) => { console.log(Client); // → refers to parameter }; console.log(Client); // → refers to import ``` **Output:** ```js const Client = []; const Config = (Client) => { // Parameter keeps its name ✓ console.log(Client); }; console.log(Client); ``` --- ### Case B: Star import member references - Renaming needed When a namespace import member (like `ns.foo`) is referenced inside a function, and a nested binding would capture that reference, the nested binding must be renamed. **Example (`argument-treeshaking-parameter-conflict`):** ```js // dep.js export let value = 0; export const mutate = () => value++; // main.js import * as dep from './dep'; function test(mutate) { // Parameter named 'mutate' dep.mutate('hello'); // Reference to dep.mutate (top-level) } test(); assert.strictEqual(dep.value, 1); ``` **Output:** ```js let value = 0; const mutate = () => value++; function test(mutate$1) { // Parameter renamed to mutate$1 mutate("hello"); // Correctly calls top-level mutate } test(); assert.strictEqual(value, 1); ``` **Why renaming is needed:** 1. The namespace import `dep.mutate` resolves to the top-level `mutate` function 2. Inside `test`, the parameter `mutate` would shadow the top-level `mutate` 3. The reference `dep.mutate('hello')` becomes just `mutate("hello")` after bundling 4. Without renaming the parameter, this call would incorrectly invoke the parameter instead of the top-level function 5. Solution: Rename the parameter to `mutate$1` so the reference correctly resolves to the top-level `mutate` --- ### Case C: Named imports - Renaming needed When a named import is renamed due to a top-level conflict, and a nested binding has the same name as the **renamed** import, that nested binding must be renamed to avoid capturing references. **Example (`basic_scoped`):** ```js // a.js const a = 'a.js'; export { a }; // main.js import { a as aJs } from './a'; const a = 'main.js'; // Local 'a' takes priority (entry module) function foo(a$1) { // Parameter named 'a$1' return [a$1, a, aJs]; // References: param, local, import } assert.deepEqual(foo('foo'), ['foo', 'main.js', 'a.js']); ``` **Output:** ```js const a$1 = "a.js"; // Import renamed (conflicts with local 'a') const a = "main.js"; // Local keeps name (entry module priority) function foo(a$1$1) { // Parameter renamed to a$1$1 return [a$1$1, a, a$1]; // Correctly resolves all three references } assert.deepEqual(foo("foo"), ["foo", "main.js", "a.js"]); ``` **Why renaming is needed:** 1. `a` from `a.js` is renamed to `a$1` (conflicts with local `a` in entry module) 2. The alias `aJs` resolves to the renamed `a$1` 3. Inside `foo`, the parameter `a$1` would capture the reference to `aJs` 4. Solution: Rename the parameter to `a$1$1` so `aJs` correctly resolves to the top-level `a$1` --- > **Technical Note:** Cases B and C use the same underlying mechanism to detect shadowing: > > 1. Get the `reference_id` from the reference site (where the symbol is used) > 2. Use `scoping.scope_ancestors(reference.scope_id())` to walk up the scope chain > 3. Check if any ancestor scope has a binding with the same name as the renamed symbol > 4. If found, rename that nested binding to avoid capture > > This detection relies on oxc-project/oxc#18053 which added `scope_id` to `Reference`, enabling us to locate the exact scope where the reference occurs. --- ### Case D: CJS wrapper parameters - Renaming needed For CommonJS wrapped modules, nested scopes must avoid shadowing the synthetic `exports` and `module` parameters injected by the CJS wrapper. **Example:** ```js // cjs-module.js (detected as CommonJS) function helper() { const exports = {}; // Would shadow CJS wrapper's exports parameter return exports; } module.exports = helper; ``` **Output:** ```js var require_cjs_module = __commonJS((exports, module) => { function helper() { const exports$1 = {}; // Renamed to avoid shadowing return exports$1; } module.exports = helper; }); ``` --- ## Implementation Details The renaming happens in two phases: 1. **`add_symbol_in_root_scope`** - Assigns canonical names to top-level symbols, checking: - Is the name already used by another top-level symbol? - For facade symbols (external module namespaces): would it conflict with nested scopes in the entry module? 2. **`NestedScopeRenamer`** - Renames nested bindings that would capture references: - **`rename_bindings_shadowing_star_imports`** (Case B): Star import member references (`ns.foo`) - **`rename_bindings_shadowing_named_imports`** (Case C): Named imports that were renamed - **`rename_bindings_shadowing_cjs_params`** (Case D): CJS wrapper parameter shadowing (`exports`, `module`) --- ## Test Cases | Test | Case | Description | |------|------|-------------| | `preserve_shadowing_param_name` | A | Parameter shadows import, keeps its name (intentional) | | `argument-treeshaking-parameter-conflict` | B | Namespace member ref, parameter renamed | | `basic_scoped` | C | Named import renamed, nested binding renamed | | `cjs` | D | CJS wrapper parameter shadowing | | Rollup: `preserves-catch-argument` | A | Catch parameter shadows import, keeps its name | | Rollup: `class-name-conflict-2` | A | Class in block scope, keeps its name | > Note for reviewer: Code comments and PR description are almost generated by Claude Code, but with careful self-review.

github-actions bot added A-semantic Area - Semantic C-enhancement Category - New feature or request labels Jan 16, 2026

Dunqing marked this pull request as ready for review January 19, 2026 01:58

Dunqing requested a review from overlookmotel as a code owner January 19, 2026 01:58

Copilot AI review requested due to automatic review settings January 19, 2026 01:58

Dunqing force-pushed the 01-16-feat_semantic_store_scope_id_in_reference branch from 3ce0bac to c8262de Compare January 19, 2026 01:58

Copilot started reviewing on behalf of Dunqing January 19, 2026 01:59 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Dunqing force-pushed the 01-16-feat_semantic_store_scope_id_in_reference branch from c8262de to a4f011a Compare January 19, 2026 02:16

Dunqing requested a review from Boshen January 19, 2026 02:46

Dunqing assigned Boshen and camc314 Jan 19, 2026

camc314 approved these changes Jan 19, 2026

View reviewed changes

graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026

graphite-app bot force-pushed the 01-16-feat_semantic_store_scope_id_in_reference branch from a4f011a to e2221e6 Compare January 19, 2026 04:54

graphite-app bot merged commit e2221e6 into main Jan 19, 2026
22 checks passed

graphite-app bot deleted the 01-16-feat_semantic_store_scope_id_in_reference branch January 19, 2026 05:00

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026

Boshen mentioned this pull request Jan 19, 2026

release(crates): oxc v0.109.0 #18222

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(semantic): store `scope_id` in `Reference`#18053

feat(semantic): store `scope_id` in `Reference`#18053
graphite-app[bot] merged 1 commit intomainfrom
01-16-feat_semantic_store_scope_id_in_reference

Dunqing commented Jan 16, 2026 •

edited

Loading

Uh oh!

Dunqing commented Jan 16, 2026 •

edited

Loading

Uh oh!

codspeed-hq bot commented Jan 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Dunqing commented Jan 19, 2026

Uh oh!

graphite-app bot commented Jan 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Dunqing commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dunqing commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Dunqing commented Jan 19, 2026

Uh oh!

graphite-app bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Dunqing commented Jan 16, 2026 •

edited

Loading

Dunqing commented Jan 16, 2026 •

edited

Loading

codspeed-hq bot commented Jan 16, 2026 •

edited

Loading

graphite-app bot commented Jan 19, 2026 •

edited

Loading