Skip to content

Commit f03e59e

Browse files
weswighamweswigham
authored
Do not covariantly mix in constraints from contravarrying positions (microsoft#43439)
* Do not covariantly mix in constraints from contravarrying positions * Exclude keyof from constraint variance tracking * Extra test case * Always subtitute on type parameter types
1 parent 2f36065 commit f03e59e

5 files changed

Lines changed: 300 additions & 1 deletion

File tree

‎src/compiler/checker.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12854,9 +12854,17 @@ namespace ts {
1285412854

1285512855
function getConditionalFlowTypeOfType(type: Type, node: Node) {
1285612856
let constraints: Type[] | undefined;
12857+
let covariant = true;
1285712858
while (node && !isStatement(node) && node.kind !== SyntaxKind.JSDocComment) {
1285812859
const parent = node.parent;
12859-
if (parent.kind === SyntaxKind.ConditionalType && node === (<ConditionalTypeNode>parent).trueType) {
12860+
// only consider variance flipped by parameter locations - `keyof` types would usually be considered variance inverting, but
12861+
// often get used in indexed accesses where they behave sortof invariantly, but our checking is lax
12862+
if (parent.kind === SyntaxKind.Parameter) {
12863+
covariant = !covariant;
12864+
}
12865+
// Always substitute on type parameters, regardless of variance, since even
12866+
// in contravarrying positions, they may be reliant on subtuted constraints to be valid
12867+
if ((covariant || type.flags & TypeFlags.TypeParameter) && parent.kind === SyntaxKind.ConditionalType && node === (<ConditionalTypeNode>parent).trueType) {
1286012868
const constraint = getImpliedConstraint(type, (<ConditionalTypeNode>parent).checkType, (<ConditionalTypeNode>parent).extendsType);
1286112869
if (constraint) {
1286212870
constraints = append(constraints, constraint);

‎tests/baselines/reference/callOfConditionalTypeWithConcreteBranches.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//// [callOfConditionalTypeWithConcreteBranches.ts]
2+
type Q<T> = number extends T ? (n: number) => void : never;
3+
function fn<T>(arg: Q<T>) {
4+
// Expected: OK
5+
// Actual: Cannot convert 10 to number & T
6+
arg(10);
7+
}
8+
// Legal invocations are not problematic
9+
fn<string | number>(m => m.toFixed());
10+
fn<number>(m => m.toFixed());
11+
12+
// Ensure the following real-world example that relies on substitution still works
13+
type ExtractParameters<T> = "parameters" extends keyof T
14+
// The above allows "parameters" to index `T` since all later
15+
// instances are actually implicitly `"parameters" & keyof T`
16+
? {
17+
[K in keyof T["parameters"]]: T["parameters"][K];
18+
}[keyof T["parameters"]]
19+
: {};
20+
21+
// Original example, but with inverted variance
22+
type Q2<T> = number extends T ? (cb: (n: number) => void) => void : never;
23+
function fn2<T>(arg: Q2<T>) {
24+
function useT(_arg: T): void {}
25+
// Expected: OK
26+
arg(arg => useT(arg));
27+
}
28+
// Legal invocations are not problematic
29+
fn2<string | number>(m => m(42));
30+
fn2<number>(m => m(42));
31+
32+
// webidl-conversions example where substituion must occur, despite contravariance of the position
33+
// due to the invariant usage in `Parameters`
34+
35+
type X<V> = V extends (...args: any[]) => any ? (...args: Parameters<V>) => void : Function;
36+
37+
//// [callOfConditionalTypeWithConcreteBranches.js]
38+
function fn(arg) {
39+
// Expected: OK
40+
// Actual: Cannot convert 10 to number & T
41+
arg(10);
42+
}
43+
// Legal invocations are not problematic
44+
fn(function (m) { return m.toFixed(); });
45+
fn(function (m) { return m.toFixed(); });
46+
function fn2(arg) {
47+
function useT(_arg) { }
48+
// Expected: OK
49+
arg(function (arg) { return useT(arg); });
50+
}
51+
// Legal invocations are not problematic
52+
fn2(function (m) { return m(42); });
53+
fn2(function (m) { return m(42); });

‎tests/baselines/reference/callOfConditionalTypeWithConcreteBranches.symbols‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
=== tests/cases/compiler/callOfConditionalTypeWithConcreteBranches.ts ===
2+
type Q<T> = number extends T ? (n: number) => void : never;
3+
>Q : Symbol(Q, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 0))
4+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 7))
5+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 7))
6+
>n : Symbol(n, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 32))
7+
8+
function fn<T>(arg: Q<T>) {
9+
>fn : Symbol(fn, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 59))
10+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 1, 12))
11+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 1, 15))
12+
>Q : Symbol(Q, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 0))
13+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 1, 12))
14+
15+
// Expected: OK
16+
// Actual: Cannot convert 10 to number & T
17+
arg(10);
18+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 1, 15))
19+
}
20+
// Legal invocations are not problematic
21+
fn<string | number>(m => m.toFixed());
22+
>fn : Symbol(fn, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 59))
23+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 7, 20))
24+
>m.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
25+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 7, 20))
26+
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
27+
28+
fn<number>(m => m.toFixed());
29+
>fn : Symbol(fn, Decl(callOfConditionalTypeWithConcreteBranches.ts, 0, 59))
30+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 8, 11))
31+
>m.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
32+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 8, 11))
33+
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
34+
35+
// Ensure the following real-world example that relies on substitution still works
36+
type ExtractParameters<T> = "parameters" extends keyof T
37+
>ExtractParameters : Symbol(ExtractParameters, Decl(callOfConditionalTypeWithConcreteBranches.ts, 8, 29))
38+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 11, 23))
39+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 11, 23))
40+
41+
// The above allows "parameters" to index `T` since all later
42+
// instances are actually implicitly `"parameters" & keyof T`
43+
? {
44+
[K in keyof T["parameters"]]: T["parameters"][K];
45+
>K : Symbol(K, Decl(callOfConditionalTypeWithConcreteBranches.ts, 15, 9))
46+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 11, 23))
47+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 11, 23))
48+
>K : Symbol(K, Decl(callOfConditionalTypeWithConcreteBranches.ts, 15, 9))
49+
50+
}[keyof T["parameters"]]
51+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 11, 23))
52+
53+
: {};
54+
55+
// Original example, but with inverted variance
56+
type Q2<T> = number extends T ? (cb: (n: number) => void) => void : never;
57+
>Q2 : Symbol(Q2, Decl(callOfConditionalTypeWithConcreteBranches.ts, 17, 7))
58+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 8))
59+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 8))
60+
>cb : Symbol(cb, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 33))
61+
>n : Symbol(n, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 38))
62+
63+
function fn2<T>(arg: Q2<T>) {
64+
>fn2 : Symbol(fn2, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 74))
65+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 13))
66+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 16))
67+
>Q2 : Symbol(Q2, Decl(callOfConditionalTypeWithConcreteBranches.ts, 17, 7))
68+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 13))
69+
70+
function useT(_arg: T): void {}
71+
>useT : Symbol(useT, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 29))
72+
>_arg : Symbol(_arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 22, 16))
73+
>T : Symbol(T, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 13))
74+
75+
// Expected: OK
76+
arg(arg => useT(arg));
77+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 16))
78+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 24, 6))
79+
>useT : Symbol(useT, Decl(callOfConditionalTypeWithConcreteBranches.ts, 21, 29))
80+
>arg : Symbol(arg, Decl(callOfConditionalTypeWithConcreteBranches.ts, 24, 6))
81+
}
82+
// Legal invocations are not problematic
83+
fn2<string | number>(m => m(42));
84+
>fn2 : Symbol(fn2, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 74))
85+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 27, 21))
86+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 27, 21))
87+
88+
fn2<number>(m => m(42));
89+
>fn2 : Symbol(fn2, Decl(callOfConditionalTypeWithConcreteBranches.ts, 20, 74))
90+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 28, 12))
91+
>m : Symbol(m, Decl(callOfConditionalTypeWithConcreteBranches.ts, 28, 12))
92+
93+
// webidl-conversions example where substituion must occur, despite contravariance of the position
94+
// due to the invariant usage in `Parameters`
95+
96+
type X<V> = V extends (...args: any[]) => any ? (...args: Parameters<V>) => void : Function;
97+
>X : Symbol(X, Decl(callOfConditionalTypeWithConcreteBranches.ts, 28, 24))
98+
>V : Symbol(V, Decl(callOfConditionalTypeWithConcreteBranches.ts, 33, 7))
99+
>V : Symbol(V, Decl(callOfConditionalTypeWithConcreteBranches.ts, 33, 7))
100+
>args : Symbol(args, Decl(callOfConditionalTypeWithConcreteBranches.ts, 33, 23))
101+
>args : Symbol(args, Decl(callOfConditionalTypeWithConcreteBranches.ts, 33, 49))
102+
>Parameters : Symbol(Parameters, Decl(lib.es5.d.ts, --, --))
103+
>V : Symbol(V, Decl(callOfConditionalTypeWithConcreteBranches.ts, 33, 7))
104+
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
105+

‎tests/baselines/reference/callOfConditionalTypeWithConcreteBranches.types‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
=== tests/cases/compiler/callOfConditionalTypeWithConcreteBranches.ts ===
2+
type Q<T> = number extends T ? (n: number) => void : never;
3+
>Q : Q<T>
4+
>n : number
5+
6+
function fn<T>(arg: Q<T>) {
7+
>fn : <T>(arg: Q<T>) => void
8+
>arg : Q<T>
9+
10+
// Expected: OK
11+
// Actual: Cannot convert 10 to number & T
12+
arg(10);
13+
>arg(10) : void
14+
>arg : Q<T>
15+
>10 : 10
16+
}
17+
// Legal invocations are not problematic
18+
fn<string | number>(m => m.toFixed());
19+
>fn<string | number>(m => m.toFixed()) : void
20+
>fn : <T>(arg: Q<T>) => void
21+
>m => m.toFixed() : (m: number) => string
22+
>m : number
23+
>m.toFixed() : string
24+
>m.toFixed : (fractionDigits?: number) => string
25+
>m : number
26+
>toFixed : (fractionDigits?: number) => string
27+
28+
fn<number>(m => m.toFixed());
29+
>fn<number>(m => m.toFixed()) : void
30+
>fn : <T>(arg: Q<T>) => void
31+
>m => m.toFixed() : (m: number) => string
32+
>m : number
33+
>m.toFixed() : string
34+
>m.toFixed : (fractionDigits?: number) => string
35+
>m : number
36+
>toFixed : (fractionDigits?: number) => string
37+
38+
// Ensure the following real-world example that relies on substitution still works
39+
type ExtractParameters<T> = "parameters" extends keyof T
40+
>ExtractParameters : ExtractParameters<T>
41+
42+
// The above allows "parameters" to index `T` since all later
43+
// instances are actually implicitly `"parameters" & keyof T`
44+
? {
45+
[K in keyof T["parameters"]]: T["parameters"][K];
46+
}[keyof T["parameters"]]
47+
: {};
48+
49+
// Original example, but with inverted variance
50+
type Q2<T> = number extends T ? (cb: (n: number) => void) => void : never;
51+
>Q2 : Q2<T>
52+
>cb : (n: number) => void
53+
>n : number
54+
55+
function fn2<T>(arg: Q2<T>) {
56+
>fn2 : <T>(arg: Q2<T>) => void
57+
>arg : Q2<T>
58+
59+
function useT(_arg: T): void {}
60+
>useT : (_arg: T) => void
61+
>_arg : T
62+
63+
// Expected: OK
64+
arg(arg => useT(arg));
65+
>arg(arg => useT(arg)) : void
66+
>arg : Q2<T>
67+
>arg => useT(arg) : (arg: T & number) => void
68+
>arg : T & number
69+
>useT(arg) : void
70+
>useT : (_arg: T) => void
71+
>arg : T & number
72+
}
73+
// Legal invocations are not problematic
74+
fn2<string | number>(m => m(42));
75+
>fn2<string | number>(m => m(42)) : void
76+
>fn2 : <T>(arg: Q2<T>) => void
77+
>m => m(42) : (m: (n: number) => void) => void
78+
>m : (n: number) => void
79+
>m(42) : void
80+
>m : (n: number) => void
81+
>42 : 42
82+
83+
fn2<number>(m => m(42));
84+
>fn2<number>(m => m(42)) : void
85+
>fn2 : <T>(arg: Q2<T>) => void
86+
>m => m(42) : (m: (n: number) => void) => void
87+
>m : (n: number) => void
88+
>m(42) : void
89+
>m : (n: number) => void
90+
>42 : 42
91+
92+
// webidl-conversions example where substituion must occur, despite contravariance of the position
93+
// due to the invariant usage in `Parameters`
94+
95+
type X<V> = V extends (...args: any[]) => any ? (...args: Parameters<V>) => void : Function;
96+
>X : X<V>
97+
>args : any[]
98+
>args : Parameters<V>
99+

