Skip to content

Conversation

Andarist
Copy link
Contributor

fixes #62307

@Copilot Copilot AI review requested due to automatic review settings August 21, 2025 09:42
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Aug 21, 2025
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 21, 2025
Copy link

@Copilot Copilot AI left a comment

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 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

*/
function binaryOperandNeedsParentheses(binaryOperator: SyntaxKind, operand: Expression, isLeftSideOfBinary: boolean, leftOperand: Expression | undefined) {
const emittedOperand = skipPartiallyEmittedExpressions(operand);
if (isBinaryExpression(emittedOperand) && mixingBinaryOperatorsRequiresParentheses(binaryOperator, emittedOperand.operatorToken.kind)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

Seems right to me. @weswigham confirm / check Corsa behavior?

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Aug 21, 2025
@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 21, 2025
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Corsa late-adds required parenthesis during printing, rather than upon construction via parenthesizer like strada, so it'll be different to ensure the same in corsa.

@jakebailey jakebailey merged commit 4f94cb2 into microsoft:main Sep 9, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Sep 9, 2025
@jakebailey
Copy link
Member

@typescript-bot cherry-pick to release-5.9

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 9, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
cherry-pick to release-5.9 ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

Hey, @jakebailey! I've created #62424 for you.

@Andarist Andarist deleted the fixup/nullish-vs-barbar-or-ampersandampersand-print branch September 9, 2025 17:11
jakebailey pushed a commit that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Manually constructed nullish coalescing + OR AST is missing parentheses when printed
5 participants