Widen inference candidates for error reporting #15221
Conversation
They are widened the same way for checking for best common super type and for reporting the error now. Previously error reporting forgot to widen and hit an assert.
Add a test to show that error reporting for type inference widens the same way that type inference proper does. Previously it asserted. Also update other baselines with the widened type.
Previously it discarded and re-added *top-level* nulls and undefineds in the inference candidate list. Which doesn't make much sense. Now it discards and re-adds nulls and undefineds that are unioned with types in the inference candidate list. This makes the new test pass now instead of failing.
|
This PR now fixes the bad inference that led to the error in the first place. |
Previous commit mistakenly only updated baselines
src/compiler/checker.ts
Outdated
| @@ -10338,17 +10345,8 @@ namespace ts { | |||
| if (!inferredType) { | |||
| const inferences = getInferenceCandidates(context, index); | |||
| if (inferences.length) { | |||
| // We widen inferred literal types if | |||
There was a problem hiding this comment.
This code moves into getInferenceCandidates so that both type inference (this section) and type inference error reporting (the other call to getInferenceCandidates) widen the same way.
|
Would you mind merging master into this branch to get things updated, then I'll take a look. |
|
I forgot how much has changed in inference in the last few weeks! It's merged now but it needed some rewriting to work with InferenceInfo. |
Add getOptionalType to support the common case of just adding undefined.
|
Travis failed because |
src/compiler/checker.ts
Outdated
| const supertype = getSupertypeOrUnion(primaryTypes); | ||
| return supertype && getNullableType(supertype, getFalsyFlagsOfTypes(types) & TypeFlags.Nullable); | ||
| const supertype = getSupertypeOrUnion(map(types, getNonNullableType)); | ||
| return supertype && getNullableType(supertype, getFalsyFlagsOfTypes(types) & (TypeFlags.Nullable | TypeFlags.Void)); |
There was a problem hiding this comment.
Here's what caused the addition TypeFlags.Void to getFalsyFlagsOfTypes. When types contains void, the old filter(types ... code did not remove it because void only has TypeFlags.Void. But map(types, getNonNullableType) relies on getTypeFacts, which treats void and undefined exactly the same. So it removes void and undefined both. This causes the following bug:
declare function f<T>(cb: () => T | undefined): T;
f(() => { });
~~~~~~~~~
Argument of type '() => void' is not assignable to parameter of type '() => undefined'.I would like getNonNullableType not to remove void, but I'm not sure how to do that without changing getTypeFacts, which I am not at all comfortable doing.
This changes the new test's baseline. Note that the new test exists to show that the compiler no longer crashes, so errors are fine.
|
Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it. |
Both type inference and type inference error reporting widen candidates the same way now.
Previously error reporting hit an assert when checking for best common supertype: it hit a condition that indicated there there was a best common supertype, even though the error reporting should only be called when there is no best common supertype.
Both type inference and error reporting call
getInferenceCandidates, so I just moved the additional widening code from type inference togetInferenceCandidates.Fixes #15116