Track non-null unknown types in control flow analysis

[ahejlsberg]

With this PR we fix the following long standing issue:

declare function obj(x: object): void;

function foo(x: unknown) {
    if (typeof x === 'object' && x !== null) obj(x);  // Ok
    if (x !== null && typeof x === 'object') obj(x);  // Was error, now ok
}

Above, the order of the checks mattered because unknown is an "indivisible" non-union type and therefore a check for x !== null couldn't narrow x in the second if statement. We implemented a partial fix in #37507, but this only worked for truthy checks combined with typeof checks using the && operator in the same expression.

With this PR we remove the partial fix and instead introduce an internal nonNullUnknownType that is used in control flow analysis to represent an unknown type that is known to be non-null. The non-null unknown type results when the unknown type is subjected to a truthiness check or an x !== null check, and when the non-null unknown is subsequently subjected to a typeof x === 'object' check, we narrow to object instead of object | null. Because we rely on control flow analysis, this solution properly reflects the effects of non-null checks in separate expressions or statements:

function bar(x: unknown) {
    if (!x) {
        return;
    }
    if (typeof x === 'object') {
        obj(x);  // Was error, now ok
    }
}

Fixes #28131.

[andrewbranch]

What’s an example where this produces nonNullUnknownType? Would nonNullUnknownType itself have to be one of the union constituents?

[ahejlsberg]

Yes, it'll produce nonNullUnknownType when there are no regular unknownType instances in the set (which implies all are nonNullUnknownType instances) and the set contains no TypeFlags.Null types. Effectively, a non-null unknown type survives only if there are no regular unknown types and no null types.

[andrewbranch]

Can this not just be handled by getTypeWithFacts?

[ExE-Boss]

Well, getTypeWithFacts(…) is used in many more places, and nonNullUnknownType should never escape control flow analysis: https://github.com/microsoft/TypeScript/blob/7e231a2ebfce3692663fe2583a19a8cb0c59e457/src/compiler/checker.ts#L23118-L23119

So probably not?

[DanielRosenwasser]

Posting as a github1s link https://github1s.com/microsoft/TypeScript/blob/7e231a2ebfce3692663fe2583a19a8cb0c59e457/src/compiler/checker.ts#L23118-L23119

[treybrisbane]

This reminded me of #29317! 😮

Does special-casing this mean the team is still undecided on Negated Types?

[treybrisbane]

In any case, thanks for this fix! I have this pop up every so often, and it'll be nice to see it finally solved. 😀

[DanielRosenwasser]

It's already pretty hard to understand what these are for. I'd encourage us to start documenting what the use-cases for each of these are. For example.

Suggested change

	const nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown");
	/**
	* An `unknown` type that is used purely for narrowing.
	* Used to record that a `x !== null` check has occurred to specially handle `typeof x === "object"`.
	*/
	const nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown");