Skip to content

Port "Fixed nullish coalesce operator's precedence" #1824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 6, 2025
Merged

Port "Fixed nullish coalesce operator's precedence" #1824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42d0279
Port "Fixed nullish coalesce operator's precedence"
Andarist Oct 5, 2025
507b13d
fix things
Andarist Oct 5, 2025
03a7baf
fmt
Andarist Oct 5, 2025
f6534fe
lint
Andarist Oct 5, 2025
4f67b06
Merge branch 'main' into port/61372
Andarist Oct 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions internal/ast/precedence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,6 @@ const (
// ShortCircuitExpression
// ShortCircuitExpression `?` AssignmentExpression `:` AssignmentExpression
OperatorPrecedenceConditional
// ShortCircuitExpression:
// LogicalORExpression
// CoalesceExpression
// CoalesceExpression:
// CoalesceExpressionHead `??` BitwiseORExpression
// CoalesceExpressionHead:
// CoalesceExpression
// BitwiseORExpression
OperatorPrecedenceCoalesce
// LogicalORExpression:
// LogicalANDExpression
// LogicalORExpression `||` LogicalANDExpression
Expand Down Expand Up @@ -181,6 +172,15 @@ const (
OperatorPrecedenceLowest = OperatorPrecedenceComma
OperatorPrecedenceHighest = OperatorPrecedenceParentheses
OperatorPrecedenceDisallowComma = OperatorPrecedenceYield
// ShortCircuitExpression:
// LogicalORExpression
// CoalesceExpression
// CoalesceExpression:
// CoalesceExpressionHead `??` BitwiseORExpression
// CoalesceExpressionHead:
// CoalesceExpression
// BitwiseORExpression
OperatorPrecedenceCoalesce = OperatorPrecedenceLogicalOR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, I could make the OperatorPrecedenceCoalesce lower than OperatorPrecedenceLogicalOR and then handle that change from Strada in parseBinaryExpressionRest and its computation of consumeCurrentOperator (and maybe in some other places too). cc @weswigham

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't that match Strada better? Or is there a downside?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strada defines those 2 with the same value so the current approach in this PR matches Strada rather closely. The alternative I mentioned would require changes that would diverge from Strada (code-wise)

Copy link
Member

Choose a reason for hiding this comment

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

Agh, my bad, read the other PR wrong and forgot to expand some code.

// -1 is lower than all other precedences. Returning it will cause binary expression
// parsing to stop.
OperatorPrecedenceInvalid OperatorPrecedence = -1
Expand Down
21 changes: 16 additions & 5 deletions internal/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12400,11 +12400,22 @@ func (c *Checker) getSyntacticTruthySemantics(node *ast.Node) PredicateSemantics
}

func (c *Checker) checkNullishCoalesceOperands(left *ast.Node, right *ast.Node) {
if ast.IsBinaryExpression(left) && ast.IsLogicalBinaryOperator(left.AsBinaryExpression().OperatorToken.Kind) {
c.grammarErrorOnNode(left, diagnostics.X_0_and_1_operations_cannot_be_mixed_without_parentheses, scanner.TokenToString(left.AsBinaryExpression().OperatorToken.Kind), scanner.TokenToString(ast.KindQuestionQuestionToken))
}
if ast.IsBinaryExpression(right) && ast.IsLogicalBinaryOperator(right.AsBinaryExpression().OperatorToken.Kind) {
c.grammarErrorOnNode(right, diagnostics.X_0_and_1_operations_cannot_be_mixed_without_parentheses, scanner.TokenToString(right.AsBinaryExpression().OperatorToken.Kind), scanner.TokenToString(ast.KindQuestionQuestionToken))
if ast.IsBinaryExpression(left.Parent.Parent) {
grandparentLeft := left.Parent.Parent.AsBinaryExpression().Left
grandparentOperatorToken := left.Parent.Parent.AsBinaryExpression().OperatorToken
if ast.IsBinaryExpression(grandparentLeft) && grandparentOperatorToken.Kind == ast.KindBarBarToken {
c.grammarErrorOnNode(grandparentLeft, diagnostics.X_0_and_1_operations_cannot_be_mixed_without_parentheses, scanner.TokenToString(ast.KindQuestionQuestionToken), scanner.TokenToString(grandparentOperatorToken.Kind))
}
} else if ast.IsBinaryExpression(left) {
operatorToken := left.AsBinaryExpression().OperatorToken
if operatorToken.Kind == ast.KindBarBarToken || operatorToken.Kind == ast.KindAmpersandAmpersandToken {
c.grammarErrorOnNode(left, diagnostics.X_0_and_1_operations_cannot_be_mixed_without_parentheses, scanner.TokenToString(operatorToken.Kind), scanner.TokenToString(ast.KindQuestionQuestionToken))
}
} else if ast.IsBinaryExpression(right) {
operatorToken := right.AsBinaryExpression().OperatorToken
if operatorToken.Kind == ast.KindAmpersandAmpersandToken {
c.grammarErrorOnNode(right, diagnostics.X_0_and_1_operations_cannot_be_mixed_without_parentheses, scanner.TokenToString(ast.KindQuestionQuestionToken), scanner.TokenToString(operatorToken.Kind))
}
}
c.checkNullishCoalesceOperandLeft(left)
c.checkNullishCoalesceOperandRight(right)
Expand Down
8 changes: 0 additions & 8 deletions internal/printer/printer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2716,14 +2716,6 @@ func (p *Printer) getBinaryExpressionPrecedence(node *ast.BinaryExpression) (lef
case ast.OperatorPrecedenceAssignment:
// assignment is right-associative
leftPrec = ast.OperatorPrecedenceLeftHandSide
case ast.OperatorPrecedenceCoalesce:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to remove it because it would now be a duplicate case - but it seems that with the current logic I don't even have to move this code anywhere as nothing has regressed

// allow coalesce on the left, but short circuit to BitwiseOR
if isBinaryOperation(node.Left, ast.KindQuestionQuestionToken) {
leftPrec = ast.OperatorPrecedenceCoalesce
} else {
leftPrec = ast.OperatorPrecedenceBitwiseOR
}
rightPrec = ast.OperatorPrecedenceBitwiseOR
case ast.OperatorPrecedenceLogicalOR:
rightPrec = ast.OperatorPrecedenceLogicalAND
case ast.OperatorPrecedenceLogicalAND:
Expand Down
10 changes: 5 additions & 5 deletions testdata/baselines/reference/submodule/conformance/nullishCoalescingOperator5.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
nullishCoalescingOperator5.ts(6,6): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
nullishCoalescingOperator5.ts(6,1): error TS5076: '??' and '||' operations cannot be mixed without parentheses.
nullishCoalescingOperator5.ts(9,1): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
nullishCoalescingOperator5.ts(12,6): error TS5076: '&&' and '??' operations cannot be mixed without parentheses.
nullishCoalescingOperator5.ts(12,6): error TS5076: '??' and '&&' operations cannot be mixed without parentheses.
nullishCoalescingOperator5.ts(15,1): error TS5076: '&&' and '??' operations cannot be mixed without parentheses.


Expand All @@ -11,8 +11,8 @@ nullishCoalescingOperator5.ts(15,1): error TS5076: '&&' and '??' operations cann

// should be a syntax error
a ?? b || c;
~~~~~~
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
~~~~~~
!!! error TS5076: '??' and '||' operations cannot be mixed without parentheses.

// should be a syntax error
a || b ?? c;
Expand All @@ -22,7 +22,7 @@ nullishCoalescingOperator5.ts(15,1): error TS5076: '&&' and '??' operations cann
// should be a syntax error
a ?? b && c;
~~~~~~
!!! error TS5076: '&&' and '??' operations cannot be mixed without parentheses.
!!! error TS5076: '??' and '&&' operations cannot be mixed without parentheses.

// should be a syntax error
a && b ?? c;
Expand Down
31 changes: 0 additions & 31 deletions ...data/baselines/reference/submodule/conformance/nullishCoalescingOperator5.errors.txt.diff
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/conformance/nullishCoalescingOperator5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ a && (b ?? c);
//// [nullishCoalescingOperator5.js]
var _a, _b, _c, _d;
// should be a syntax error
a !== null && a !== void 0 ? a : b || c;
(a !== null && a !== void 0 ? a : b) || c;
// should be a syntax error
(_a = a || b) !== null && _a !== void 0 ? _a : c;
// should be a syntax error
Expand Down
6 changes: 1 addition & 5 deletions testdata/baselines/reference/submodule/conformance/nullishCoalescingOperator5.js.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,4 @@
-"use strict";
var _a, _b, _c, _d;
// should be a syntax error
-(a !== null && a !== void 0 ? a : b) || c;
+a !== null && a !== void 0 ? a : b || c;
// should be a syntax error
(_a = a || b) !== null && _a !== void 0 ? _a : c;
// should be a syntax error
(a !== null && a !== void 0 ? a : b) || c;
2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/conformance/nullishCoalescingOperator5.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ declare const c: string | undefined
// should be a syntax error
a ?? b || c;
>a ?? b || c : string | undefined
>a ?? b : string | undefined
>a : string | undefined
>b || c : string | undefined
>b : string | undefined
>c : string | undefined

Expand Down
11 changes: 0 additions & 11 deletions testdata/baselines/reference/submodule/conformance/nullishCoalescingOperator5.types.diff
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions testdata/baselines/reference/submodule/conformance/plainJSGrammarErrors.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ plainJSGrammarErrors.js(92,33): error TS2566: A rest element cannot have a prope
plainJSGrammarErrors.js(93,42): error TS1186: A rest element cannot have an initializer.
plainJSGrammarErrors.js(96,4): error TS1123: Variable declaration list cannot be empty.
plainJSGrammarErrors.js(97,9): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
plainJSGrammarErrors.js(98,14): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
plainJSGrammarErrors.js(98,9): error TS5076: '??' and '||' operations cannot be mixed without parentheses.
plainJSGrammarErrors.js(100,3): error TS1200: Line terminator not permitted before arrow.
plainJSGrammarErrors.js(102,4): error TS1358: Tagged template expressions are not permitted in an optional chain.
plainJSGrammarErrors.js(104,6): error TS1171: A comma expression is not allowed in a computed property name.
Expand Down Expand Up @@ -320,8 +320,8 @@ plainJSGrammarErrors.js(205,36): error TS1325: Argument of dynamic import cannot
~~~~~~
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
var x = 2 ?? 3 || 4
~~~~~~
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
~~~~~~
!!! error TS5076: '??' and '||' operations cannot be mixed without parentheses.
const arr = x
=> x + 1
~~
Expand Down
4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/conformance/plainJSGrammarErrors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ const { e: eep, m: em, ...rest: noRestAllowed } = doom;
const { e: erp, m: erm, ...noInitialiser = true } = doom;
// left-over parsing
var ;
var x = (1 || 2) ?? 3;
var x = 2 ?? (3 || 4);
var x = 1 || 2 ?? 3;
var x = 2 ?? 3 || 4;
const arr = x => x + 1;
var a = [1, 2];
a `length`;
Expand Down
10 changes: 1 addition & 9 deletions testdata/baselines/reference/submodule/conformance/plainJSGrammarErrors.js.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,7 @@
export import 'fs';
export { C };
function nestedExports() {
@@= skipped -30, +30 lines =@@
const { e: erp, m: erm, ...noInitialiser = true } = doom;
// left-over parsing
var ;
-var x = 1 || 2 ?? 3;
-var x = 2 ?? 3 || 4;
+var x = (1 || 2) ?? 3;
+var x = 2 ?? (3 || 4);
const arr = x => x + 1;
@@= skipped -36, +36 lines =@@
var a = [1, 2];
a `length`;
const o = {
Expand Down
2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/conformance/plainJSGrammarErrors.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ var x = 1 || 2 ?? 3
var x = 2 ?? 3 || 4
>x : number
>2 ?? 3 || 4 : 2 | 3 | 4
>2 ?? 3 : 2 | 3
>2 : 2
>3 || 4 : 3 | 4
>3 : 3
>4 : 4

Expand Down
24 changes: 2 additions & 22 deletions ...ta/baselines/reference/submoduleAccepted/conformance/plainJSGrammarErrors.errors.txt.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,7 @@
plainJSGrammarErrors.js(64,1): error TS1042: 'async' modifier cannot be used here.
plainJSGrammarErrors.js(65,1): error TS1042: 'async' modifier cannot be used here.
plainJSGrammarErrors.js(66,1): error TS1042: 'async' modifier cannot be used here.
@@= skipped -21, +20 lines =@@
plainJSGrammarErrors.js(93,42): error TS1186: A rest element cannot have an initializer.
plainJSGrammarErrors.js(96,4): error TS1123: Variable declaration list cannot be empty.
plainJSGrammarErrors.js(97,9): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
-plainJSGrammarErrors.js(98,9): error TS5076: '??' and '||' operations cannot be mixed without parentheses.
+plainJSGrammarErrors.js(98,14): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
plainJSGrammarErrors.js(100,3): error TS1200: Line terminator not permitted before arrow.
plainJSGrammarErrors.js(102,4): error TS1358: Tagged template expressions are not permitted in an optional chain.
plainJSGrammarErrors.js(104,6): error TS1171: A comma expression is not allowed in a computed property name.
@@= skipped -43, +43 lines =@@
@@= skipped -64, +63 lines =@@
plainJSGrammarErrors.js(205,36): error TS1325: Argument of dynamic import cannot be spread element.


Expand Down Expand Up @@ -62,15 +53,4 @@
-!!! error TS8009: The 'async' modifier can only be used in TypeScript files.
}
async const cantAsyncConst = 2
~~~~~
@@= skipped -78, +76 lines =@@
~~~~~~
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
var x = 2 ?? 3 || 4
- ~~~~~~
-!!! error TS5076: '??' and '||' operations cannot be mixed without parentheses.
+ ~~~~~~
+!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
const arr = x
=> x + 1
~~
~~~~~
12 changes: 1 addition & 11 deletions testdata/baselines/reference/submoduleAccepted/conformance/plainJSGrammarErrors.types.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,7 @@

export { staticParam }
>staticParam : any
@@= skipped -97, +97 lines =@@
var x = 2 ?? 3 || 4
>x : number
>2 ?? 3 || 4 : 2 | 3 | 4
->2 ?? 3 : 2 | 3
>2 : 2
+>3 || 4 : 3 | 4
>3 : 3
>4 : 4

@@= skipped -194, +194 lines =@@
@@= skipped -291, +291 lines =@@
>b : any

switch (b) {
Expand Down