Skip to content

Commit 91cdbd5

Browse files
AndaristAndarist
authored
Fixed nullish coalesce operator's precedence (#61372)
1 parent 58665cf commit 91cdbd5

15 files changed

+238
-28
lines changed

‎src/compiler/checker.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40472,18 +40472,29 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4047240472
}
4047340473

4047440474
function checkNullishCoalesceOperands(node: BinaryExpression) {
40475-
const { left, operatorToken, right } = node;
40476-
if (operatorToken.kind === SyntaxKind.QuestionQuestionToken) {
40477-
if (isBinaryExpression(left) && (left.operatorToken.kind === SyntaxKind.BarBarToken || left.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
40478-
grammarErrorOnNode(left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(left.operatorToken.kind), tokenToString(operatorToken.kind));
40475+
if (node.operatorToken.kind !== SyntaxKind.QuestionQuestionToken) {
40476+
return;
40477+
}
40478+
if (isBinaryExpression(node.parent)) {
40479+
const { left, operatorToken } = node.parent;
40480+
if (isBinaryExpression(left) && operatorToken.kind === SyntaxKind.BarBarToken) {
40481+
grammarErrorOnNode(left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(SyntaxKind.QuestionQuestionToken), tokenToString(operatorToken.kind));
40482+
}
40483+
}
40484+
else if (isBinaryExpression(node.left)) {
40485+
const { operatorToken } = node.left;
40486+
if (operatorToken.kind === SyntaxKind.BarBarToken || operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
40487+
grammarErrorOnNode(node.left, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(operatorToken.kind), tokenToString(SyntaxKind.QuestionQuestionToken));
4047940488
}
40480-
if (isBinaryExpression(right) && (right.operatorToken.kind === SyntaxKind.BarBarToken || right.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
40481-
grammarErrorOnNode(right, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(right.operatorToken.kind), tokenToString(operatorToken.kind));
40489+
}
40490+
else if (isBinaryExpression(node.right)) {
40491+
const { operatorToken } = node.right;
40492+
if (operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
40493+
grammarErrorOnNode(node.right, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(SyntaxKind.QuestionQuestionToken), tokenToString(operatorToken.kind));
4048240494
}
40483-
40484-
checkNullishCoalesceOperandLeft(node);
40485-
checkNullishCoalesceOperandRight(node);
4048640495
}
40496+
checkNullishCoalesceOperandLeft(node);
40497+
checkNullishCoalesceOperandRight(node);
4048740498
}
4048840499

4048940500
function checkNullishCoalesceOperandLeft(node: BinaryExpression) {

‎src/compiler/utilities.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5675,17 +5675,17 @@ export const enum OperatorPrecedence {
56755675
// CoalesceExpression
56765676
Conditional,
56775677

5678+
// LogicalORExpression:
5679+
// LogicalANDExpression
5680+
// LogicalORExpression `||` LogicalANDExpression
5681+
LogicalOR,
5682+
56785683
// CoalesceExpression:
56795684
// CoalesceExpressionHead `??` BitwiseORExpression
56805685
// CoalesceExpressionHead:
56815686
// CoalesceExpression
56825687
// BitwiseORExpression
5683-
Coalesce = Conditional, // NOTE: This is wrong
5684-
5685-
// LogicalORExpression:
5686-
// LogicalANDExpression
5687-
// LogicalORExpression `||` LogicalANDExpression
5688-
LogicalOR,
5688+
Coalesce = LogicalOR,
56895689

56905690
// LogicalANDExpression:
56915691
// BitwiseORExpression

‎src/testRunner/unittests/printer.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,5 +366,62 @@ describe("unittests:: PrinterAPI", () => {
366366
ts.factory.createNoSubstitutionTemplateLiteral("\n"),
367367
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
368368
));
369+
370+
printsCorrectly("binaryBarBarExpressionWithLeftConditionalExpression", {}, printer =>
371+
printer.printNode(
372+
ts.EmitHint.Unspecified,
373+
ts.factory.createExpressionStatement(
374+
ts.factory.createBinaryExpression(
375+
ts.factory.createConditionalExpression(
376+
ts.factory.createIdentifier("a"),
377+
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
378+
ts.factory.createIdentifier("b"),
379+
ts.factory.createToken(ts.SyntaxKind.ColonToken),
380+
ts.factory.createIdentifier("c"),
381+
),
382+
ts.factory.createToken(ts.SyntaxKind.BarBarToken),
383+
ts.factory.createIdentifier("d"),
384+
),
385+
),
386+
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
387+
));
388+
389+
printsCorrectly("binaryAmpersandAmpersandExpressionWithLeftConditionalExpression", {}, printer =>
390+
printer.printNode(
391+
ts.EmitHint.Unspecified,
392+
ts.factory.createExpressionStatement(
393+
ts.factory.createBinaryExpression(
394+
ts.factory.createConditionalExpression(
395+
ts.factory.createIdentifier("a"),
396+
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
397+
ts.factory.createIdentifier("b"),
398+
ts.factory.createToken(ts.SyntaxKind.ColonToken),
399+
ts.factory.createIdentifier("c"),
400+
),
401+
ts.factory.createToken(ts.SyntaxKind.AmpersandAmpersandToken),
402+
ts.factory.createIdentifier("d"),
403+
),
404+
),
405+
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
406+
));
407+
408+
printsCorrectly("binaryQuestionQuestionExpressionWithLeftConditionalExpression", {}, printer =>
409+
printer.printNode(
410+
ts.EmitHint.Unspecified,
411+
ts.factory.createExpressionStatement(
412+
ts.factory.createBinaryExpression(
413+
ts.factory.createConditionalExpression(
414+
ts.factory.createIdentifier("a"),
415+
ts.factory.createToken(ts.SyntaxKind.QuestionToken),
416+
ts.factory.createIdentifier("b"),
417+
ts.factory.createToken(ts.SyntaxKind.ColonToken),
418+
ts.factory.createIdentifier("c"),
419+
),
420+
ts.factory.createToken(ts.SyntaxKind.QuestionQuestionToken),
421+
ts.factory.createIdentifier("d"),
422+
),
423+
),
424+
ts.createSourceFile("source.ts", "", ts.ScriptTarget.ESNext),
425+
));
369426
});
370427
});

‎tests/baselines/reference/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//// [tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts] ////
2+
3+
//// [nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts]
4+
// https://github.com/microsoft/TypeScript/issues/61109
5+
6+
class Cls {
7+
#privateProp: number | undefined;
8+
9+
problem() {
10+
this.#privateProp ??= false ? neverThis() : 20;
11+
}
12+
}
13+
14+
function neverThis(): never {
15+
throw new Error("This should really really never happen!");
16+
}
17+
18+
19+
//// [nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.js]
20+
"use strict";
21+
// https://github.com/microsoft/TypeScript/issues/61109
22+
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, state, kind, f) {
23+
if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter");
24+
if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
25+
return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
26+
};
27+
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, state, value, kind, f) {
28+
if (kind === "m") throw new TypeError("Private method is not writable");
29+
if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter");
30+
if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it");
31+
return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value;
32+
};
33+
var _Cls_privateProp;
34+
class Cls {
35+
constructor() {
36+
_Cls_privateProp.set(this, void 0);
37+
}
38+
problem() {
39+
__classPrivateFieldSet(this, _Cls_privateProp, __classPrivateFieldGet(this, _Cls_privateProp, "f") ?? (false ? neverThis() : 20), "f");
40+
}
41+
}
42+
_Cls_privateProp = new WeakMap();
43+
function neverThis() {
44+
throw new Error("This should really really never happen!");
45+
}

‎tests/baselines/reference/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.symbols

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//// [tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts] ////
2+
3+
=== nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts ===
4+
// https://github.com/microsoft/TypeScript/issues/61109
5+
6+
class Cls {
7+
>Cls : Symbol(Cls, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 0, 0))
8+
9+
#privateProp: number | undefined;
10+
>#privateProp : Symbol(Cls.#privateProp, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 2, 11))
11+
12+
problem() {
13+
>problem : Symbol(Cls.problem, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 3, 35))
14+
15+
this.#privateProp ??= false ? neverThis() : 20;
16+
>this.#privateProp : Symbol(Cls.#privateProp, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 2, 11))
17+
>this : Symbol(Cls, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 0, 0))
18+
>neverThis : Symbol(neverThis, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 8, 1))
19+
}
20+
}
21+
22+
function neverThis(): never {
23+
>neverThis : Symbol(neverThis, Decl(nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts, 8, 1))
24+
25+
throw new Error("This should really really never happen!");
26+
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
27+
}
28+

