Track non-null unknown types in control flow analysis#45575
Conversation
| return includes & TypeFlags.Any ? includes & TypeFlags.IncludesWildcard ? wildcardType : anyType : unknownType; | ||
| return includes & TypeFlags.Any ? | ||
| includes & TypeFlags.IncludesWildcard ? wildcardType : anyType : | ||
| includes & TypeFlags.Null || containsType(typeSet, unknownType) ? unknownType : nonNullUnknownType; |
There was a problem hiding this comment.
What’s an example where this produces nonNullUnknownType? Would nonNullUnknownType itself have to be one of the union constituents?
There was a problem hiding this comment.
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.
| assumeTrue ? TypeFacts.EQNull : TypeFacts.NENull : | ||
| assumeTrue ? TypeFacts.EQUndefined : TypeFacts.NEUndefined; | ||
| return getTypeWithFacts(type, facts); | ||
| return type.flags & TypeFlags.Unknown && facts & (TypeFacts.NENull | TypeFacts.NEUndefinedOrNull) ? nonNullUnknownType : getTypeWithFacts(type, facts); |
There was a problem hiding this comment.
Can this not just be handled by getTypeWithFacts?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
|
This reminded me of #29317! 😮 Does special-casing this mean the team is still undecided on Negated Types? |
|
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. 😀 |
| const nonInferrableAnyType = createIntrinsicType(TypeFlags.Any, "any", ObjectFlags.ContainsWideningType); | ||
| const intrinsicMarkerType = createIntrinsicType(TypeFlags.Any, "intrinsic"); | ||
| const unknownType = createIntrinsicType(TypeFlags.Unknown, "unknown"); | ||
| const nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown"); |
There was a problem hiding this comment.
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.
| 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"); |
With this PR we fix the following long standing issue:
Above, the order of the checks mattered because
unknownis an "indivisible" non-union type and therefore a check forx !== nullcouldn't narrowxin the secondifstatement. We implemented a partial fix in #37507, but this only worked for truthy checks combined withtypeofchecks using the&&operator in the same expression.With this PR we remove the partial fix and instead introduce an internal
nonNullUnknownTypethat is used in control flow analysis to represent anunknowntype that is known to be non-null. The non-null unknown type results when theunknowntype is subjected to a truthiness check or anx !== nullcheck, and when the non-null unknown is subsequently subjected to atypeof x === 'object'check, we narrow toobjectinstead ofobject | null. Because we rely on control flow analysis, this solution properly reflects the effects of non-null checks in separate expressions or statements:Fixes #28131.