Skip to content

Commit c574e40

Browse files
CopilotjakebaileyRyanCavanaugh
authored
Fix "never nullish" diagnostic missing expressions wrapped in parentheses (#62789)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jakebailey <[email protected]> Co-authored-by: Ryan Cavanaugh <[email protected]> Co-authored-by: Ryan Cavanaugh <[email protected]>
1 parent 632479f commit c574e40

File tree

7 files changed

+320
-52
lines changed

7 files changed

+320
-52
lines changed

‎src/compiler/checker.ts‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40548,12 +40548,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4054840548
}
4054940549
}
4055040550
checkNullishCoalesceOperandLeft(node);
40551-
checkNullishCoalesceOperandRight(node);
4055240551
}
4055340552

4055440553
function checkNullishCoalesceOperandLeft(node: BinaryExpression) {
4055540554
const leftTarget = skipOuterExpressions(node.left, OuterExpressionKinds.All);
40556-
4055740555
const nullishSemantics = getSyntacticNullishnessSemantics(leftTarget);
4055840556
if (nullishSemantics !== PredicateSemantics.Sometimes) {
4055940557
if (nullishSemantics === PredicateSemantics.Always) {
@@ -40565,25 +40563,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4056540563
}
4056640564
}
4056740565

40568-
function checkNullishCoalesceOperandRight(node: BinaryExpression) {
40569-
const rightTarget = skipOuterExpressions(node.right, OuterExpressionKinds.All);
40570-
const nullishSemantics = getSyntacticNullishnessSemantics(rightTarget);
40571-
if (isNotWithinNullishCoalesceExpression(node)) {
40572-
return;
40573-
}
40574-
40575-
if (nullishSemantics === PredicateSemantics.Always) {
40576-
error(rightTarget, Diagnostics.This_expression_is_always_nullish);
40577-
}
40578-
else if (nullishSemantics === PredicateSemantics.Never) {
40579-
error(rightTarget, Diagnostics.This_expression_is_never_nullish);
40580-
}
40581-
}
40582-
40583-
function isNotWithinNullishCoalesceExpression(node: BinaryExpression) {
40584-
return !isBinaryExpression(node.parent) || node.parent.operatorToken.kind !== SyntaxKind.QuestionQuestionToken;
40585-
}
40586-
4058740566
function getSyntacticNullishnessSemantics(node: Node): PredicateSemantics {
4058840567
node = skipOuterExpressions(node);
4058940568
switch (node.kind) {
@@ -40601,15 +40580,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4060140580
// List of operators that can produce null/undefined:
4060240581
// = ??= ?? || ||= && &&=
4060340582
switch ((node as BinaryExpression).operatorToken.kind) {
40604-
case SyntaxKind.EqualsToken:
40605-
case SyntaxKind.QuestionQuestionToken:
40606-
case SyntaxKind.QuestionQuestionEqualsToken:
4060740583
case SyntaxKind.BarBarToken:
4060840584
case SyntaxKind.BarBarEqualsToken:
4060940585
case SyntaxKind.AmpersandAmpersandToken:
4061040586
case SyntaxKind.AmpersandAmpersandEqualsToken:
4061140587
return PredicateSemantics.Sometimes;
40588+
// For these operator kinds, the right operand is effectively controlling
4061240589
case SyntaxKind.CommaToken:
40590+
case SyntaxKind.EqualsToken:
40591+
case SyntaxKind.QuestionQuestionToken:
40592+
case SyntaxKind.QuestionQuestionEqualsToken:
4061340593
return getSyntacticNullishnessSemantics((node as BinaryExpression).right);
4061440594
}
4061540595
return PredicateSemantics.Never;

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
neverNullishThroughParentheses.ts(6,13): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
2+
neverNullishThroughParentheses.ts(7,14): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
3+
neverNullishThroughParentheses.ts(10,15): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
4+
neverNullishThroughParentheses.ts(11,16): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
5+
neverNullishThroughParentheses.ts(14,15): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
6+
neverNullishThroughParentheses.ts(15,16): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
7+
neverNullishThroughParentheses.ts(16,17): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
8+
neverNullishThroughParentheses.ts(16,17): error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
9+
10+
11+
==== neverNullishThroughParentheses.ts (8 errors) ====
12+
// Repro for issue where "never nullish" checks miss "never nullish" through parentheses
13+
14+
const x: { y: string | undefined } | undefined = undefined as any;
15+
16+
// Both should error - both expressions are guaranteed to be "oops"
17+
const foo = x?.y ?? `oops` ?? "";
18+
~~~~~~~~~~~~~~
19+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
20+
const bar = (x?.y ?? `oops`) ?? "";
21+
~~~~~~~~~~~~~~
22+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
23+
24+
// Additional test cases with various levels of nesting
25+
const baz = ((x?.y ?? `oops`)) ?? "";
26+
~~~~~~~~~~~~~~
27+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
28+
const qux = (((x?.y ?? `oops`))) ?? "";
29+
~~~~~~~~~~~~~~
30+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
31+
32+
// Test with different types
33+
const str1 = ("literal") ?? "fallback";
34+
~~~~~~~~~
35+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
36+
const str2 = (("nested")) ?? "fallback";
37+
~~~~~~~~
38+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
39+
const nested = ("a" ?? "b") ?? "c";
40+
~~~
41+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
42+
~~~~~~~~~~
43+
!!! error TS2869: Right operand of ?? is unreachable because the left operand is never nullish.
44+

‎tests/baselines/reference/neverNullishThroughParentheses.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//// [tests/cases/compiler/neverNullishThroughParentheses.ts] ////
2+
3+
//// [neverNullishThroughParentheses.ts]
4+
// Repro for issue where "never nullish" checks miss "never nullish" through parentheses
5+
6+
const x: { y: string | undefined } | undefined = undefined as any;
7+
8+
// Both should error - both expressions are guaranteed to be "oops"
9+
const foo = x?.y ?? `oops` ?? "";
10+
const bar = (x?.y ?? `oops`) ?? "";
11+
12+
// Additional test cases with various levels of nesting
13+
const baz = ((x?.y ?? `oops`)) ?? "";
14+
const qux = (((x?.y ?? `oops`))) ?? "";
15+
16+
// Test with different types
17+
const str1 = ("literal") ?? "fallback";
18+
const str2 = (("nested")) ?? "fallback";
19+
const nested = ("a" ?? "b") ?? "c";
20+
21+
22+
//// [neverNullishThroughParentheses.js]
23+
"use strict";
24+
// Repro for issue where "never nullish" checks miss "never nullish" through parentheses
25+
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l;
26+
var x = undefined;
27+
// Both should error - both expressions are guaranteed to be "oops"
28+
var foo = (_b = (_a = x === null || x === void 0 ? void 0 : x.y) !== null && _a !== void 0 ? _a : "oops") !== null && _b !== void 0 ? _b : "";
29+
var bar = (_d = ((_c = x === null || x === void 0 ? void 0 : x.y) !== null && _c !== void 0 ? _c : "oops")) !== null && _d !== void 0 ? _d : "";
30+
// Additional test cases with various levels of nesting
31+
var baz = (_f = (((_e = x === null || x === void 0 ? void 0 : x.y) !== null && _e !== void 0 ? _e : "oops"))) !== null && _f !== void 0 ? _f : "";
32+
var qux = (_h = ((((_g = x === null || x === void 0 ? void 0 : x.y) !== null && _g !== void 0 ? _g : "oops")))) !== null && _h !== void 0 ? _h : "";
33+
// Test with different types
34+
var str1 = (_j = ("literal")) !== null && _j !== void 0 ? _j : "fallback";
35+
var str2 = (_k = (("nested"))) !== null && _k !== void 0 ? _k : "fallback";
36+
var nested = (_l = ("a" !== null && "a" !== void 0 ? "a" : "b")) !== null && _l !== void 0 ? _l : "c";

‎tests/baselines/reference/neverNullishThroughParentheses.symbols‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
//// [tests/cases/compiler/neverNullishThroughParentheses.ts] ////
2+
3+
=== neverNullishThroughParentheses.ts ===
4+
// Repro for issue where "never nullish" checks miss "never nullish" through parentheses
5+
6+
const x: { y: string | undefined } | undefined = undefined as any;
7+
>x : Symbol(x, Decl(neverNullishThroughParentheses.ts, 2, 5))
8+
>y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
9+
>undefined : Symbol(undefined)
10+
11+
// Both should error - both expressions are guaranteed to be "oops"
12+
const foo = x?.y ?? `oops` ?? "";
13+
>foo : Symbol(foo, Decl(neverNullishThroughParentheses.ts, 5, 5))
14+
>x?.y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
15+
>x : Symbol(x, Decl(neverNullishThroughParentheses.ts, 2, 5))
16+
>y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
17+
18+
const bar = (x?.y ?? `oops`) ?? "";
19+
>bar : Symbol(bar, Decl(neverNullishThroughParentheses.ts, 6, 5))
20+
>x?.y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
21+
>x : Symbol(x, Decl(neverNullishThroughParentheses.ts, 2, 5))
22+
>y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
23+
24+
// Additional test cases with various levels of nesting
25+
const baz = ((x?.y ?? `oops`)) ?? "";
26+
>baz : Symbol(baz, Decl(neverNullishThroughParentheses.ts, 9, 5))
27+
>x?.y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
28+
>x : Symbol(x, Decl(neverNullishThroughParentheses.ts, 2, 5))
29+
>y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
30+
31+
const qux = (((x?.y ?? `oops`))) ?? "";
32+
>qux : Symbol(qux, Decl(neverNullishThroughParentheses.ts, 10, 5))
33+
>x?.y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
34+
>x : Symbol(x, Decl(neverNullishThroughParentheses.ts, 2, 5))
35+
>y : Symbol(y, Decl(neverNullishThroughParentheses.ts, 2, 10))
36+
37+
// Test with different types
38+
const str1 = ("literal") ?? "fallback";
39+
>str1 : Symbol(str1, Decl(neverNullishThroughParentheses.ts, 13, 5))
40+
41+
const str2 = (("nested")) ?? "fallback";
42+
>str2 : Symbol(str2, Decl(neverNullishThroughParentheses.ts, 14, 5))
43+
44+
const nested = ("a" ?? "b") ?? "c";
45+
>nested : Symbol(nested, Decl(neverNullishThroughParentheses.ts, 15, 5))
46+

‎tests/baselines/reference/neverNullishThroughParentheses.types‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
//// [tests/cases/compiler/neverNullishThroughParentheses.ts] ////
2+
3+
=== neverNullishThroughParentheses.ts ===
4+
// Repro for issue where "never nullish" checks miss "never nullish" through parentheses
5+
6+
const x: { y: string | undefined } | undefined = undefined as any;
7+
>x : { y: string | undefined; } | undefined
8+
> : ^^^^^ ^^^^^^^^^^^^^^^
9+
>y : string | undefined
10+
> : ^^^^^^^^^^^^^^^^^^
11+
>undefined as any : any
12+
> : ^^^
13+
>undefined : undefined
14+
> : ^^^^^^^^^
15+
16+
// Both should error - both expressions are guaranteed to be "oops"
17+
const foo = x?.y ?? `oops` ?? "";
18+
>foo : string
19+
> : ^^^^^^
20+
>x?.y ?? `oops` ?? "" : string
21+
> : ^^^^^^
22+
>x?.y ?? `oops` : string
23+
> : ^^^^^^
24+
>x?.y : string | undefined
25+
> : ^^^^^^^^^^^^^^^^^^
26+
>x : { y: string | undefined; } | undefined
27+
> : ^^^^^ ^^^^^^^^^^^^^^^
28+
>y : string | undefined
29+
> : ^^^^^^^^^^^^^^^^^^
30+
>`oops` : "oops"
31+
> : ^^^^^^
32+
>"" : ""
33+
> : ^^
34+
35+
const bar = (x?.y ?? `oops`) ?? "";
36+
>bar : string
37+
> : ^^^^^^
38+
>(x?.y ?? `oops`) ?? "" : string
39+
> : ^^^^^^
40+
>(x?.y ?? `oops`) : string
41+
> : ^^^^^^
42+
>x?.y ?? `oops` : string
43+
> : ^^^^^^
44+
>x?.y : string | undefined
45+
> : ^^^^^^^^^^^^^^^^^^
46+
>x : { y: string | undefined; } | undefined
47+
> : ^^^^^ ^^^^^^^^^^^^^^^
48+
>y : string | undefined
49+
> : ^^^^^^^^^^^^^^^^^^
50+
>`oops` : "oops"
51+
> : ^^^^^^
52+
>"" : ""
53+
> : ^^
54+
55+
// Additional test cases with various levels of nesting
56+
const baz = ((x?.y ?? `oops`)) ?? "";
57+
>baz : string
58+
> : ^^^^^^
59+
>((x?.y ?? `oops`)) ?? "" : string
60+
> : ^^^^^^
61+
>((x?.y ?? `oops`)) : string
62+
> : ^^^^^^
63+
>(x?.y ?? `oops`) : string
64+
> : ^^^^^^
65+
>x?.y ?? `oops` : string
66+
> : ^^^^^^
67+
>x?.y : string | undefined
68+
> : ^^^^^^^^^^^^^^^^^^
69+
>x : { y: string | undefined; } | undefined
70+
> : ^^^^^ ^^^^^^^^^^^^^^^
71+
>y : string | undefined
72+
> : ^^^^^^^^^^^^^^^^^^
73+
>`oops` : "oops"
74+
> : ^^^^^^
75+
>"" : ""
76+
> : ^^
77+
78+
const qux = (((x?.y ?? `oops`))) ?? "";
79+
>qux : string
80+
> : ^^^^^^
81+
>(((x?.y ?? `oops`))) ?? "" : string
82+
> : ^^^^^^
83+
>(((x?.y ?? `oops`))) : string
84+
> : ^^^^^^
85+
>((x?.y ?? `oops`)) : string
86+
> : ^^^^^^
87+
>(x?.y ?? `oops`) : string
88+
> : ^^^^^^
89+
>x?.y ?? `oops` : string
90+
> : ^^^^^^
91+
>x?.y : string | undefined
92+
> : ^^^^^^^^^^^^^^^^^^
93+
>x : { y: string | undefined; } | undefined
94+
> : ^^^^^ ^^^^^^^^^^^^^^^
95+
>y : string | undefined
96+
> : ^^^^^^^^^^^^^^^^^^
97+
>`oops` : "oops"
98+
> : ^^^^^^
99+
>"" : ""
100+
> : ^^
101+
102+
// Test with different types
103+
const str1 = ("literal") ?? "fallback";
104+
>str1 : "literal"
105+
> : ^^^^^^^^^
106+
>("literal") ?? "fallback" : "literal"
107+
> : ^^^^^^^^^
108+
>("literal") : "literal"
109+
> : ^^^^^^^^^
110+
>"literal" : "literal"
111+
> : ^^^^^^^^^
112+
>"fallback" : "fallback"
113+
> : ^^^^^^^^^^
114+
115+
const str2 = (("nested")) ?? "fallback";
116+
>str2 : "nested"
117+
> : ^^^^^^^^
118+
>(("nested")) ?? "fallback" : "nested"
119+
> : ^^^^^^^^
120+
>(("nested")) : "nested"
121+
> : ^^^^^^^^
122+
>("nested") : "nested"
123+
> : ^^^^^^^^
124+
>"nested" : "nested"
125+
> : ^^^^^^^^
126+
>"fallback" : "fallback"
127+
> : ^^^^^^^^^^
128+
129+
const nested = ("a" ?? "b") ?? "c";
130+
>nested : "a"
131+
> : ^^^
132+
>("a" ?? "b") ?? "c" : "a"
133+
> : ^^^
134+
>("a" ?? "b") : "a"
135+
> : ^^^
136+
>"a" ?? "b" : "a"
137+
> : ^^^
138+
>"a" : "a"
139+
> : ^^^
140+
>"b" : "b"
141+
> : ^^^
142+
>"c" : "c"
143+
> : ^^^
144+

0 commit comments

Comments
 (0)