fix(semantic): mark declared computed MethodDefinitions as type references#24296
Conversation
MethodDefinitions as type references
There was a problem hiding this comment.
Pull request overview
Fixes TypeScript semantic reference-flagging for computed class method keys so that ambient (declare class) and abstract computed methods are treated as type-capable references (allowing import type symbols), while concrete computed methods keep runtime read semantics. This aligns oxc_semantic’s symbol/reference classification with TypeScript’s ambient/abstract behavior and updates snapshots accordingly.
Changes:
- Add a bounded “containing declare class” check and use it when deciding reference flags for computed class elements.
- Customize
visit_method_definitionto mark computed keys in ambient/abstract methods asValueAsType, preservingReadfor concrete computed methods. - Add semantic fixture + snapshot coverage and update the TypeScript conformance snapshot output.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tasks/coverage/snapshots/semantic_typescript.snap | Updates TS conformance snapshot expectations for unresolved references after the semantic fix. |
| crates/oxc_semantic/tests/fixtures/oxc/ts/declare-computed-class-methods.ts | Adds a focused fixture covering computed method keys in ambient and abstract classes (and a concrete runtime method). |
| crates/oxc_semantic/tests/fixtures/oxc/ts/declare-computed-class-methods.snap | Adds the corresponding semantic snapshot asserting the intended reference flags (Type vs Read). |
| crates/oxc_semantic/src/builder.rs | Implements the new declare-class-element ancestry check and updates method/property computed-key reference flag handling. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deafd10dea
ℹ️ 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".
Merge activity
|
…erences (#24296) ## Summary - mark computed method keys in ambient and abstract classes as type references - preserve runtime read flags for concrete computed methods - bound declare-class ancestry checks to the containing class - add semantic snapshot coverage and update the TypeScript conformance snapshot Follows the work done in 588d997, 9b95632
deafd10 to
97690f1
Compare
…erences (#24296) ## Summary - mark computed method keys in ambient and abstract classes as type references - preserve runtime read flags for concrete computed methods - bound declare-class ancestry checks to the containing class - add semantic snapshot coverage and update the TypeScript conformance snapshot Follows the work done in 588d997, 9b95632
97690f1 to
ffd2765
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
### 🚀 Features - 616bfa2 minifier: Remove unreachable code after terminating statements (#24441) (Dunqing) - ddab89a data_structures: Add `likely` and `unlikely` functions (#24368) (overlookmotel) - a3a39f9 react_compiler: Implement enableEmitHookGuards codegen (#24329) (Boshen) - b79eef7 minifier: Apply De Morgan's law to negated comparison chains in jump guards and loop tests (#24279) (Dunqing) - 34ff7b4 minifier: Drop write-only property assignments to unused local bindings by default (#24112) (Dunqing) - 1b829d8 semantic: Record const enums in EnumData (#24268) (Dunqing) - ba0944c semantic: Add `Scoping::set_symbol_span` (#24221) (camc314) ### 🐛 Bug Fixes - 7d33363 minifier: Preserve guaranteed throws from class heritage evaluation (#24349) (Dunqing) - 058a62f semantic: Track ambient contexts in `SemanticBuilder` (#24327) (camc314) - 721eb0b transformer/decorator: Scope accessor class binding (#24330) (camc314) - 1ebdce3 semantic: Allow reserved keywords in ambient declaration types (#24325) (camc314) - 460176a track-memory-allocations: Exclude arena chunks from Sys allocs (#24292) (Dunqing) - af4922b transformer: Clear lowered namespace redeclarations (#24300) (camc314) - ffd2765 semantic: Mark declared computed `MethodDefinition`s as type references (#24296) (camc314) - f17514b isolated-declarations: Emit const readonly fields as types (#24288) (camc314) - 40f769d minifier: Make `__proto__` write tracking execution-order independent (#24280) (Dunqing) - 6371fed transformer: Remove stale enum member bindings (#24272) (camc314) - f05dfab transformer: Correct symbol flags for lowered namespaces (#24271) (Dunqing) - 84eeb55 transformer: Correct symbol flags for lowered enums (#24269) (Dunqing) - c3057da transformer: Preserve generated class binding spans (#24220) (camc314) - 8260096 transformer: Correct span for lowered namespace symbol (#24222) (camc314) - 42d00d3 semantic: Mark declared class heritage as type references (#24237) (camc314) - 588d997 semantic: Mark TS `PropertyDefinition`s computed fields as type references (#24233) (camc314) - 9b95632 semantic: Mark computed method keys in `TSMethodSignature`s as type references (#24232) (camc314) ### ⚡ Performance - 5b26643 transformer_plugins: Dispatch global defines by trailing name (#23666) (Boshen) - dce0f29 react_compiler: Replace all compiled functions in a single AST walk (#24403) (Boshen) - f85f0d8 ast: Delegate inherited enum variants in clone_in and estree derives (#23555) (Boshen) - 3ff0234 allocator: Remove `unwrap` from `ReplaceWith` (#24365) (overlookmotel) - ab22e80 transformer: Fix Rust 1.97 performance regression (#24354) (camc314) - b47585c parser: Use `ReplaceWith` instead of `TakeIn` (#24018) (overlookmotel) - b227a06 minifier: Use `ReplaceWith` instead of `TakeIn` (#24017) (overlookmotel) Co-authored-by: Boshen <[email protected]>
Summary
Follows the work done in 588d997, 9b95632