fix(codegen): preserve parens for TSNonNullExpression#20125
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the TypeScript codegen where parentheses inside a TSNonNullExpression (the ! postfix operator) were dropped when the parser option preserve_parens: false was used. For example, (a ?? b)! was being incorrectly emitted as a ?? b!, which has a different meaning.
Changes:
- The codegen for
TSNonNullExpressionnow passesPrecedence::Postfixto the inner expression'sprint_expr, ensuring any expression with lower precedence thanPostfix(such as??,||,&&, conditionals, assignments, etc.) gets properly wrapped in parentheses. - A regression test is added to validate the
(a ?? b)!case withpreserve_parens: false.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/oxc_codegen/src/gen.rs |
Fixes the TSNonNullExpression codegen to use Precedence::Postfix for its inner expression, replacing the old approach that only wrapped ParenthesizedExpression nodes |
crates/oxc_codegen/tests/integration/ts.rs |
Adds a test case for (a ?? b)! with preserve_parens: false to prevent regressions |
You can also share your feedback on Copilot code review. Take the survey.
Merging this PR will not alter performance
Comparing Footnotes
|
9081dc4 to
8d813c8
Compare
Merge activity
|
b3eae30 to
ae830b2
Compare
8d813c8 to
a40870e
Compare
…Gen impls We've had multiple bugs where new fields were added to AST structs but the codegen `Gen`/`GenExpr` implementations forgot to handle them (e.g., #20124 `declare` for TSInterfaceDeclaration, #20125 parens for TSNonNullExpression, #20127 type_arguments for JSXOpeningElement, #20128 type for TSImportEqualsDeclaration). These bugs are silent — the code compiles fine but produces incorrect output. This test uses `syn` to parse AST struct definitions and cross-reference them against the codegen `Gen`/`GenExpr` implementations, detecting any struct fields that are never accessed. It catches: - Direct `self.field` accesses in Gen/GenExpr impl blocks - Field accesses via helper functions (e.g., `print_if(stmt: &IfStatement)`) - Field accesses through enum wrapper match arms (e.g., `Binaryish::Binary(e) => e.left`) Fields that are intentionally not printed (internal metadata like `node_id`, `scope_id`, or fields accessed via methods like `as_str()`) are tracked in explicit skip lists with comments explaining why. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
### 🚀 Features - e8547cc parser: Report error for using declarations in ambient contexts (#19934) (camc314) - 8345318 allocator: Add methods for boxed slices `ArenaBox<[T]>` (#19968) (overlookmotel) - f83be30 allocator: Add `Vec::push_fast` method (#19959) (overlookmotel) ### 🐛 Bug Fixes - 291d867 transformer_plugins: Unwrap ChainExpression after define replacement removes optional markers (#20058) (IWANABETHATGUY) - 36b2e56 codegen: Print type for TSImportEqualsDeclaration (#20128) (camc314) - 5a246ec codegen: Print type arguments for JSXOpeningElement (#20127) (camc314) - a40870e codegen: Preserve parens for TSNonNullExpression (#20125) (camc314) - ae830b2 codegen: Print `declare` for `TSInterfaceDeclaration` (#20124) (camc314) - 92cfb14 linter/plugins: Fix types for `walkProgram` and `walkProgramWithCfg` (#20081) (overlookmotel) - ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071) (camc314) - 588009e codegen: Print `static` keyword for TSIndexSignature (#19755) (Dunqing) - 5a8799c codegen: Print `with_clause` for `ExportNamedDeclaration` (#20002) (Dunqing) - 7502afe parser: Correct capacity for tokens `Vec` (#19967) (overlookmotel) ### ⚡ Performance - 4ea8f9a napi: Remove `napi_build::setup()` from `oxc_napi` to avoid redundant rebuilds (#20094) (Boshen) - 2baa5fb napi: Unify build-test profile to coverage for cache sharing (#20090) (Boshen) - 8ba61dd parser: Make pushing tokens faster (#19960) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>

No description provided.