Skip to content

fix(transformer): clean up erased namespace bindings#24302

Open
camc314 wants to merge 1 commit into
mainfrom
c/clean-up-erased-ambient-decls
Open

fix(transformer): clean up erased namespace bindings#24302
camc314 wants to merge 1 commit into
mainfrom
c/clean-up-erased-ambient-decls

Conversation

@camc314

@camc314 camc314 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Keep semantic scoping in sync when the TypeScript namespace transform erases a namespace declaration.

Empty, type-only, and ambient namespaces can be removed from the output while their original binding and resolved references remain in Scoping. Rebuilding semantics from the transformed AST therefore produces different bindings and unresolved references.

This PR:

  • records namespace bindings erased during transformation;
  • preserves bindings that still have a non-ambient runtime declaration;
  • removes fully erased bindings after traversal;
  • converts references previously resolved to an erased namespace into root unresolved references.

Why finalize after traversal?

The binding cannot always be removed while visiting the namespace.

Later namespace declarations may merge with the same symbol and emit a runtime value. Removing the binding immediately would make the decision before all declarations have been transformed. It can also interfere with later TypeScript processing, such as export removal, which still relies on the original semantic information.

The namespace transform therefore records candidate bindings as declarations are erased, then finalizes scoping after traversal. Candidates are deduplicated by symbol, and only symbols without a surviving runtime declaration are removed.

This operates only on the recorded namespace symbols and their existing reference lists; it does not require another full-program AST traversal.

@github-actions github-actions Bot added A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler labels Jul 8, 2026
@camc314
camc314 marked this pull request as ready for review July 8, 2026 14:05
@camc314
camc314 requested a review from overlookmotel as a code owner July 8, 2026 14:05
Copilot AI review requested due to automatic review settings July 8, 2026 14:05
@camc314
camc314 requested a review from Dunqing as a code owner July 8, 2026 14:05
@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing c/clean-up-erased-ambient-decls (b76f3cf) with main (1ebdce3)

Open in CodSpeed

Footnotes

  1. 9 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a87e3bc13c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/oxc_transformer/src/typescript/namespace.rs

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

This PR keeps Scoping consistent with the transformed AST when the TypeScript namespace transform fully erases namespace declarations (empty/type-only/ambient), preventing bindings and references from diverging when semantics are rebuilt from the output program.

Changes:

  • Track namespace bindings that are erased during the namespace transform, deduplicate by SymbolId, and finalize scoping after traversal.
  • Remove fully-erased bindings from their scope and convert any previously-resolved references to those symbols into root unresolved references.
  • Update conformance/coverage snapshots to reflect the improved semantic consistency (fewer binding/reference mismatches).

Reviewed changes

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

Show a summary per file
File Description
crates/oxc_transformer/src/typescript/namespace.rs Records erased namespace bindings during traversal and applies scoping cleanup in a post-pass.
crates/oxc_transformer/src/typescript/mod.rs Adds a TypeScript finalize step to apply namespace scoping cleanup.
crates/oxc_transformer/src/lib.rs Calls the TypeScript finalize hook after traversal and before returning TransformerReturn.
crates/oxc_semantic/src/scoping.rs Adds unresolve_symbol_references to move resolved refs for a removed symbol into root unresolved refs.
crates/oxc_syntax/src/reference.rs Adds Reference::clear_symbol_id to support unresolving references.
tasks/transform_conformance/snapshots/oxc.snap.md Snapshot updates reflecting fixed scoping mismatches in transform conformance.
tasks/transform_conformance/snapshots/babel.snap.md Snapshot updates reflecting improved results in Babel transform conformance.
tasks/coverage/snapshots/semantic_typescript.snap Snapshot updates reflecting improved semantic consistency on TypeScript coverage cases.
tasks/coverage/snapshots/semantic_babel.snap Snapshot updates reflecting improved semantic consistency on Babel coverage cases.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b107df7b9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/oxc_semantic/src/scoping.rs Outdated
@camc314

camc314 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58b04f3f8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/oxc_transformer/src/typescript/namespace.rs
@camc314
camc314 force-pushed the c/clean-up-erased-ambient-decls branch from 58b04f3 to 72eb055 Compare July 8, 2026 15:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72eb055b5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/oxc_semantic/src/scoping.rs
@camc314
camc314 force-pushed the c/clean-up-erased-ambient-decls branch from 72eb055 to b76f3cf Compare July 9, 2026 11:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b76f3cf8cc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +139 to 140
self.record_removed_binding(&ident);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Record erased nested namespace bindings

For an erased namespace that contains only nested type-only namespaces, e.g. namespace Foo { namespace Bar { export type T = number } } (or the dotted form namespace Foo.Bar { ... }), this branch records only Foo and returns before visiting the nested TSModuleDeclaration that the semantic builder already bound as Bar in Foo's module scope. The transformed AST contains neither namespace, but the returned Scoping still retains the inner Bar binding in an erased scope, so consumers that inspect symbol bindings after transform can still observe TS-only namespace bindings; please collect removed bindings from nested module declarations before returning.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants