Skip to content

Improve logic that chooses co- vs. contra-variant inferences #57909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 17, 2024
Merged

Improve logic that chooses co- vs. contra-variant inferences #57909

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
80491eb
Improve logic that chooses co- vs. contra-variant inferences
Andarist Mar 22, 2024
f78cea6
rename test file
Andarist Mar 23, 2024
237aba6
ignore covariant any inferences when contravariant candidates are ava…
Andarist Mar 23, 2024
6688f3c
Merge remote-tracking branch 'origin/main' into tweak-co-contra-infer…
Andarist Mar 25, 2024
03d0eb1
Merge remote-tracking branch 'origin/main' into tweak-co-contra-infer…
Andarist May 1, 2024
810943a
Merge remote-tracking branch 'origin/main' into tweak-co-contra-infer…
Andarist May 8, 2024
b715e32
add an extra test case
Andarist May 8, 2024
2226e0b
tweak comment
Andarist Jun 14, 2024
1c3f6c9
Merge remote-tracking branch 'origin/main' into tweak-co-contra-infer…
Andarist Jun 14, 2024
169d720
update baseline
Andarist Jun 14, 2024
91d69a1
update a baseline
Andarist Jun 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26465,10 +26465,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// and has inferences that would conflict. Otherwise, we prefer the contra-variant inference.
const preferCovariantType = inferredCovariantType && (!inferredContravariantType ||
!(inferredCovariantType.flags & TypeFlags.Never) &&
some(inference.contraCandidates, t => isTypeSubtypeOf(inferredCovariantType, t)) &&
some(inference.contraCandidates, t => isTypeAssignableTo(inferredCovariantType, t)) &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is not the correct change then at the very least a test case should be proposed that shows how the isTypeSubtypeOf is better.

When this check was originally introduced by @ahejlsberg here it was states that:

Furthermore, knowing that an error will result when the co-variant inference is not a subtype of the contra-variant inference, we now prefer the contra-variant inference because it is likely to have come from an explicit type annotation on a function. This improves our error reporting.

An alternative idea to fix this example would be to keep coAndContraRelationCheck on inferenceContext. From what I understand, the subtypeRelation in this context is mainly used when dealing with multiple overloads - a single signature case uses assignableRelation with chooseOverload. So perhaps the used relation should determine what is being used here.

every(context.inferences, other =>
other !== inference && getConstraintOfTypeParameter(other.typeParameter) !== inference.typeParameter ||
every(other.candidates, t => isTypeSubtypeOf(t, inferredCovariantType))));
every(other.candidates, t => isTypeAssignableTo(t, inferredCovariantType))));
inferredType = preferCovariantType ? inferredCovariantType : inferredContravariantType;
fallbackType = preferCovariantType ? inferredContravariantType : inferredCovariantType;
}
Expand Down
77 changes: 77 additions & 0 deletions tests/baselines/reference/coAndContraVariantInferences6.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//// [tests/cases/compiler/coAndContraVariantInferences6.ts] ////

=== coAndContraVariantInferences6.ts ===
type Request<TSchema extends Schema> = {
>Request : Symbol(Request, Decl(coAndContraVariantInferences6.ts, 0, 0))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 0, 13))
>Schema : Symbol(Schema, Decl(coAndContraVariantInferences6.ts, 2, 2))

query: TSchema["query"];
>query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 0, 40))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 0, 13))

};

type Schema = { query?: unknown; body?: unknown };
>Schema : Symbol(Schema, Decl(coAndContraVariantInferences6.ts, 2, 2))
>query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 4, 15))
>body : Symbol(body, Decl(coAndContraVariantInferences6.ts, 4, 32))

declare function route<TSchema extends Schema>(obj: {
>route : Symbol(route, Decl(coAndContraVariantInferences6.ts, 4, 50))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 6, 23))
>Schema : Symbol(Schema, Decl(coAndContraVariantInferences6.ts, 2, 2))
>obj : Symbol(obj, Decl(coAndContraVariantInferences6.ts, 6, 47))

pre: (a: TSchema) => void;
>pre : Symbol(pre, Decl(coAndContraVariantInferences6.ts, 6, 53))
>a : Symbol(a, Decl(coAndContraVariantInferences6.ts, 7, 8))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 6, 23))

schema: TSchema;
>schema : Symbol(schema, Decl(coAndContraVariantInferences6.ts, 7, 28))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 6, 23))

handle: (req: Request<TSchema>) => void;
>handle : Symbol(handle, Decl(coAndContraVariantInferences6.ts, 8, 18))
>req : Symbol(req, Decl(coAndContraVariantInferences6.ts, 9, 11))
>Request : Symbol(Request, Decl(coAndContraVariantInferences6.ts, 0, 0))
>TSchema : Symbol(TSchema, Decl(coAndContraVariantInferences6.ts, 6, 23))

}): void;