‎tests/baselines/reference/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.types

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//// [tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts] ////
2+
3+
=== nullishCoalescingAssignmentVsPrivateFieldsJsEmit1.ts ===
4+
// https://github.com/microsoft/TypeScript/issues/61109
5+
6+
class Cls {
7+
>Cls : Cls
8+
> : ^^^
9+
10+
#privateProp: number | undefined;
11+
>#privateProp : number | undefined
12+
> : ^^^^^^^^^^^^^^^^^^
13+
14+
problem() {
15+
>problem : () => void
16+
> : ^^^^^^^^^^
17+
18+
this.#privateProp ??= false ? neverThis() : 20;
19+
>this.#privateProp ??= false ? neverThis() : 20 : number
20+
> : ^^^^^^
21+
>this.#privateProp : number | undefined
22+
> : ^^^^^^^^^^^^^^^^^^
23+
>this : this
24+
> : ^^^^
25+
>false ? neverThis() : 20 : 20
26+
> : ^^
27+
>false : false
28+
> : ^^^^^
29+
>neverThis() : never
30+
> : ^^^^^
31+
>neverThis : () => never
32+
> : ^^^^^^
33+
>20 : 20
34+
> : ^^
35+
}
36+
}
37+
38+
function neverThis(): never {
39+
>neverThis : () => never
40+
> : ^^^^^^
41+
42+
throw new Error("This should really really never happen!");
43+
>new Error("This should really really never happen!") : Error
44+
> : ^^^^^
45+
>Error : ErrorConstructor
46+
> : ^^^^^^^^^^^^^^^^
47+
>"This should really really never happen!" : "This should really really never happen!"
48+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
49+
}
50+

