Incorrect narrowing of union to never after no-op switch in while loop

[RyanCavanaugh]

Bug Report

🔎 Search Terms

switch while union never

🕗 Version & Regression Information

Playground link with relevant code

💻 Code

declare function isNever(n: never): boolean;

function f() {
    let foo: "aaa" | "bbb" = "aaa";
    while (true) {
        switch (foo) {
            case "aaa":
        }

        if (foo === "aaa") {
            foo = "bbb";
        } else if (isNever(foo)) { // incorrect OK
            break;
        }
    }
}

🙁 Actual behavior

The code incorrectly checks as OK.

🙂 Expected behavior

The call to isNever should fail; on the second iteration of the loop isNever is invoked with "bbb"

Note that we don't see this behavior with the equivalent if:

declare function isNever(n: never): boolean;

function f() {
    let foo: "aaa" | "bbb" = "aaa";
    while (true) {
        //switch (foo) {
        //    case "aaa":
        //}
        if (foo === "aaa") { }

        if (foo === "aaa") {
            foo = "bbb";
        } else if (isNever(foo)) { // correct error
            break;
        }
    }
}

See also #47538

[gabritto]

This seems to have broken between 3.7 and 3.7.

[ahejlsberg]

I just took a quick look at this one. The issue is basically caused by a circularity: As we're computing the CFA type for foo immediately following the switch statement (i.e. in getTypeAtFlowBranchLabel for the flow label representing the flows through the switch statement), we rely on isExhaustiveSwitchStatement to determine if we should process the "bypass" flow. However, isExhaustiveSwitchStatement in turn needs to know the type of foo at the beginning of the switch statement, and since we're inside a loop we can't yet fully answer that. So we rely on the type that has been cached (using flowLoopTypes) and all we see is type "aaa". So we wrongly think that the switch statement is exhaustive and skip the bypass flow, which causes us to not see type "bbb".

Looks like it is possible to solve this by having isExhaustiveSwitchStatement initially cache a value of false that will be seen by any recursive invocations, and then computing the switch expression type from a "clean slate" in a manner similar to checkExpressionCached.

[ahejlsberg]

BTW, I think the original issue reported in #46475 is the same as this one. However, the reduced repros are not, though they are also caused by circularities.