typechecking of return types broken with try/catch when strictNullChecks and noImplicitAny are enabled

[zrait]

Bug Report

🔎 Search Terms

try/catch, exception handling, strict mode

🕗 Version & Regression Information

Broken in 4.3.5, still broken in nightly

⏯ Playground Link

Playground link with strict mode

Disable --noImplicitAny and --strict (but leave --strictNullChecks) and typechecker will correctly note that val may have type null

💻 Code

declare function bar(): any;

async function foo(): Promise<string> {
    let val = null;
    try {
        val = bar();
    } catch (e) {}
    return val;
}

🙁 Actual behavior

Compile with --strict and observe no errors
Compile simply with --strictNullChecks and typechecker will correctly note that val may have type null and does not conform to the return type

🙂 Expected behavior

typechecker should correctly determine that val can have type null and does not conform to the return type

[DanielRosenwasser]

When we return, we look at all possible paths of control flow and form a union. We see we have null in one branch and any in the other. The any spoils the pot and reduces that union to any, which in turn is assignable with any other type. I think it's unfortunate, but it's a design limitation.

[fatcerberus]

How come turning off noImplicitAny makes the error show up?

[zrait]

I understand that any contamination issue @DanielRosenwasser, but the fact that disabling noImplicitAny caused the typechecker to flag this as an error seemed unusual to me.

[DanielRosenwasser]

How come turning off noImplicitAny makes the error show up?

When TypeScript originally introduced the noImplicitAny flag, we would widen that null to an any implicitly, and it was an error; however, in TypeScript 2.1 (#11263), we started using control-flow analysis to track the types of null/undefined-initialized (or uninitialized) variables.

Here, the any poisons the control-flow analyzed type for later locations. The only solution that I can think of is to ensure that val has a type annotation (i.e. string | null).

the fact that disabling noImplicitAny caused the typechecker to flag this as an error seemed unusual to me.

Yeah, I agree, this is not desirable. Personally, I try to steer clear of auto-typed variables. I often need to include a type anyway since they get initialized in complex ways with try/catch and loops.

The only thing we could probably do now is to provide a flag for forbidding assignments of any to auto-typed variables.

[fatcerberus]

What surprises me most is that this doesn't get flagged as a "not definitely assigned in all branches" error. It seems like that should happen regardless, even in the presence of any?

[DanielRosenwasser]

So we do if you provide an explicit type for your variable. Check it out here.

We don't in the original post, because the analysis to decide if something is uninitialized checks whether the original type was declared without undefined. We add an undefined if there's no initializer (and no ! on the declaration), and see if CFA was able to get rid of it.

But here, there's no explicit type, so you get no Variable 'val' is used before being assigned. error, and you have an any in one branch, so there's no Object is possibly 'undefined'. error.

[DanielRosenwasser]

One thing we could possibly do is create a special variant of any and a "missing" type for variables, and say that when any and a missing union together, you get that special any type. I'm not sure how much value we'd get from that, but maybe we can talk it over in a backlog discussion.

[fatcerberus]

Ah, I see, I was thinking of this error message (see #11263):

Variable 'x' implicitly has type 'any' in some locations where its type cannot be determined.

but now that I've done some more digging, it looks like that only applies in cases related to #9998 (i.e. closing over the auto-typed variable). That said, what is the mechanism used to produce that error, if it's different from this case?