Merge pull request #11675 from Microsoft/vladima/fix-11665

fix flow in finally blocks

Author: vladima

src/compiler/binder.ts

@@ -984,24 +984,44 @@ namespace ts {
         }
 
         function bindTryStatement(node: TryStatement): void {
-            const postFinallyLabel = createBranchLabel();
+            const preFinallyLabel = createBranchLabel();
             const preTryFlow = currentFlow;
             // TODO: Every statement in try block is potentially an exit point!
             bind(node.tryBlock);
-            addAntecedent(postFinallyLabel, currentFlow);
+            addAntecedent(preFinallyLabel, currentFlow);
+
+            const flowAfterTry = currentFlow;
+            let flowAfterCatch = unreachableFlow;
+
             if (node.catchClause) {
                 currentFlow = preTryFlow;
                 bind(node.catchClause);
-                addAntecedent(postFinallyLabel, currentFlow);
+                addAntecedent(preFinallyLabel, currentFlow);
+
+                flowAfterCatch = currentFlow;
             }
             if (node.finallyBlock) {
-                currentFlow = preTryFlow;
+                // in finally flow is combined from pre-try/flow from try/flow from catch
+                // pre-flow is necessary to make sure that finally is reachable even if finally flows in both try and finally blocks are unreachable
+                addAntecedent(preFinallyLabel, preTryFlow);
+                currentFlow = finishFlowLabel(preFinallyLabel);
                 bind(node.finallyBlock);
+                // if flow after finally is unreachable - keep it
+                // otherwise check if flows after try and after catch are unreachable 
+                // if yes - convert current flow to unreachable
+                // i.e.
+                // try { return "1" } finally { console.log(1); }
+                // console.log(2); // this line should be unreachable even if flow falls out of finally block
+                if (!(currentFlow.flags & FlowFlags.Unreachable)) {
+                    if ((flowAfterTry.flags & FlowFlags.Unreachable) && (flowAfterCatch.flags & FlowFlags.Unreachable)) {
+                        currentFlow = flowAfterTry === reportedUnreachableFlow || flowAfterCatch === reportedUnreachableFlow
+                            ? reportedUnreachableFlow
+                            : unreachableFlow;
+                    }
+                }
             }
-            // if try statement has finally block and flow after finally block is unreachable - keep it
-            // otherwise use whatever flow was accumulated at postFinallyLabel
-            if (!node.finallyBlock || !(currentFlow.flags & FlowFlags.Unreachable)) {
-                currentFlow = finishFlowLabel(postFinallyLabel);
+            else {
+                currentFlow = finishFlowLabel(preFinallyLabel);
             }
         }
 

tests/baselines/reference/flowInFinally1.js

@@ -0,0 +1,33 @@
+//// [flowInFinally1.ts]
+
+class A {
+  constructor() { }
+  method() { }
+}
+
+let a: A | null = null;
+
+try {
+  a = new A();
+} finally {
+  if (a) {
+    a.method();
+  }
+}
+
+//// [flowInFinally1.js]
+var A = (function () {
+    function A() {
+    }
+    A.prototype.method = function () { };
+    return A;
+}());
+var a = null;
+try {
+    a = new A();
+}
+finally {
+    if (a) {
+        a.method();
+    }
+}

tests/baselines/reference/flowInFinally1.symbols

@@ -0,0 +1,29 @@
+=== tests/cases/compiler/flowInFinally1.ts ===
+
+class A {
+>A : Symbol(A, Decl(flowInFinally1.ts, 0, 0))
+
+  constructor() { }
+  method() { }
+>method : Symbol(A.method, Decl(flowInFinally1.ts, 2, 19))
+}
+
+let a: A | null = null;
+>a : Symbol(a, Decl(flowInFinally1.ts, 6, 3))
+>A : Symbol(A, Decl(flowInFinally1.ts, 0, 0))
+
+try {
+  a = new A();
+>a : Symbol(a, Decl(flowInFinally1.ts, 6, 3))
+>A : Symbol(A, Decl(flowInFinally1.ts, 0, 0))
+
+} finally {
+  if (a) {
+>a : Symbol(a, Decl(flowInFinally1.ts, 6, 3))
+
+    a.method();
+>a.method : Symbol(A.method, Decl(flowInFinally1.ts, 2, 19))
+>a : Symbol(a, Decl(flowInFinally1.ts, 6, 3))
+>method : Symbol(A.method, Decl(flowInFinally1.ts, 2, 19))
+  }
+}

tests/baselines/reference/flowInFinally1.types

@@ -0,0 +1,34 @@
+=== tests/cases/compiler/flowInFinally1.ts ===
+
+class A {
+>A : A
+
+  constructor() { }
+  method() { }
+>method : () => void
+}
+
+let a: A | null = null;
+>a : A | null
+>A : A
+>null : null
+>null : null
+
+try {
+  a = new A();
+>a = new A() : A
+>a : A | null
+>new A() : A
+>A : typeof A
+
+} finally {
+  if (a) {
+>a : A | null
+
+    a.method();
+>a.method() : void
+>a.method : () => void
+>a : A
+>method : () => void
+  }
+}

tests/cases/compiler/flowInFinally1.ts

@@ -0,0 +1,16 @@
+// @strictNullChecks: true
+
+class A {
+  constructor() { }
+  method() { }
+}
+
+let a: A | null = null;
+
+try {
+  a = new A();
+} finally {
+  if (a) {
+    a.method();
+  }
+}