Skip to content

Commit f76452c

Browse files
weswighamweswigham
authored
Fix conditional type type parameter leak (microsoft#31455)
* Fix conditional type type parameter leak * Monkey with comment text per code review * Conditionally clone type param * Reuse input array and avoid making mapper where possible
1 parent fc82c67 commit f76452c

6 files changed

+125
-4
lines changed

‎src/compiler/checker.ts

+41-4
Original file line numberDiff line numberDiff line change
@@ -15817,6 +15817,11 @@ namespace ts {
1581715817
return type;
1581815818
}
1581915819

15820+
function maybeCloneTypeParameter(p: TypeParameter) {
15821+
const constraint = getConstraintOfTypeParameter(p);
15822+
return constraint && (isGenericObjectType(constraint) || isGenericIndexType(constraint)) ? cloneTypeParameter(p) : p;
15823+
}
15824+
1582015825
function isTypicalNondistributiveConditional(root: ConditionalRoot) {
1582115826
return !root.isDistributive && isSingletonTupleType(root.node.checkType) && isSingletonTupleType(root.node.extendsType);
1582215827
}
@@ -15858,17 +15863,49 @@ namespace ts {
1585815863
}
1585915864
let combinedMapper: TypeMapper | undefined;
1586015865
if (root.inferTypeParameters) {
15861-
const context = createInferenceContext(root.inferTypeParameters, /*signature*/ undefined, InferenceFlags.None);
15862-
if (!checkTypeInstantiable) {
15866+
// When we're looking at making an inference for an infer type, when we get its constraint, it'll automagically be
15867+
// instantiated with the context, so it doesn't need the mapper for the inference contex - however the constraint
15868+
// may refer to another _root_, _uncloned_ `infer` type parameter [1], or to something mapped by `mapper` [2].
15869+
// [1] Eg, if we have `Foo<T, U extends T>` and `Foo<number, infer B>` - `B` is constrained to `T`, which, in turn, has been instantiated
15870+
// as `number`
15871+
// Conversely, if we have `Foo<infer A, infer B>`, `B` is still constrained to `T` and `T` is instantiated as `A`
15872+
// [2] Eg, if we have `Foo<T, U extends T>` and `Foo<Q, infer B>` where `Q` is mapped by `mapper` into `number` - `B` is constrained to `T`
15873+
// which is in turn instantiated as `Q`, which is in turn instantiated as `number`.
15874+
// So we need to:
15875+
// * Clone the type parameters so their constraints can be instantiated in the context of `mapper` (otherwise theyd only get inference context information)
15876+
// * Set the clones to both map the conditional's enclosing `mapper` and the original params
15877+
// * instantiate the extends type with the clones
15878+
// * incorporate all of the component mappers into the combined mapper for the true and false members
15879+
// This means we have three mappers that need applying:
15880+
// * The original `mapper` used to create this conditional
15881+
// * The mapper that maps the old root type parameter to the clone (`freshMapper`)
15882+
// * The mapper that maps the clone to its inference result (`context.mapper`)
15883+
const freshParams = sameMap(root.inferTypeParameters, maybeCloneTypeParameter);
15884+
const freshMapper = freshParams !== root.inferTypeParameters ? createTypeMapper(root.inferTypeParameters, freshParams) : undefined;
15885+
const context = createInferenceContext(freshParams, /*signature*/ undefined, InferenceFlags.None);
15886+
if (freshMapper) {
15887+
const freshCombinedMapper = combineTypeMappers(mapper, freshMapper);
15888+
for (const p of freshParams) {
15889+
if (root.inferTypeParameters.indexOf(p) === -1) {
15890+
p.mapper = freshCombinedMapper;
15891+
}
15892+
}
15893+
}
15894+
// We skip inference of the possible `infer` types unles the `extendsType` _is_ an infer type
15895+
// if it was, it's trivial to say that extendsType = checkType, however such a pattern is used to
15896+
// "reset" the type being build up during constraint calculation and avoid making an apparently "infinite" constraint
15897+
// so in those cases we refain from performing inference and retain the uninfered type parameter
15898+
if (!checkTypeInstantiable || !some(root.inferTypeParameters, t => t === extendsType)) {
1586315899
// We don't want inferences from constraints as they may cause us to eagerly resolve the
1586415900
// conditional type instead of deferring resolution. Also, we always want strict function
1586515901
// types rules (i.e. proper contravariance) for inferences.
15866-
inferTypes(context.inferences, checkType, extendsType, InferencePriority.NoConstraints | InferencePriority.AlwaysStrict);
15902+
inferTypes(context.inferences, checkType, instantiateType(extendsType, freshMapper), InferencePriority.NoConstraints | InferencePriority.AlwaysStrict);
1586715903
}
15904+
const innerMapper = combineTypeMappers(freshMapper, context.mapper);
1586815905
// It's possible for 'infer T' type paramteters to be given uninstantiated constraints when the
1586915906
// those type parameters are used in type references (see getInferredTypeParameterConstraint). For
1587015907
// that reason we need context.mapper to be first in the combined mapper. See #42636 for examples.
15871-
combinedMapper = mapper ? combineTypeMappers(context.mapper, mapper) : context.mapper;
15908+
combinedMapper = mapper ? combineTypeMappers(innerMapper, mapper) : innerMapper;
1587215909
}
1587315910
// Instantiate the extends type including inferences for 'infer T' type parameters
1587415911
const inferredExtendsType = combinedMapper ? instantiateType(unwrapNondistributiveConditionalTuple(root, root.extendsType), combinedMapper) : extendsType;

‎tests/baselines/reference/conditionalDoesntLeakUninstantiatedTypeParameter.errors.txt

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts(7,7): error TS2322: Type 'string' is not assignable to type 'number'.
2+
3+
4+
==== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts (1 errors) ====
5+
interface Synthetic<A, B extends A> {}
6+
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
7+
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
8+
SyntheticDestination<number, Synthetic<number, number>>;
9+
10+
const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
11+
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
12+
~
13+
!!! error TS2322: Type 'string' is not assignable to type 'number'.
14+

‎tests/baselines/reference/conditionalDoesntLeakUninstantiatedTypeParameter.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//// [conditionalDoesntLeakUninstantiatedTypeParameter.ts]
2+
interface Synthetic<A, B extends A> {}
3+
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
4+
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
5+
SyntheticDestination<number, Synthetic<number, number>>;
6+
7+
const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
8+
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
9+
10+
11+
//// [conditionalDoesntLeakUninstantiatedTypeParameter.js]
12+
var y = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
13+
var z = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)

‎tests/baselines/reference/conditionalDoesntLeakUninstantiatedTypeParameter.symbols

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
=== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts ===
2+
interface Synthetic<A, B extends A> {}
3+
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))
4+
>A : Symbol(A, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 20))
5+
>B : Symbol(B, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 22))
6+
>A : Symbol(A, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 20))
7+
8+
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
9+
>SyntheticDestination : Symbol(SyntheticDestination, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 38))
10+
>T : Symbol(T, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 26))
11+
>U : Symbol(U, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 28))
12+
>U : Symbol(U, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 28))
13+
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))
14+
>T : Symbol(T, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 26))
15+
>V : Symbol(V, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 62))
16+
>V : Symbol(V, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 62))
17+
18+
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
19+
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))
20+
21+
SyntheticDestination<number, Synthetic<number, number>>;
22+
>SyntheticDestination : Symbol(SyntheticDestination, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 38))
23+
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))
24+
25+
const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
26+
>y : Symbol(y, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 5, 5))
27+
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))
28+
29+
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
30+
>z : Symbol(z, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 6, 5))
31+
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))
32+

‎tests/baselines/reference/conditionalDoesntLeakUninstantiatedTypeParameter.types

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
=== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts ===
2+
interface Synthetic<A, B extends A> {}
3+
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
4+
>SyntheticDestination : SyntheticDestination<T, U>
5+
6+
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
7+
>TestSynthetic : number
8+
9+
SyntheticDestination<number, Synthetic<number, number>>;
10+
11+
const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
12+
>y : number
13+
>3 : 3
14+
15+
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
16+
>z : number
17+
>'3' : "3"
18+

‎tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
interface Synthetic<A, B extends A> {}
2+
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
3+
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
4+
SyntheticDestination<number, Synthetic<number, number>>;
5+
6+
const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
7+
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)

0 commit comments

Comments
 (0)