diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 04dc4011a1591..41bf7b58c187b 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1045,7 +1045,35 @@ namespace ts { if (node.finallyBlock) { // 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); + + // also for finally blocks we inject two extra edges into the flow graph. + // first -> edge that connects pre-try flow with the label at the beginning of the finally block, it has lock associated with it + // second -> edge that represents post-finally flow. + // these edges are used in following scenario: + // let a; (1) + // try { a = someOperation(); (2)} + // finally { (3) console.log(a) } (4) + // (5) a + + // flow graph for this case looks roughly like this (arrows show ): + // (1-pre-try-flow) <--.. <-- (2-post-try-flow) + // ^ ^ + // |*****(3-pre-finally-label) -----| + // ^ + // |-- ... <-- (4-post-finally-label) <--- (5) + // In case when we walk the flow starting from inside the finally block we want to take edge '*****' into account + // since it ensures that finally is always reachable. However when we start outside the finally block and go through label (5) + // then edge '*****' should be discarded because label 4 is only reachable if post-finally label-4 is reachable + // Simply speaking code inside finally block is treated as reachable as pre-try-flow + // since we conservatively assume that any line in try block can throw or return in which case we'll enter finally. + // However code after finally is reachable only if control flow was not abrupted in try/catch or finally blocks - it should be composed from + // final flows of these blocks without taking pre-try flow into account. + // + // extra edges that we inject allows to control this behavior + // if when walking the flow we step on post-finally edge - we can mark matching pre-finally edge as locked so it will be skipped. + const preFinallyFlow: PreFinallyFlow = { flags: FlowFlags.PreFinally, antecedent: preTryFlow, lock: {} }; + addAntecedent(preFinallyLabel, preFinallyFlow); + currentFlow = finishFlowLabel(preFinallyLabel); bind(node.finallyBlock); // if flow after finally is unreachable - keep it @@ -1061,6 +1089,11 @@ namespace ts { : unreachableFlow; } } + if (!(currentFlow.flags & FlowFlags.Unreachable)) { + const afterFinallyFlow: AfterFinallyFlow = { flags: FlowFlags.AfterFinally, antecedent: currentFlow }; + preFinallyFlow.lock = afterFinallyFlow; + currentFlow = afterFinallyFlow; + } } else { currentFlow = finishFlowLabel(preFinallyLabel); diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b5306771dd7f7..401fd9e480423 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9813,7 +9813,19 @@ namespace ts { } } let type: FlowType; - if (flow.flags & FlowFlags.Assignment) { + if (flow.flags & FlowFlags.AfterFinally) { + // block flow edge: finally -> pre-try (for larger explanation check comment in binder.ts - bindTryStatement + (flow).locked = true; + type = getTypeAtFlowNode((flow).antecedent); + (flow).locked = false; + } + else if (flow.flags & FlowFlags.PreFinally) { + // locked pre-finally flows are filtered out in getTypeAtFlowBranchLabel + // so here just redirect to antecedent + flow = (flow).antecedent; + continue; + } + else if (flow.flags & FlowFlags.Assignment) { type = getTypeAtFlowAssignment(flow); if (!type) { flow = (flow).antecedent; @@ -9969,6 +9981,12 @@ namespace ts { let subtypeReduction = false; let seenIncomplete = false; for (const antecedent of flow.antecedents) { + if (antecedent.flags & FlowFlags.PreFinally && (antecedent).lock.locked) { + // if flow correspond to branch from pre-try to finally and this branch is locked - this means that + // we initially have started following the flow outside the finally block. + // in this case we should ignore this branch. + continue; + } const flowType = getTypeAtFlowNode(antecedent); const type = getTypeFromFlowType(flowType); // If the type at a particular antecedent path is the declared type and the diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8660e6138cdad..075935e2740a3 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2069,10 +2069,25 @@ ArrayMutation = 1 << 8, // Potential array mutation Referenced = 1 << 9, // Referenced as antecedent once Shared = 1 << 10, // Referenced as antecedent more than once + PreFinally = 1 << 11, // Injected edge that links pre-finally label and pre-try flow + AfterFinally = 1 << 12, // Injected edge that links post-finally flow with the rest of the graph Label = BranchLabel | LoopLabel, Condition = TrueCondition | FalseCondition } + export interface FlowLock { + locked?: boolean; + } + + export interface AfterFinallyFlow extends FlowNode, FlowLock { + antecedent: FlowNode; + } + + export interface PreFinallyFlow extends FlowNode { + antecedent: FlowNode; + lock: FlowLock; + } + export interface FlowNode { flags: FlowFlags; id?: number; // Node id used by flow type cache in checker diff --git a/tests/baselines/reference/flowAfterFinally1.js b/tests/baselines/reference/flowAfterFinally1.js new file mode 100644 index 0000000000000..93f3425d50625 --- /dev/null +++ b/tests/baselines/reference/flowAfterFinally1.js @@ -0,0 +1,25 @@ +//// [flowAfterFinally1.ts] +declare function openFile(): void +declare function closeFile(): void +declare function someOperation(): {} + +var result: {} +openFile() +try { + result = someOperation() +} finally { + closeFile() +} + +result // should not error here + +//// [flowAfterFinally1.js] +var result; +openFile(); +try { + result = someOperation(); +} +finally { + closeFile(); +} +result; // should not error here diff --git a/tests/baselines/reference/flowAfterFinally1.symbols b/tests/baselines/reference/flowAfterFinally1.symbols new file mode 100644 index 0000000000000..ad0ffbeb5cfd0 --- /dev/null +++ b/tests/baselines/reference/flowAfterFinally1.symbols @@ -0,0 +1,29 @@ +=== tests/cases/compiler/flowAfterFinally1.ts === +declare function openFile(): void +>openFile : Symbol(openFile, Decl(flowAfterFinally1.ts, 0, 0)) + +declare function closeFile(): void +>closeFile : Symbol(closeFile, Decl(flowAfterFinally1.ts, 0, 33)) + +declare function someOperation(): {} +>someOperation : Symbol(someOperation, Decl(flowAfterFinally1.ts, 1, 34)) + +var result: {} +>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3)) + +openFile() +>openFile : Symbol(openFile, Decl(flowAfterFinally1.ts, 0, 0)) + +try { + result = someOperation() +>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3)) +>someOperation : Symbol(someOperation, Decl(flowAfterFinally1.ts, 1, 34)) + +} finally { + closeFile() +>closeFile : Symbol(closeFile, Decl(flowAfterFinally1.ts, 0, 33)) +} + +result // should not error here +>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3)) + diff --git a/tests/baselines/reference/flowAfterFinally1.types b/tests/baselines/reference/flowAfterFinally1.types new file mode 100644 index 0000000000000..93aae4764b689 --- /dev/null +++ b/tests/baselines/reference/flowAfterFinally1.types @@ -0,0 +1,33 @@ +=== tests/cases/compiler/flowAfterFinally1.ts === +declare function openFile(): void +>openFile : () => void + +declare function closeFile(): void +>closeFile : () => void + +declare function someOperation(): {} +>someOperation : () => {} + +var result: {} +>result : {} + +openFile() +>openFile() : void +>openFile : () => void + +try { + result = someOperation() +>result = someOperation() : {} +>result : {} +>someOperation() : {} +>someOperation : () => {} + +} finally { + closeFile() +>closeFile() : void +>closeFile : () => void +} + +result // should not error here +>result : {} + diff --git a/tests/cases/compiler/flowAfterFinally1.ts b/tests/cases/compiler/flowAfterFinally1.ts new file mode 100644 index 0000000000000..74df6a1be2e56 --- /dev/null +++ b/tests/cases/compiler/flowAfterFinally1.ts @@ -0,0 +1,14 @@ +// @strictNullChecks: true +declare function openFile(): void +declare function closeFile(): void +declare function someOperation(): {} + +var result: {} +openFile() +try { + result = someOperation() +} finally { + closeFile() +} + +result // should not error here \ No newline at end of file