Fix reachability analysis for ?? operator (#34702)

ahejlsberg · web-flow · commit c404c08fdefd · 2019-10-24T15:57:14.000-07:00
* 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
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -1,3 +1,4 @@
+
 /* @internal */
 namespace ts {
     export const enum ModuleInstanceState {
@@ -951,11 +952,10 @@ namespace ts {
             if (!expression) {
                 return flags & FlowFlags.TrueCondition ? antecedent : unreachableFlow;
             }
-            if (expression.kind === SyntaxKind.TrueKeyword && flags & FlowFlags.FalseCondition ||
-                expression.kind === SyntaxKind.FalseKeyword && flags & FlowFlags.TrueCondition) {
-                if (!isExpressionOfOptionalChainRoot(expression)) {
-                    return unreachableFlow;
-                }
+            if ((expression.kind === SyntaxKind.TrueKeyword && flags & FlowFlags.FalseCondition ||
+                expression.kind === SyntaxKind.FalseKeyword && flags & FlowFlags.TrueCondition) &&
+                !isExpressionOfOptionalChainRoot(expression) && !isNullishCoalesce(expression.parent)) {
+                return unreachableFlow;
             }
             if (!isNarrowingExpression(expression)) {
                 return antecedent;
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -5955,6 +5955,10 @@ namespace ts {
         return isOptionalChainRoot(node.parent) && node.parent.expression === node;
     }
 
+    export function isNullishCoalesce(node: Node) {
+        return node.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>node).operatorToken.kind === SyntaxKind.QuestionQuestionToken;
+    }
+
     export function isNewExpression(node: Node): node is NewExpression {
         return node.kind === SyntaxKind.NewExpression;
     }
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -3525,6 +3525,7 @@ declare namespace ts {
     function isCallExpression(node: Node): node is CallExpression;
     function isCallChain(node: Node): node is CallChain;
     function isOptionalChain(node: Node): node is PropertyAccessChain | ElementAccessChain | CallChain;
+    function isNullishCoalesce(node: Node): boolean;
     function isNewExpression(node: Node): node is NewExpression;
     function isTaggedTemplateExpression(node: Node): node is TaggedTemplateExpression;
     function isTypeAssertion(node: Node): node is TypeAssertion;
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -3525,6 +3525,7 @@ declare namespace ts {
     function isCallExpression(node: Node): node is CallExpression;
     function isCallChain(node: Node): node is CallChain;
     function isOptionalChain(node: Node): node is PropertyAccessChain | ElementAccessChain | CallChain;
+    function isNullishCoalesce(node: Node): boolean;
     function isNewExpression(node: Node): node is NewExpression;
     function isTaggedTemplateExpression(node: Node): node is TaggedTemplateExpression;
     function isTypeAssertion(node: Node): node is TypeAssertion;
diff --git a/tests/baselines/reference/nullishCoalescingOperator1.js b/tests/baselines/reference/nullishCoalescingOperator1.js
@@ -38,10 +38,36 @@ const cc4 = c4 ?? true;
 const dd1 = d1 ?? {b: 1};
 const dd2 = d2 ?? {b: 1};
 const dd3 = d3 ?? {b: 1};
-const dd4 = d4 ?? {b: 1};
+const dd4 = d4 ?? {b: 1};
+
+// Repro from #34635
+
+declare function foo(): void;
+
+const maybeBool = false;
+
+if (!(maybeBool ?? true)) {
+    foo();
+}
+
+if (maybeBool ?? true) {
+    foo();
+}
+else {
+    foo();
+}
+
+if (false ?? true) {
+    foo();
+}
+else {
+    foo();
+}
+
 
 //// [nullishCoalescingOperator1.js]
 "use strict";
+var _a;
 var aa1 = (a1 !== null && a1 !== void 0 ? a1 : 'whatever');
 var aa2 = (a2 !== null && a2 !== void 0 ? a2 : 'whatever');
 var aa3 = (a3 !== null && a3 !== void 0 ? a3 : 'whatever');
@@ -58,3 +84,19 @@ var dd1 = (d1 !== null && d1 !== void 0 ? d1 : { b: 1 });
 var dd2 = (d2 !== null && d2 !== void 0 ? d2 : { b: 1 });
 var dd3 = (d3 !== null && d3 !== void 0 ? d3 : { b: 1 });
 var dd4 = (d4 !== null && d4 !== void 0 ? d4 : { b: 1 });
+var maybeBool = false;
+if (!((maybeBool !== null && maybeBool !== void 0 ? maybeBool : true))) {
+    foo();
+}
+if ((maybeBool !== null && maybeBool !== void 0 ? maybeBool : true)) {
+    foo();
+}
+else {
+    foo();
+}
+if (_a = false, (_a !== null && _a !== void 0 ? _a : true)) {
+    foo();
+}
+else {
+    foo();
+}
diff --git a/tests/baselines/reference/nullishCoalescingOperator1.symbols b/tests/baselines/reference/nullishCoalescingOperator1.symbols
@@ -123,3 +123,38 @@ const dd4 = d4 ?? {b: 1};
 >d4 : Symbol(d4, Decl(nullishCoalescingOperator1.ts, 19, 13))
 >b : Symbol(b, Decl(nullishCoalescingOperator1.ts, 39, 19))
 
+// Repro from #34635
+
+declare function foo(): void;
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+
+const maybeBool = false;
+>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
+
+if (!(maybeBool ?? true)) {
+>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
+
+    foo();
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+}
+
+if (maybeBool ?? true) {
+>maybeBool : Symbol(maybeBool, Decl(nullishCoalescingOperator1.ts, 45, 5))
+
+    foo();
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+}
+else {
+    foo();
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+}
+
+if (false ?? true) {
+    foo();
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+}
+else {
+    foo();
+>foo : Symbol(foo, Decl(nullishCoalescingOperator1.ts, 39, 25))
+}
+
diff --git a/tests/baselines/reference/nullishCoalescingOperator1.types b/tests/baselines/reference/nullishCoalescingOperator1.types
@@ -170,3 +170,54 @@ const dd4 = d4 ?? {b: 1};
 >b : number
 >1 : 1
 
+// Repro from #34635
+
+declare function foo(): void;
+>foo : () => void
+
+const maybeBool = false;
+>maybeBool : false
+>false : false
+
+if (!(maybeBool ?? true)) {
+>!(maybeBool ?? true) : true
+>(maybeBool ?? true) : false
+>maybeBool ?? true : false
+>maybeBool : false
+>true : true
+
+    foo();
+>foo() : void
+>foo : () => void
+}
+
+if (maybeBool ?? true) {
+>maybeBool ?? true : false
+>maybeBool : false
+>true : true
+
+    foo();
+>foo() : void
+>foo : () => void
+}
+else {
+    foo();
+>foo() : void
+>foo : () => void
+}
+
+if (false ?? true) {
+>false ?? true : false
+>false : false
+>true : true
+
+    foo();
+>foo() : void
+>foo : () => void
+}
+else {
+    foo();
+>foo() : void
+>foo : () => void
+}
+
diff --git a/tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator1.ts b/tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator1.ts
@@ -1,4 +1,5 @@
 // @strict: true
+// @allowUnreachableCode: false
 
 declare const a1: string | undefined | null
 declare const a2: string | undefined | null
@@ -39,4 +40,28 @@ const cc4 = c4 ?? true;
 const dd1 = d1 ?? {b: 1};
 const dd2 = d2 ?? {b: 1};
 const dd3 = d3 ?? {b: 1};
-const dd4 = d4 ?? {b: 1};
+const dd4 = d4 ?? {b: 1};
+
+// Repro from #34635
+
+declare function foo(): void;
+
+const maybeBool = false;
+
+if (!(maybeBool ?? true)) {
+    foo();
+}
+
+if (maybeBool ?? true) {
+    foo();
+}
+else {
+    foo();
+}
+
+if (false ?? true) {
+    foo();
+}
+else {
+    foo();
+}

Original file line number	Diff line number	Diff line change
`@@ -5955,6 +5955,10 @@ namespace ts {`
`5955`	`5955`	`return isOptionalChainRoot(node.parent) && node.parent.expression === node;`
`5956`	`5956`	`}`
`5957`	`5957`
	`5958`	`+ export function isNullishCoalesce(node: Node) {`
	`5959`	`+ return node.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>node).operatorToken.kind === SyntaxKind.QuestionQuestionToken;`
	`5960`	`+ }`
	`5961`	`+`
`5958`	`5962`	`export function isNewExpression(node: Node): node is NewExpression {`
`5959`	`5963`	`return node.kind === SyntaxKind.NewExpression;`
`5960`	`5964`	`}`