‎tests/cases/compiler/callOfConditionalTypeWithConcreteBranches.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
type Q<T> = number extends T ? (n: number) => void : never;
2+
function fn<T>(arg: Q<T>) {
3+
// Expected: OK
4+
// Actual: Cannot convert 10 to number & T
5+
arg(10);
6+
}
7+
// Legal invocations are not problematic
8+
fn<string | number>(m => m.toFixed());
9+
fn<number>(m => m.toFixed());
10+
11+
// Ensure the following real-world example that relies on substitution still works
12+
type ExtractParameters<T> = "parameters" extends keyof T
13+
// The above allows "parameters" to index `T` since all later
14+
// instances are actually implicitly `"parameters" & keyof T`
15+
? {
16+
[K in keyof T["parameters"]]: T["parameters"][K];
17+
}[keyof T["parameters"]]
18+
: {};
19+
20+
// Original example, but with inverted variance
21+
type Q2<T> = number extends T ? (cb: (n: number) => void) => void : never;
22+
function fn2<T>(arg: Q2<T>) {
23+
function useT(_arg: T): void {}
24+
// Expected: OK
25+
arg(arg => useT(arg));
26+
}
27+
// Legal invocations are not problematic
28+
fn2<string | number>(m => m(42));
29+
fn2<number>(m => m(42));
30+
31+
// webidl-conversions example where substituion must occur, despite contravariance of the position
32+
// due to the invariant usage in `Parameters`
33+
34+
type X<V> = V extends (...args: any[]) => any ? (...args: Parameters<V>) => void : Function;

0 commit comments

Comments
 (0)