‎tests/baselines/reference/nullishCoalescingOperator5.errors.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
nullishCoalescingOperator5.ts(6,6): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
1+
nullishCoalescingOperator5.ts(6,1): error TS5076: '??' and '||' operations cannot be mixed without parentheses.
22
nullishCoalescingOperator5.ts(9,1): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
3-
nullishCoalescingOperator5.ts(12,6): error TS5076: '&&' and '??' operations cannot be mixed without parentheses.
3+
nullishCoalescingOperator5.ts(12,6): error TS5076: '??' and '&&' operations cannot be mixed without parentheses.
44
nullishCoalescingOperator5.ts(15,1): error TS5076: '&&' and '??' operations cannot be mixed without parentheses.
55

66

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

1212
// should be a syntax error
1313
a ?? b || c;
14-
~~~~~~
15-
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
14+
~~~~~~
15+
!!! error TS5076: '??' and '||' operations cannot be mixed without parentheses.
1616

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

2727
// should be a syntax error
2828
a && b ?? c;

‎tests/baselines/reference/nullishCoalescingOperator5.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ a && (b ?? c);
4646
"use strict";
4747
var _a, _b, _c, _d;
4848
// should be a syntax error
49-
a !== null && a !== void 0 ? a : b || c;
49+
(a !== null && a !== void 0 ? a : b) || c;
5050
// should be a syntax error
5151
(_a = a || b) !== null && _a !== void 0 ? _a : c;
5252
// should be a syntax error

‎tests/baselines/reference/nullishCoalescingOperator5.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ declare const c: string | undefined
1717
a ?? b || c;
1818
>a ?? b || c : string | undefined
1919
> : ^^^^^^^^^^^^^^^^^^
20+
>a ?? b : string | undefined
21+
> : ^^^^^^^^^^^^^^^^^^
2022
>a : string | undefined
2123
> : ^^^^^^^^^^^^^^^^^^
22-
>b || c : string | undefined
23-
> : ^^^^^^^^^^^^^^^^^^
2424
>b : string | undefined
2525
> : ^^^^^^^^^^^^^^^^^^
2626
>c : string | undefined

‎tests/baselines/reference/plainJSGrammarErrors.errors.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ plainJSGrammarErrors.js(92,33): error TS2566: A rest element cannot have a prope
5959
plainJSGrammarErrors.js(93,42): error TS1186: A rest element cannot have an initializer.
6060
plainJSGrammarErrors.js(96,4): error TS1123: Variable declaration list cannot be empty.
6161
plainJSGrammarErrors.js(97,9): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
62-
plainJSGrammarErrors.js(98,14): error TS5076: '||' and '??' operations cannot be mixed without parentheses.
62+
plainJSGrammarErrors.js(98,9): error TS5076: '??' and '||' operations cannot be mixed without parentheses.
6363
plainJSGrammarErrors.js(100,3): error TS1200: Line terminator not permitted before arrow.
6464
plainJSGrammarErrors.js(102,4): error TS1358: Tagged template expressions are not permitted in an optional chain.
6565
plainJSGrammarErrors.js(104,6): error TS1171: A comma expression is not allowed in a computed property name.
@@ -323,8 +323,8 @@ plainJSGrammarErrors.js(205,36): error TS1325: Argument of dynamic import cannot
323323
~~~~~~
324324
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
325325
var x = 2 ?? 3 || 4
326-
~~~~~~
327-
!!! error TS5076: '||' and '??' operations cannot be mixed without parentheses.
326+
~~~~~~
327+
!!! error TS5076: '??' and '||' operations cannot be mixed without parentheses.
328328
const arr = x
329329
=> x + 1
330330
~~

0 commit comments

Comments
 (0)