Skip to content
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.

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

Fix conditional type type parameter leak #31455

Merged
merged 8 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)