Skip to content

Commit 6b6926f

Browse files
Copilotjakebailey
andauthored
Port #62311: Fix parenthesizer rules for binary expressions mixing ?? with ||/&& (#2758)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
1 parent a5c64bd commit 6b6926f

File tree

3 files changed

+84
-0
lines changed

3 files changed

+84
-0
lines changed

internal/printer/printer.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2773,6 +2773,12 @@ func (p *Printer) getBinaryExpressionPrecedence(node *ast.BinaryExpression) (lef
27732773

27742774
func (p *Printer) emitBinaryExpression(node *ast.BinaryExpression) {
27752775
leftPrec, rightPrec := p.getBinaryExpressionPrecedence(node)
2776+
if emittedLeft := ast.SkipPartiallyEmittedExpressions(node.Left); ast.NodeIsSynthesized(emittedLeft) && emittedLeft.Kind == ast.KindBinaryExpression && mixingBinaryOperatorsRequiresParentheses(node.OperatorToken.Kind, emittedLeft.AsBinaryExpression().OperatorToken.Kind) {
2777+
leftPrec = ast.OperatorPrecedenceHighest
2778+
}
2779+
if emittedRight := ast.SkipPartiallyEmittedExpressions(node.Right); ast.NodeIsSynthesized(emittedRight) && emittedRight.Kind == ast.KindBinaryExpression && mixingBinaryOperatorsRequiresParentheses(node.OperatorToken.Kind, emittedRight.AsBinaryExpression().OperatorToken.Kind) {
2780+
rightPrec = ast.OperatorPrecedenceHighest
2781+
}
27762782
state := p.enterNode(node.AsNode())
27772783
p.emitExpression(node.Left, leftPrec)
27782784
linesBeforeOperator := p.getLinesBetweenNodes(node.AsNode(), node.Left, node.OperatorToken)

internal/printer/printer_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,3 +2517,71 @@ func TestPartiallyEmittedExpression(t *testing.T) {
25172517
.expression
25182518
.expression;`)
25192519
}
2520+
2521+
func TestParenthesizeBinaryExpressionMixingNullishCoalescing(t *testing.T) {
2522+
t.Parallel()
2523+
2524+
tests := []struct {
2525+
title string
2526+
innerOp ast.Kind
2527+
outerOp ast.Kind
2528+
side string
2529+
output string
2530+
}{
2531+
// inner ?? on left side of || or &&
2532+
{title: "BarBarWithLeftQuestionQuestion", innerOp: ast.KindQuestionQuestionToken, outerOp: ast.KindBarBarToken, side: "left", output: "(a ?? b) || c;"},
2533+
{title: "AmpersandAmpersandWithLeftQuestionQuestion", innerOp: ast.KindQuestionQuestionToken, outerOp: ast.KindAmpersandAmpersandToken, side: "left", output: "(a ?? b) && c;"},
2534+
// inner ?? on right side of || or &&
2535+
{title: "BarBarWithRightQuestionQuestion", innerOp: ast.KindQuestionQuestionToken, outerOp: ast.KindBarBarToken, side: "right", output: "a || (b ?? c);"},
2536+
{title: "AmpersandAmpersandWithRightQuestionQuestion", innerOp: ast.KindQuestionQuestionToken, outerOp: ast.KindAmpersandAmpersandToken, side: "right", output: "a && (b ?? c);"},
2537+
// inner || or && on left side of ??
2538+
{title: "QuestionQuestionWithLeftBarBar", innerOp: ast.KindBarBarToken, outerOp: ast.KindQuestionQuestionToken, side: "left", output: "(a || b) ?? c;"},
2539+
{title: "QuestionQuestionWithLeftAmpersandAmpersand", innerOp: ast.KindAmpersandAmpersandToken, outerOp: ast.KindQuestionQuestionToken, side: "left", output: "(a && b) ?? c;"},
2540+
// inner || or && on right side of ??
2541+
{title: "QuestionQuestionWithRightBarBar", innerOp: ast.KindBarBarToken, outerOp: ast.KindQuestionQuestionToken, side: "right", output: "a ?? (b || c);"},
2542+
{title: "QuestionQuestionWithRightAmpersandAmpersand", innerOp: ast.KindAmpersandAmpersandToken, outerOp: ast.KindQuestionQuestionToken, side: "right", output: "a ?? (b && c);"},
2543+
}
2544+
2545+
for _, tt := range tests {
2546+
t.Run(tt.title, func(t *testing.T) {
2547+
t.Parallel()
2548+
var factory ast.NodeFactory
2549+
innerExpr := factory.NewBinaryExpression(
2550+
nil, /*modifiers*/
2551+
factory.NewIdentifier("a"),
2552+
nil, /*typeNode*/
2553+
factory.NewToken(tt.innerOp),
2554+
factory.NewIdentifier("b"),
2555+
)
2556+
var outerExpr *ast.Node
2557+
if tt.side == "left" {
2558+
outerExpr = factory.NewBinaryExpression(
2559+
nil, /*modifiers*/
2560+
innerExpr, /*left: (a innerOp b)*/
2561+
nil, /*typeNode*/
2562+
factory.NewToken(tt.outerOp),
2563+
factory.NewIdentifier("c"),
2564+
)
2565+
} else {
2566+
outerExpr = factory.NewBinaryExpression(
2567+
nil, /*modifiers*/
2568+
factory.NewIdentifier("a"),
2569+
nil, /*typeNode*/
2570+
factory.NewToken(tt.outerOp),
2571+
innerExpr, /*right: (b innerOp c)*/
2572+
)
2573+
// adjust identifiers for right side
2574+
innerExpr.AsBinaryExpression().Left = factory.NewIdentifier("b")
2575+
innerExpr.AsBinaryExpression().Right = factory.NewIdentifier("c")
2576+
}
2577+
file := factory.NewSourceFile(ast.SourceFileParseOptions{FileName: "/file.ts", Path: "/file.ts"}, "", factory.NewNodeList(
2578+
[]*ast.Node{
2579+
factory.NewExpressionStatement(outerExpr),
2580+
},
2581+
), factory.NewToken(ast.KindEndOfFile))
2582+
2583+
parsetestutil.MarkSyntheticRecursive(file)
2584+
emittestutil.CheckEmit(t, nil, file.AsSourceFile(), tt.output)
2585+
})
2586+
}
2587+
}

internal/printer/utilities.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,16 @@ func isBinaryOperation(node *ast.Node, token ast.Kind) bool {
625625
node.AsBinaryExpression().OperatorToken.Kind == token
626626
}
627627

628+
func mixingBinaryOperatorsRequiresParentheses(a ast.Kind, b ast.Kind) bool {
629+
if a == ast.KindQuestionQuestionToken {
630+
return b == ast.KindAmpersandAmpersandToken || b == ast.KindBarBarToken
631+
}
632+
if b == ast.KindQuestionQuestionToken {
633+
return a == ast.KindAmpersandAmpersandToken || a == ast.KindBarBarToken
634+
}
635+
return false
636+
}
637+
628638
func isImmediatelyInvokedFunctionExpressionOrArrowFunction(node *ast.Expression) bool {
629639
node = ast.SkipPartiallyEmittedExpressions(node)
630640
if !ast.IsCallExpression(node) {

0 commit comments

Comments
 (0)