Improve logic that chooses co- vs. contra-variant return type inferences #59709
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26982,18 +26982,38 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
| const inferredCovariantType = inference.candidates ? getCovariantInference(inference, context.signature) : undefined; | ||
| const inferredContravariantType = inference.contraCandidates ? getContravariantInference(inference) : undefined; | ||
| if (inferredCovariantType || inferredContravariantType) { | ||
| // ideally all inferences would be tried out in a ranked order but that's too costly | ||
| // a local heuristic (created based on empirical testing) is used to pick the best candidate here: | ||
| // 1. covariant `never` and `any` are ignored as they are assignable to most types and choosing them would spoil the overall result | ||
| // 2. covariant inference has to be compatible with at least some contravariant inferences | ||
| // 3. the variable being inferred isn't referred to by another variable's constraint directly | ||
| // or if it is then the all of the other's covariant candidates must be compatible with the inferred covariant type | ||
| // | ||
| // to determine compatibility between the types assignability is used to allow for own covariant inference to be chosen | ||
| // when the contravariant inference has optional properties that are not present in the covariant inference | ||
| // this helps in situations like this when the covariant source can be used to call the source of the contravariant inference: | ||
| // | ||
| // const validate = (_: { query?: unknown; body?: unknown }) => {}; | ||
| // route({ | ||
| // pre: validate, | ||
| // schema: { | ||
| // query: '' | ||
| // }, | ||
| // }); | ||
| // | ||
| // however, for return types inferences subtyping is used. In those situations, the contravariant inference often comes from the argument of mapping function | ||
| // and the outer covariant requirement can still be satisfied by the return position of the return type when it's a subtype of that required return type. | ||
| // this helps to provide better contextual parameter types in scenarios like this: | ||
| // | ||
| // declare const obs: Observable<{ a?: string; b?: number }>; | ||
| // const test = (): Observable<{ a?: string }> => obs.pipe(tap((arg) => {})); | ||
|
Comment on lines
+27004
to
+27009
There was a problem hiding this comment. This part of the comment tries to explain the idea behind the fix. An alternative idea I had was to experiment with a new inference priority to differentiate between covariant inferences made from return positions. However, that's a bigger experiment and what I propose here is much safer at this stage. It's essentially reverting part of #57909 to 5.5 behavior. So it shouldn't behave worse in any case. But also given that return positions could themselves be within nested functions, I don't have a good intuition on how it could play out. At the moment, there is no concept of "depth" in the inference and the core of this idea conceptually is that a return position might not be more important than a contravariant inference made from the (any?) containing function. This would have some potential to solve Wes' concern raised here related to inferring from |
||
| const compareTypes = inference.priority! & InferencePriority.ReturnType ? isTypeSubtypeOf : isTypeAssignableTo; | ||
| const preferCovariantType = inferredCovariantType && (!inferredContravariantType || | ||
| !(inferredCovariantType.flags & (TypeFlags.Never | TypeFlags.Any)) && | ||
| some(inference.contraCandidates, t => compareTypes(inferredCovariantType, t)) && | ||
| every(context.inferences, other => | ||
| other !== inference && getConstraintOfTypeParameter(other.typeParameter) !== inference.typeParameter || | ||
| every(other.candidates, t => compareTypes(t, inferredCovariantType)))); | ||
| inferredType = preferCovariantType ? inferredCovariantType : inferredContravariantType; | ||
| fallbackType = preferCovariantType ? inferredContravariantType : inferredCovariantType; | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| //// [tests/cases/compiler/coAndContraVariantInferences10.ts] //// | ||
|
|
||
| === coAndContraVariantInferences10.ts === | ||
| // based on https://github.com/microsoft/TypeScript/issues/59656 | ||
|
|
||
| interface Observable<T> { | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 2, 21)) | ||
|
|
||
| pipe: <A>(op: (source: Observable<T>) => Observable<A>) => Observable<A>; | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >A : Symbol(A, Decl(coAndContraVariantInferences10.ts, 3, 9)) | ||
| >op : Symbol(op, Decl(coAndContraVariantInferences10.ts, 3, 12)) | ||
| >source : Symbol(source, Decl(coAndContraVariantInferences10.ts, 3, 17)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 2, 21)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >A : Symbol(A, Decl(coAndContraVariantInferences10.ts, 3, 9)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >A : Symbol(A, Decl(coAndContraVariantInferences10.ts, 3, 9)) | ||
|
|
||
| _v: T; | ||
| >_v : Symbol(Observable._v, Decl(coAndContraVariantInferences10.ts, 3, 75)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 2, 21)) | ||
| } | ||
| declare function tap<T>( | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 6, 21)) | ||
|
|
||
| next: (value: T) => void, | ||
| >next : Symbol(next, Decl(coAndContraVariantInferences10.ts, 6, 24)) | ||
| >value : Symbol(value, Decl(coAndContraVariantInferences10.ts, 7, 9)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 6, 21)) | ||
|
|
||
| ): (source: Observable<T>) => Observable<T>; | ||
| >source : Symbol(source, Decl(coAndContraVariantInferences10.ts, 8, 4)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 6, 21)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >T : Symbol(T, Decl(coAndContraVariantInferences10.ts, 6, 21)) | ||
|
|
||
| declare const obs1: Observable<{ | ||
| >obs1 : Symbol(obs1, Decl(coAndContraVariantInferences10.ts, 10, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop?: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 10, 32)) | ||
|
|
||
| }>; | ||
| function test1(): Observable<{}> { | ||
| >test1 : Symbol(test1, Decl(coAndContraVariantInferences10.ts, 12, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| return obs1.pipe(tap((arg) => {})); | ||
| >obs1.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs1 : Symbol(obs1, Decl(coAndContraVariantInferences10.ts, 10, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 14, 24)) | ||
| } | ||
|
|
||
| declare const obs2: Observable<{ | ||
| >obs2 : Symbol(obs2, Decl(coAndContraVariantInferences10.ts, 17, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 17, 32)) | ||
|
|
||
| }>; | ||
| function test2(): Observable<{}> { | ||
| >test2 : Symbol(test2, Decl(coAndContraVariantInferences10.ts, 19, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| return obs2.pipe(tap((arg) => {})); | ||
| >obs2.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs2 : Symbol(obs2, Decl(coAndContraVariantInferences10.ts, 17, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 21, 24)) | ||
| } | ||
|
|
||
| declare const obs3: Observable<{ | ||
| >obs3 : Symbol(obs3, Decl(coAndContraVariantInferences10.ts, 24, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 24, 32)) | ||
|
|
||
| prop2?: number; | ||
| >prop2 : Symbol(prop2, Decl(coAndContraVariantInferences10.ts, 25, 15)) | ||
|
|
||
| }>; | ||
| function test3(): Observable<{}> { | ||
| >test3 : Symbol(test3, Decl(coAndContraVariantInferences10.ts, 27, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| return obs3.pipe(tap((arg) => {})); | ||
| >obs3.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs3 : Symbol(obs3, Decl(coAndContraVariantInferences10.ts, 24, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 29, 24)) | ||
| } | ||
|
|
||
| declare const obs4: Observable<{ | ||
| >obs4 : Symbol(obs4, Decl(coAndContraVariantInferences10.ts, 32, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop?: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 32, 32)) | ||
|
|
||
| prop2?: number; | ||
| >prop2 : Symbol(prop2, Decl(coAndContraVariantInferences10.ts, 33, 16)) | ||
|
|
||
| }>; | ||
| function test4(): Observable<{ prop?: string }> { | ||
| >test4 : Symbol(test4, Decl(coAndContraVariantInferences10.ts, 35, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 36, 30)) | ||
|
|
||
| return obs4.pipe(tap((arg) => {})); | ||
| >obs4.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs4 : Symbol(obs4, Decl(coAndContraVariantInferences10.ts, 32, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 37, 24)) | ||
| } | ||
|
|
||
| declare const obs5: Observable<{ | ||
| >obs5 : Symbol(obs5, Decl(coAndContraVariantInferences10.ts, 40, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 40, 32)) | ||
|
|
||
| prop2?: number; | ||
| >prop2 : Symbol(prop2, Decl(coAndContraVariantInferences10.ts, 41, 15)) | ||
|
|
||
| }>; | ||
| function test5(): Observable<{ prop: string }> { | ||
| >test5 : Symbol(test5, Decl(coAndContraVariantInferences10.ts, 43, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 44, 30)) | ||
|
|
||
| return obs5.pipe(tap((arg) => {})); | ||
| >obs5.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs5 : Symbol(obs5, Decl(coAndContraVariantInferences10.ts, 40, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 45, 24)) | ||
| } | ||
|
|
||
| declare const obs6: Observable<{ | ||
| >obs6 : Symbol(obs6, Decl(coAndContraVariantInferences10.ts, 48, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 48, 32)) | ||
|
|
||
| prop2?: number; | ||
| >prop2 : Symbol(prop2, Decl(coAndContraVariantInferences10.ts, 49, 15)) | ||
|
|
||
| }>; | ||
| function test6(): Observable<{ prop2?: number }> { | ||
| >test6 : Symbol(test6, Decl(coAndContraVariantInferences10.ts, 51, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
| >prop2 : Symbol(prop2, Decl(coAndContraVariantInferences10.ts, 52, 30)) | ||
|
|
||
| return obs6.pipe(tap((arg) => {})); | ||
| >obs6.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs6 : Symbol(obs6, Decl(coAndContraVariantInferences10.ts, 48, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 53, 24)) | ||
| } | ||
|
|
||
| declare const obs7: Observable<{ | ||
| >obs7 : Symbol(obs7, Decl(coAndContraVariantInferences10.ts, 56, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop?: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 56, 32)) | ||
|
|
||
| }>; | ||
| function test7(): Observable<any> { | ||
| >test7 : Symbol(test7, Decl(coAndContraVariantInferences10.ts, 58, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| return obs7.pipe(tap((arg) => {})); | ||
| >obs7.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs7 : Symbol(obs7, Decl(coAndContraVariantInferences10.ts, 56, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 60, 24)) | ||
| } | ||
|
|
||
| declare const obs8: Observable<{ | ||
| >obs8 : Symbol(obs8, Decl(coAndContraVariantInferences10.ts, 63, 13)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| prop?: string; | ||
| >prop : Symbol(prop, Decl(coAndContraVariantInferences10.ts, 63, 32)) | ||
|
|
||
| }>; | ||
| function test8(): Observable<unknown> { | ||
| >test8 : Symbol(test8, Decl(coAndContraVariantInferences10.ts, 65, 3)) | ||
| >Observable : Symbol(Observable, Decl(coAndContraVariantInferences10.ts, 0, 0)) | ||
|
|
||
| return obs8.pipe(tap((arg) => {})); | ||
| >obs8.pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >obs8 : Symbol(obs8, Decl(coAndContraVariantInferences10.ts, 63, 13)) | ||
| >pipe : Symbol(Observable.pipe, Decl(coAndContraVariantInferences10.ts, 2, 25)) | ||
| >tap : Symbol(tap, Decl(coAndContraVariantInferences10.ts, 5, 1)) | ||
| >arg : Symbol(arg, Decl(coAndContraVariantInferences10.ts, 67, 24)) | ||
| } | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewritten this comment based on the excellent comment by @weswigham here. I tried to be concise though as this is already pretty long and Wes' comment was even longer 😉 Feel free to suggest any improvement to this to make it better. I write better in TS than in English.