From c404c08fdefde5306e49df2d87f4d55435ddad44 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 24 Oct 2019 15:57:14 -0700 Subject: [PATCH] Fix reachability analysis for ?? operator (#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 --- src/compiler/binder.ts | 10 ++-- src/compiler/utilities.ts | 4 ++ .../reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + .../reference/nullishCoalescingOperator1.js | 44 +++++++++++++++- .../nullishCoalescingOperator1.symbols | 35 +++++++++++++ .../nullishCoalescingOperator1.types | 51 +++++++++++++++++++ .../nullishCoalescingOperator1.ts | 27 +++++++++- 8 files changed, 166 insertions(+), 7 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 5f36d429556..1cc29326660 100644 --- 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 index 12b11eeae29..4bff443754b 100644 --- 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 && (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 index c43f28c6e90..c06d040800d 100644 --- 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 index e84b948c938..fe23df054da 100644 --- 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 index 87a149c1855..9b857a1473d 100644 --- 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 index fb95dee5096..83bd5162280 100644 --- 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 index 0e800540ee8..81aa6a7404c 100644 --- 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 index 80f0c64e7d9..f7024fd8292 100644 --- 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}; \ No newline at end of file +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(); +}