Skip to content
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.

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

Fixed nullish coalesce operator's precedence #61372

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 15 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39843,18 +39843,23 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function checkNullishCoalesceOperands(node: BinaryExpression) {
const { left, operatorToken, right } = node;
if (operatorToken.kind === SyntaxKind.QuestionQuestionToken) {
if (isBinaryExpression(left) && (left.operatorToken.kind === SyntaxKind.BarBarToken || left.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
grammarErrorOnNode(left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(left.operatorToken.kind), tokenToString(operatorToken.kind));
}
if (isBinaryExpression(right) && (right.operatorToken.kind === SyntaxKind.BarBarToken || right.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
grammarErrorOnNode(right, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(right.operatorToken.kind), tokenToString(operatorToken.kind));
if (node.operatorToken.kind !== SyntaxKind.QuestionQuestionToken) {
return;
}
if (isBinaryExpression(node.parent)) {
const { left, operatorToken } = node.parent;
if (isBinaryExpression(left) && (operatorToken.kind === SyntaxKind.BarBarToken || operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
grammarErrorOnNode(left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(SyntaxKind.QuestionQuestionToken), tokenToString(operatorToken.kind));
}

checkNullishCoalesceOperandLeft(node);
checkNullishCoalesceOperandRight(node);
}
else if (isBinaryExpression(node.left) && node.left.operatorToken.kind === SyntaxKind.BarBarToken) {
grammarErrorOnNode(node.left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(node.left.operatorToken.kind), tokenToString(SyntaxKind.QuestionQuestionToken));
}
else if (isBinaryExpression(node.right) && node.right.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
grammarErrorOnNode(node.right, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(SyntaxKind.QuestionQuestionToken), tokenToString(node.right.operatorToken.kind));
}
checkNullishCoalesceOperandLeft(node);
checkNullishCoalesceOperandRight(node);
}

function checkNullishCoalesceOperandLeft(node: BinaryExpression) {
Expand Down
12 changes: 6 additions & 6 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5658,17 +5658,17 @@ export const enum OperatorPrecedence {
// CoalesceExpression
Conditional,

// LogicalORExpression:
// LogicalANDExpression
// LogicalORExpression `||` LogicalANDExpression
LogicalOR,

// CoalesceExpression:
// CoalesceExpressionHead `??` BitwiseORExpression
// CoalesceExpressionHead:
// CoalesceExpression
// BitwiseORExpression
Coalesce = Conditional, // NOTE: This is wrong
Copy link
Member

Choose a reason for hiding this comment

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

I wonder how old this comment is...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

6 years, it was added in #35282


// LogicalORExpression:
// LogicalANDExpression
// LogicalORExpression `||` LogicalANDExpression
LogicalOR,
Coalesce = LogicalOR,

// LogicalANDExpression:
// BitwiseORExpression
Expand Down
57 changes: 57 additions & 0 deletions src/testRunner/unittests/printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,5 +366,62 @@ describe("unittests:: PrinterAPI", () => {
ts.factory.createNoSubstitutionTemplateLiteral("\n"),
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
));

printsCorrectly("binaryBarBarExpressionWithLeftConditionalExpression", {}, printer =>
printer.printNode(
ts.EmitHint.Unspecified,
ts.factory.createExpressionStatement(
ts.factory.createBinaryExpression(
ts.factory.createConditionalExpression(
ts.factory.createIdentifier("a"),
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
ts.factory.createIdentifier("b"),
ts.factory.createToken(ts.SyntaxKind.ColonToken),
ts.factory.createIdentifier("c"),
),
ts.factory.createToken(ts.SyntaxKind.BarBarToken),
ts.factory.createIdentifier("d"),
),
),
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
));

printsCorrectly("binaryAmpersandAmpersandExpressionWithLeftConditionalExpression", {}, printer =>
printer.printNode(
ts.EmitHint.Unspecified,
ts.factory.createExpressionStatement(
ts.factory.createBinaryExpression(
ts.factory.createConditionalExpression(
ts.factory.createIdentifier("a"),
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
ts.factory.createIdentifier("b"),
ts.factory.createToken(ts.SyntaxKind.ColonToken),
ts.factory.createIdentifier("c"),
),
ts.factory.createToken(ts.SyntaxKind.AmpersandAmpersandToken),
ts.factory.createIdentifier("d"),
),
),
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
));

printsCorrectly("binaryQuestionQuestionExpressionWithLeftConditionalExpression", {}, printer =>
printer.printNode(
ts.EmitHint.Unspecified,
ts.factory.createExpressionStatement(
ts.factory.createBinaryExpression(
ts.factory.createConditionalExpression(
ts.factory.createIdentifier("a"),
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
ts.factory.createIdentifier("b"),
ts.factory.createToken(ts.SyntaxKind.ColonToken),
ts.factory.createIdentifier("c"),
),
ts.factory.createToken(ts.SyntaxKind.QuestionQuestionToken),
ts.factory.createIdentifier("d"),
),
),
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
));
});
});
15 changes: 6 additions & 9 deletions tests/baselines/reference/nullishCoalescingOperator5.errors.txt
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
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(15,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 (4 errors) ====
==== nullishCoalescingOperator5.ts (3 errors) ====
declare const a: string | undefined
declare const b: string | undefined
declare const c: string | undefined

// 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,12 +21,10 @@ 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;
~~~~~~
!!! error TS5076: '&&' and '??' operations cannot be mixed without parentheses.

// Valid according to spec
a ?? (b || c);
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/nullishCoalescingOperator5.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ a && (b ?? c);
"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
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/nullishCoalescingOperator5.types
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ declare const c: string | undefined
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
6 changes: 3 additions & 3 deletions tests/baselines/reference/plainJSGrammarErrors.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,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 @@ -323,8 +323,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 tests/baselines/reference/plainJSGrammarErrors.types
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,10 @@ var x = 2 ?? 3 || 4
> : ^^^^^^
>2 ?? 3 || 4 : 2 | 3 | 4
> : ^^^^^^^^^
>2 ?? 3 : 2 | 3
> : ^^^^^
>2 : 2
> : ^
>3 || 4 : 3 | 4
> : ^^^^^
>3 : 3
> : ^
>4 : 4
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(a ? b : c) && d;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(a ? b : c) || d;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(a ? b : c) ?? d;