Fix parenthesizer rules for manually constructed binary expressions with ?? and ||/&& mix
#62311
Conversation
…ith `??` and `||`/`&&` mix
typescript-bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in TypeScript's printer parenthesizer rules for manually constructed binary expressions that mix the nullish coalescing operator (??) with logical operators (||/&&). The fix ensures proper parenthesization when these operators are combined to maintain correct evaluation order and avoid syntax errors.
Key changes:
- Added logic to detect mixing of
??with||/&&operators and force parenthesization - Added comprehensive unit tests to verify correct printing behavior for all operator combinations
- Updated test baselines to reflect the corrected parenthesization output
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/compiler/factory/parenthesizerRules.ts | Core fix adding mixingBinaryOperatorsRequiresParentheses function and early parenthesization check |
| src/testRunner/unittests/printer.ts | Comprehensive unit tests for all binary operator combinations with ?? |
| tests/baselines/reference/printerApi/*.js | Test baseline files showing expected parenthesized output for various operator combinations |
| @@ -121,6 +131,10 @@ export function createParenthesizerRules(factory: NodeFactory): ParenthesizerRul | |||
| * BinaryExpression. | |||
| */ | |||
| function binaryOperandNeedsParentheses(binaryOperator: SyntaxKind, operand: Expression, isLeftSideOfBinary: boolean, leftOperand: Expression | undefined) { | |||
| const emittedOperand = skipPartiallyEmittedExpressions(operand); | |||
| if (isBinaryExpression(emittedOperand) && mixingBinaryOperatorsRequiresParentheses(binaryOperator, emittedOperand.operatorToken.kind)) { | |||
There was a problem hiding this comment.
mixing them without parentheses is a special grammar-based syntax error, so one can't simply handle both of those cases using precedence and associativity alone, and for that reason I'm also specialcasing this here
There was a problem hiding this comment.
Seems right to me. @weswigham confirm / check Corsa behavior?
typescript-bot
added
For Backlog Bug
|
@typescript-bot cherry-pick to release-5.9 |
…ith `??` and `||`/`&&` mix (#62311)
|
Hey, @jakebailey! I've created #62424 for you. |
Andarist
deleted the
fixup/nullish-vs-barbar-or-ampersandampersand-print
branch
fixes #62307