Skip to content

Commit

Permalink
Fix conditional type type parameter leak (#31455)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
weswigham authored Mar 9, 2022
1 parent fc82c67 commit f76452c
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 4 deletions.
45 changes: 41 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15817,6 +15817,11 @@ namespace ts {
return type;
}

function maybeCloneTypeParameter(p: TypeParameter) {
const constraint = getConstraintOfTypeParameter(p);
return constraint && (isGenericObjectType(constraint) || isGenericIndexType(constraint)) ? cloneTypeParameter(p) : p;
}

function isTypicalNondistributiveConditional(root: ConditionalRoot) {
return !root.isDistributive && isSingletonTupleType(root.node.checkType) && isSingletonTupleType(root.node.extendsType);
}
Expand Down Expand Up @@ -15858,17 +15863,49 @@ namespace ts {
}
let combinedMapper: TypeMapper | undefined;
if (root.inferTypeParameters) {
const context = createInferenceContext(root.inferTypeParameters, /*signature*/ undefined, InferenceFlags.None);
if (!checkTypeInstantiable) {
// When we're looking at making an inference for an infer type, when we get its constraint, it'll automagically be
// instantiated with the context, so it doesn't need the mapper for the inference contex - however the constraint
// may refer to another _root_, _uncloned_ `infer` type parameter [1], or to something mapped by `mapper` [2].
// [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
// as `number`
// Conversely, if we have `Foo<infer A, infer B>`, `B` is still constrained to `T` and `T` is instantiated as `A`
// [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`
// which is in turn instantiated as `Q`, which is in turn instantiated as `number`.
// So we need to:
// * Clone the type parameters so their constraints can be instantiated in the context of `mapper` (otherwise theyd only get inference context information)
// * Set the clones to both map the conditional's enclosing `mapper` and the original params
// * instantiate the extends type with the clones
// * incorporate all of the component mappers into the combined mapper for the true and false members
// This means we have three mappers that need applying:
// * The original `mapper` used to create this conditional
// * The mapper that maps the old root type parameter to the clone (`freshMapper`)
// * The mapper that maps the clone to its inference result (`context.mapper`)
const freshParams = sameMap(root.inferTypeParameters, maybeCloneTypeParameter);
const freshMapper = freshParams !== root.inferTypeParameters ? createTypeMapper(root.inferTypeParameters, freshParams) : undefined;
const context = createInferenceContext(freshParams, /*signature*/ undefined, InferenceFlags.None);
if (freshMapper) {
const freshCombinedMapper = combineTypeMappers(mapper, freshMapper);
for (const p of freshParams) {
if (root.inferTypeParameters.indexOf(p) === -1) {
p.mapper = freshCombinedMapper;
}
}
}
// We skip inference of the possible `infer` types unles the `extendsType` _is_ an infer type
// if it was, it's trivial to say that extendsType = checkType, however such a pattern is used to
// "reset" the type being build up during constraint calculation and avoid making an apparently "infinite" constraint
// so in those cases we refain from performing inference and retain the uninfered type parameter
if (!checkTypeInstantiable || !some(root.inferTypeParameters, t => t === extendsType)) {
// We don't want inferences from constraints as they may cause us to eagerly resolve the
// conditional type instead of deferring resolution. Also, we always want strict function
// types rules (i.e. proper contravariance) for inferences.
inferTypes(context.inferences, checkType, extendsType, InferencePriority.NoConstraints | InferencePriority.AlwaysStrict);
inferTypes(context.inferences, checkType, instantiateType(extendsType, freshMapper), InferencePriority.NoConstraints | InferencePriority.AlwaysStrict);
}
const innerMapper = combineTypeMappers(freshMapper, context.mapper);
// It's possible for 'infer T' type paramteters to be given uninstantiated constraints when the
// those type parameters are used in type references (see getInferredTypeParameterConstraint). For
// that reason we need context.mapper to be first in the combined mapper. See #42636 for examples.
combinedMapper = mapper ? combineTypeMappers(context.mapper, mapper) : context.mapper;
combinedMapper = mapper ? combineTypeMappers(innerMapper, mapper) : innerMapper;
}
// Instantiate the extends type including inferences for 'infer T' type parameters
const inferredExtendsType = combinedMapper ? instantiateType(unwrapNondistributiveConditionalTuple(root, root.extendsType), combinedMapper) : extendsType;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts(7,7): error TS2322: Type 'string' is not assignable to type 'number'.


==== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts (1 errors) ====
interface Synthetic<A, B extends A> {}
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
SyntheticDestination<number, Synthetic<number, number>>;

const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
~
!!! error TS2322: Type 'string' is not assignable to type 'number'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [conditionalDoesntLeakUninstantiatedTypeParameter.ts]
interface Synthetic<A, B extends A> {}
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
SyntheticDestination<number, Synthetic<number, number>>;

const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)


//// [conditionalDoesntLeakUninstantiatedTypeParameter.js]
var y = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
var z = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
=== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts ===
interface Synthetic<A, B extends A> {}
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))
>A : Symbol(A, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 20))
>B : Symbol(B, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 22))
>A : Symbol(A, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 20))

type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
>SyntheticDestination : Symbol(SyntheticDestination, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 38))
>T : Symbol(T, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 26))
>U : Symbol(U, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 28))
>U : Symbol(U, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 28))
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))
>T : Symbol(T, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 26))
>V : Symbol(V, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 62))
>V : Symbol(V, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 62))

type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))

SyntheticDestination<number, Synthetic<number, number>>;
>SyntheticDestination : Symbol(SyntheticDestination, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 38))
>Synthetic : Symbol(Synthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 0, 0))

const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
>y : Symbol(y, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 5, 5))
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))

const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
>z : Symbol(z, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 6, 5))
>TestSynthetic : Symbol(TestSynthetic, Decl(conditionalDoesntLeakUninstantiatedTypeParameter.ts, 1, 78))

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
=== tests/cases/compiler/conditionalDoesntLeakUninstantiatedTypeParameter.ts ===
interface Synthetic<A, B extends A> {}
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
>SyntheticDestination : SyntheticDestination<T, U>

type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
>TestSynthetic : number

SyntheticDestination<number, Synthetic<number, number>>;

const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
>y : number
>3 : 3

const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)
>z : number
>'3' : "3"

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
interface Synthetic<A, B extends A> {}
type SyntheticDestination<T, U> = U extends Synthetic<T, infer V> ? V : never;
type TestSynthetic = // Resolved to T, should be `number` or an inference failure (`unknown`)
SyntheticDestination<number, Synthetic<number, number>>;

const y: TestSynthetic = 3; // Type '3' is not assignable to type 'T'. (shouldn't error)
const z: TestSynthetic = '3'; // Type '"3""' is not assignable to type 'T'. (should not mention T)

0 comments on commit f76452c

Please sign in to comment.