Skip to content

Commit

Permalink
Fixes for control-flow analysis
Browse files Browse the repository at this point in the history
  • Loading branch information
rbuckton committed Sep 30, 2019
1 parent 8c572b1 commit b030f73
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 21 deletions.
14 changes: 2 additions & 12 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,8 @@ namespace ts {
else {
return node.kind === SyntaxKind.BinaryExpression && (
(<BinaryExpression>node).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken ||
(<BinaryExpression>node).operatorToken.kind === SyntaxKind.BarBarToken);
(<BinaryExpression>node).operatorToken.kind === SyntaxKind.BarBarToken ||
(<BinaryExpression>node).operatorToken.kind === SyntaxKind.QuestionQuestionToken);
}
}
}
Expand Down Expand Up @@ -1440,14 +1441,6 @@ namespace ts {
bindCondition(node.right, trueTarget, falseTarget);
}

function bindNullishCoalescingExpression(node: BinaryExpression, trueTarget: FlowLabel, falseTarget: FlowLabel) {
const notNullLabel = createBranchLabel();
bindCondition(node.left, trueTarget, notNullLabel);
currentFlow = finishFlowLabel(notNullLabel);
bind(node.operatorToken);
bindCondition(node.right, trueTarget, falseTarget);
}

function bindPrefixUnaryExpressionFlow(node: PrefixUnaryExpression) {
if (node.operator === SyntaxKind.ExclamationToken) {
const saveTrueTarget = currentTrueTarget;
Expand Down Expand Up @@ -1480,9 +1473,6 @@ namespace ts {
bindLogicalExpression(node, postExpressionLabel, postExpressionLabel);
currentFlow = finishFlowLabel(postExpressionLabel);
}
else if (operator === SyntaxKind.QuestionQuestionToken) {
bindNullishCoalescingExpression(node, currentTrueTarget!, currentFalseTarget!);
}
else {
bindLogicalExpression(node, currentTrueTarget!, currentFalseTarget!);
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19697,7 +19697,8 @@ namespace ts {
// will be a subtype or the same type as the argument.
function narrowType(type: Type, expr: Expression, assumeTrue: boolean): Type {
// for `a?.b`, we emulate a synthetic `a !== null && a !== undefined` condition for `a`
if (isOptionalChainRoot(expr.parent)) {
if (isOptionalChainRoot(expr.parent) ||
isBinaryExpression(expr.parent) && expr.parent.operatorToken.kind === SyntaxKind.QuestionQuestionToken && expr.parent.left === expr) {
return narrowTypeByOptionality(type, expr, assumeTrue);
}
switch (expr.kind) {
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/controlFlowNullishCoalesce.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
tests/cases/conformance/controlFlow/controlFlowNullishCoalesce.ts(4,1): error TS2454: Variable 'a' is used before being assigned.


==== tests/cases/conformance/controlFlow/controlFlowNullishCoalesce.ts (1 errors) ====
// assignments in shortcutting rhs
let a: number;
o ?? (a = 1);
a.toString();
~
!!! error TS2454: Variable 'a' is used before being assigned.

// assignment flow
declare const o: { x: number } | undefined;
let x: { x: number } | boolean;
if (x = o ?? true) {
x;
}


25 changes: 25 additions & 0 deletions tests/baselines/reference/controlFlowNullishCoalesce.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//// [controlFlowNullishCoalesce.ts]
// assignments in shortcutting rhs
let a: number;
o ?? (a = 1);
a.toString();

// assignment flow
declare const o: { x: number } | undefined;
let x: { x: number } | boolean;
if (x = o ?? true) {
x;
}



//// [controlFlowNullishCoalesce.js]
"use strict";
// assignments in shortcutting rhs
var a;
(o !== null && o !== void 0 ? o : (a = 1));
a.toString();
var x;
if (x = (o !== null && o !== void 0 ? o : true)) {
x;
}
32 changes: 32 additions & 0 deletions tests/baselines/reference/controlFlowNullishCoalesce.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
=== tests/cases/conformance/controlFlow/controlFlowNullishCoalesce.ts ===
// assignments in shortcutting rhs
let a: number;
>a : Symbol(a, Decl(controlFlowNullishCoalesce.ts, 1, 3))

o ?? (a = 1);
>o : Symbol(o, Decl(controlFlowNullishCoalesce.ts, 6, 13))
>a : Symbol(a, Decl(controlFlowNullishCoalesce.ts, 1, 3))

a.toString();
>a.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>a : Symbol(a, Decl(controlFlowNullishCoalesce.ts, 1, 3))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

// assignment flow
declare const o: { x: number } | undefined;
>o : Symbol(o, Decl(controlFlowNullishCoalesce.ts, 6, 13))
>x : Symbol(x, Decl(controlFlowNullishCoalesce.ts, 6, 18))

let x: { x: number } | boolean;
>x : Symbol(x, Decl(controlFlowNullishCoalesce.ts, 7, 3))
>x : Symbol(x, Decl(controlFlowNullishCoalesce.ts, 7, 8))

if (x = o ?? true) {
>x : Symbol(x, Decl(controlFlowNullishCoalesce.ts, 7, 3))
>o : Symbol(o, Decl(controlFlowNullishCoalesce.ts, 6, 13))

x;
>x : Symbol(x, Decl(controlFlowNullishCoalesce.ts, 7, 3))
}


40 changes: 40 additions & 0 deletions tests/baselines/reference/controlFlowNullishCoalesce.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/conformance/controlFlow/controlFlowNullishCoalesce.ts ===
// assignments in shortcutting rhs
let a: number;
>a : number

o ?? (a = 1);
>o ?? (a = 1) : { x: number; } | 1
>o : { x: number; } | undefined
>(a = 1) : 1
>a = 1 : 1
>a : number
>1 : 1

a.toString();
>a.toString() : string
>a.toString : (radix?: number | undefined) => string
>a : number
>toString : (radix?: number | undefined) => string

// assignment flow
declare const o: { x: number } | undefined;
>o : { x: number; } | undefined
>x : number

let x: { x: number } | boolean;
>x : boolean | { x: number; }
>x : number

if (x = o ?? true) {
>x = o ?? true : true | { x: number; }
>x : boolean | { x: number; }
>o ?? true : true | { x: number; }
>o : { x: number; } | undefined
>true : true

x;
>x : true | { x: number; }
}


Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator11.ts(3,18): error TS2533: Object is possibly 'null' or 'undefined'.
tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator11.ts(3,22): error TS2339: Property 'toFixed' does not exist on type '"" | 0'.
Property 'toFixed' does not exist on type '""'.


==== tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator11.ts (2 errors) ====
==== tests/cases/conformance/expressions/nullishCoalescingOperator/nullishCoalescingOperator11.ts (1 errors) ====
declare const f11: 1 | 0 | '' | null | undefined;

let g11 = f11 ?? f11.toFixed()
~~~
!!! error TS2533: Object is possibly 'null' or 'undefined'.
~~~~~~~
!!! error TS2339: Property 'toFixed' does not exist on type '"" | 0'.
!!! error TS2339: Property 'toFixed' does not exist on type '""'.



Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ let g11 = f11 ?? f11.toFixed()
>f11 : "" | 0 | 1 | null | undefined
>f11.toFixed() : any
>f11.toFixed : any
>f11 : "" | 0 | null | undefined
>f11 : null | undefined
>toFixed : any


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/nullishCoalescingOperator5.types
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ a ?? b || c;
a || b ?? c;
>a || b ?? c : string | undefined
>a || b : string | undefined
>a : string
>a : string | undefined
>b : string | undefined
>c : string | undefined

Expand Down
14 changes: 14 additions & 0 deletions tests/cases/conformance/controlFlow/controlFlowNullishCoalesce.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// @strict: true

// assignments in shortcutting rhs
let a: number;
o ?? (a = 1);
a.toString();

// assignment flow
declare const o: { x: number } | undefined;
let x: { x: number } | boolean;
if (x = o ?? true) {
x;
}

1 comment on commit b030f73

@Kingwl
Copy link
Contributor

@Kingwl Kingwl commented on b030f73 Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh......Thanks

Please sign in to comment.