Skip to content

Commit c404c08

Browse files
ahejlsbergahejlsberg
authored
Fix reachability analysis for ?? operator (microsoft#34702)
* Exclude ?? operator from true/false literal check in createFlowCondition * Accept new API baselines * Add tests * Accept new baselines * Address CR feedback * Accept new API baselines
1 parent a03227d commit c404c08

File tree

8 files changed

+166
-7
lines changed

8 files changed

+166
-7
lines changed

‎src/compiler/binder.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
/* @internal */
23
namespace ts {
34
export const enum ModuleInstanceState {
@@ -951,11 +952,10 @@ namespace ts {
951952
if (!expression) {
952953
return flags & FlowFlags.TrueCondition ? antecedent : unreachableFlow;
953954
}
954-
if (expression.kind === SyntaxKind.TrueKeyword && flags & FlowFlags.FalseCondition ||
955-
expression.kind === SyntaxKind.FalseKeyword && flags & FlowFlags.TrueCondition) {
956-
if (!isExpressionOfOptionalChainRoot(expression)) {
957-
return unreachableFlow;
958-
}
955+
if ((expression.kind === SyntaxKind.TrueKeyword && flags & FlowFlags.FalseCondition ||
956+
expression.kind === SyntaxKind.FalseKeyword && flags & FlowFlags.TrueCondition) &&
957+
!isExpressionOfOptionalChainRoot(expression) && !isNullishCoalesce(expression.parent)) {
958+
return unreachableFlow;
959959
}
960960
if (!isNarrowingExpression(expression)) {
961961
return antecedent;

‎src/compiler/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5955,6 +5955,10 @@ namespace ts {
59555955
return isOptionalChainRoot(node.parent) && node.parent.expression === node;
59565956
}
59575957

5958+
export function isNullishCoalesce(node: Node) {
5959+
return node.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>node).operatorToken.kind === SyntaxKind.QuestionQuestionToken;
5960+
}
5961+
59585962
export function isNewExpression(node: Node): node is NewExpression {
59595963
return node.kind === SyntaxKind.NewExpression;
59605964
}

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3525,6 +3525,7 @@ declare namespace ts {
35253525
function isCallExpression(node: Node): node is CallExpression;
35263526
function isCallChain(node: Node): node is CallChain;
35273527
function isOptionalChain(node: Node): node is PropertyAccessChain | ElementAccessChain | CallChain;
3528+
function isNullishCoalesce(node: Node): boolean;
35283529
function isNewExpression(node: Node): node is NewExpression;
35293530
function isTaggedTemplateExpression(node: Node): node is TaggedTemplateExpression;
35303531
function isTypeAssertion(node: Node): node is TypeAssertion;

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3525,6 +3525,7 @@ declare namespace ts {
35253525
function isCallExpression(node: Node): node is CallExpression;
35263526
function isCallChain(node: Node): node is CallChain;
35273527
function isOptionalChain(node: Node): node is PropertyAccessChain | ElementAccessChain | CallChain;
3528+
function isNullishCoalesce(node: Node): boolean;
35283529
function isNewExpression(node: Node): node is NewExpression;
35293530
function isTaggedTemplateExpression(node: Node): node is TaggedTemplateExpression;
35303531
function isTypeAssertion(node: Node): node is TypeAssertion;

‎tests/baselines/reference/nullishCoalescingOperator1.js

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,36 @@ const cc4 = c4 ?? true;
3838
const dd1 = d1 ?? {b: 1};
3939
const dd2 = d2 ?? {b: 1};
4040
const dd3 = d3 ?? {b: 1};
41-
const dd4 = d4 ?? {b: 1};
41+
const dd4 = d4 ?? {b: 1};
42+
43+
// Repro from #34635
44+
45+
declare function foo(): void;
46+
47+
const maybeBool = false;
48+
49+
if (!(maybeBool ?? true)) {
50+
foo();
51+
}
52+
53+
if (maybeBool ?? true) {
54+
foo();
55+
}
56+
else {
57+
foo();
58+
}
59+
60+
if (false ?? true) {
61+
foo();
62+
}
63+
else {
64+
foo();
65+
}
66+
4267

4368
//// [nullishCoalescingOperator1.js]
4469
"use strict";
70+
var _a;
4571
var aa1 = (a1 !== null && a1 !== void 0 ? a1 : 'whatever');
4672
var aa2 = (a2 !== null && a2 !== void 0 ? a2 : 'whatever');
4773
var aa3 = (a3 !== null && a3 !== void 0 ? a3 : 'whatever');
@@ -58,3 +84,19 @@ var dd1 = (d1 !== null && d1 !== void 0 ? d1 : { b: 1 });
5884
var dd2 = (d2 !== null && d2 !== void 0 ? d2 : { b: 1 });
5985
var dd3 = (d3 !== null && d3 !== void 0 ? d3 : { b: 1 });
6086
var dd4 = (d4 !== null && d4 !== void 0 ? d4 : { b: 1 });
87+
var maybeBool = false;
88+
if (!((maybeBool !== null && maybeBool !== void 0 ? maybeBool : true))) {
89+
foo();
90+
}
91+
if ((maybeBool !== null && maybeBool !== void 0 ? maybeBool : true)) {
92+
foo();
93+
}
94+
else {
95+
foo();
96+
}
97+
if (_a = false, (_a !== null && _a !== void 0 ? _a : true)) {
98+
foo();
99+
}
100+
else {
101+
foo();
102+
}

‎tests/baselines/reference/nullishCoalescingOperator1.symbols

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,38 @@ const dd4 = d4 ?? {b: 1};
123123
>d4 : Symbol(d4, Decl(nullishCoalescingOperator1.ts, 19, 13))
124124
>b : Symbol(b, Decl(nullishCoalescingOperator1.ts, 39, 19))
125125

126+
// Repro from #34635
127+
128+
declare function foo(): void;
129+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
130+
131+
const maybeBool = false;
132+
>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
133+
134+
if (!(maybeBool ?? true)) {
135+
>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
136+
137+
foo();
138+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
139+
}
140+
141+
if (maybeBool ?? true) {
142+
>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
143+
144+
foo();
145+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
146+
}
147+
else {
148+
foo();
149+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
150+
}
151+
152+
if (false ?? true) {
153+
foo();
154+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
155+
}
156+
else {
157+
foo();
158+
>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
159+
}
160+

‎tests/baselines/reference/nullishCoalescingOperator1.types

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,54 @@ const dd4 = d4 ?? {b: 1};
170170
>b : number
171171
>1 : 1
172172

173+
// Repro from #34635
174+
175+
declare function foo(): void;
176+
>foo : () => void
177+
178+
const maybeBool = false;
179+
>maybeBool : false
180+
>false : false
181+
182+
if (!(maybeBool ?? true)) {
183+
>!(maybeBool ?? true) : true
184+
>(maybeBool ?? true) : false
185+
>maybeBool ?? true : false
186+
>maybeBool : false
187+
>true : true
188+
189+
foo();
190+
>foo() : void
191+
>foo : () => void
192+
}
193+
194+
if (maybeBool ?? true) {
195+
>maybeBool ?? true : false
196+
>maybeBool : false
197+
>true : true
198+
199+
foo();
200+
>foo() : void
201+
>foo : () => void
202+
}
203+
else {
204+
foo();
205+
>foo() : void
206+
>foo : () => void
207+
}
208+
209+
if (false ?? true) {
210+
>false ?? true : false
211+
>false : false
212+
>true : true
213+
214+
foo();
215+
>foo() : void
216+
>foo : () => void
217+
}
218+
else {
219+
foo();
220+
>foo() : void
221+
>foo : () => void
222+
}
223+

‎tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator1.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// @strict: true
2+
// @allowUnreachableCode: false
23

34
declare const a1: string | undefined | null
45
declare const a2: string | undefined | null
@@ -39,4 +40,28 @@ const cc4 = c4 ?? true;
3940
const dd1 = d1 ?? {b: 1};
4041
const dd2 = d2 ?? {b: 1};
4142
const dd3 = d3 ?? {b: 1};
42-
const dd4 = d4 ?? {b: 1};
43+
const dd4 = d4 ?? {b: 1};
44+
45+
// Repro from #34635
46+
47+
declare function foo(): void;
48+
49+
const maybeBool = false;
50+
51+
if (!(maybeBool ?? true)) {
52+
foo();
53+
}
54+
55+
if (maybeBool ?? true) {
56+
foo();
57+
}
58+
else {
59+
foo();
60+
}
61+
62+
if (false ?? true) {
63+
foo();
64+
}
65+
else {
66+
foo();
67+
}

0 commit comments

Comments
 (0)