const validate = (_: { query?: unknown; body?: unknown }) => {};
>validate : Symbol(validate, Decl(coAndContraVariantInferences6.ts, 12, 5))
>_ : Symbol(_, Decl(coAndContraVariantInferences6.ts, 12, 18))
>query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 12, 22))
>body : Symbol(body, Decl(coAndContraVariantInferences6.ts, 12, 39))

route({
>route : Symbol(route, Decl(coAndContraVariantInferences6.ts, 4, 50))

pre: validate,
>pre : Symbol(pre, Decl(coAndContraVariantInferences6.ts, 14, 7))
>validate : Symbol(validate, Decl(coAndContraVariantInferences6.ts, 12, 5))

schema: {
>schema : Symbol(schema, Decl(coAndContraVariantInferences6.ts, 15, 16))

query: "",
>query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 16, 11))

},
handle: (req) => {
>handle : Symbol(handle, Decl(coAndContraVariantInferences6.ts, 18, 4))
>req : Symbol(req, Decl(coAndContraVariantInferences6.ts, 19, 11))

const test: string = req.query;
>test : Symbol(test, Decl(coAndContraVariantInferences6.ts, 20, 9))
>req.query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 0, 40))
>req : Symbol(req, Decl(coAndContraVariantInferences6.ts, 19, 11))
>query : Symbol(query, Decl(coAndContraVariantInferences6.ts, 0, 40))

},
});

export {};

74 changes: 74 additions & 0 deletions tests/baselines/reference/coAndContraVariantInferences6.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//// [tests/cases/compiler/coAndContraVariantInferences6.ts] ////

=== coAndContraVariantInferences6.ts ===
type Request<TSchema extends Schema> = {
>Request : Request<TSchema>

query: TSchema["query"];
>query : TSchema["query"]

};

type Schema = { query?: unknown; body?: unknown };
>Schema : { query?: unknown; body?: unknown; }
>query : unknown
>body : unknown

declare function route<TSchema extends Schema>(obj: {
>route : <TSchema extends Schema>(obj: { pre: (a: TSchema) => void; schema: TSchema; handle: (req: Request<TSchema>) => void;}) => void
>obj : { pre: (a: TSchema) => void; schema: TSchema; handle: (req: Request<TSchema>) => void; }

pre: (a: TSchema) => void;
>pre : (a: TSchema) => void
>a : TSchema

schema: TSchema;
>schema : TSchema

handle: (req: Request<TSchema>) => void;
>handle : (req: Request<TSchema>) => void
>req : Request<TSchema>

}): void;

const validate = (_: { query?: unknown; body?: unknown }) => {};
>validate : (_: { query?: unknown; body?: unknown;}) => void
>(_: { query?: unknown; body?: unknown }) => {} : (_: { query?: unknown; body?: unknown;}) => void
>_ : { query?: unknown; body?: unknown; }
>query : unknown
>body : unknown

route({
>route({ pre: validate, schema: { query: "", }, handle: (req) => { const test: string = req.query; },}) : void
>route : <TSchema extends Schema>(obj: { pre: (a: TSchema) => void; schema: TSchema; handle: (req: Request<TSchema>) => void; }) => void
>{ pre: validate, schema: { query: "", }, handle: (req) => { const test: string = req.query; },} : { pre: (_: { query?: unknown; body?: unknown; }) => void; schema: { query: string; }; handle: (req: Request<{ query: string; }>) => void; }

pre: validate,
>pre : (_: { query?: unknown; body?: unknown; }) => void
>validate : (_: { query?: unknown; body?: unknown; }) => void

schema: {
>schema : { query: string; }
>{ query: "", } : { query: string; }

query: "",
>query : string
>"" : ""

},
handle: (req) => {
>handle : (req: Request<{ query: string; }>) => void
>(req) => { const test: string = req.query; } : (req: Request<{ query: string; }>) => void
>req : Request<{ query: string; }>

const test: string = req.query;
>test : string
>req.query : string
>req : Request<{ query: string; }>
>query : string

},
});

export {};

28 changes: 28 additions & 0 deletions tests/cases/compiler/coAndContraVariantInferences6.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// @strict: true
// @noEmit: true

type Request<TSchema extends Schema> = {
query: TSchema["query"];
};

type Schema = { query?: unknown; body?: unknown };

declare function route<TSchema extends Schema>(obj: {
pre: (a: TSchema) => void;
schema: TSchema;
handle: (req: Request<TSchema>) => void;
}): void;

const validate = (_: { query?: unknown; body?: unknown }) => {};

route({
pre: validate,
schema: {
query: "",
},
handle: (req) => {
const test: string = req.query;
},
});

export {};