Skip to content

Commit 8577f7d

Browse files
committed
Err on the safe side
1 parent 17e215c commit 8577f7d

File tree

3 files changed

+43
-9
lines changed

3 files changed

+43
-9
lines changed

src/compiler/checker.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11964,19 +11964,22 @@ namespace ts {
1196411964
const context = createInferenceContext(root.inferTypeParameters, /*signature*/ undefined, InferenceFlags.None);
1196511965
const instantiatedExtends = instantiateType(root.extendsType, mapper);
1196611966
const checkConstraint = getSimplifiedType(instantiateType(root.checkType, mapper));
11967-
// TODO: Use InferencePriority.NoConstraints for type parameters which are being used contravariantly/in a write-only context
11968-
// (Or, ideally, some future InferencePriority.SuperConstraints should we start tracking them!)
11969-
// As-is, this is unsound - it can be made stricter by replacing `instantiateType(root.trueType, combinedMapper)` with
11970-
// `getintersectionType([instantiateType(root.trueType, combinedMapper), instantiateType(root.trueType, mapper)])` which
11971-
// would preserve the type-parametery-ness of the check; but such a limitation makes this branch almost useless, as only
11967+
// TODO:
11968+
// As-is, this is effectively sound, but not particularly useful, thanks to all the types it wrongly rejects - only
1197211969
// conditional types with effectively "independent" inference parameters will end up being assignable via this branch, eg
1197311970
// `type InferBecauseWhyNot<T> = T extends (p: infer P1) => any ? T | P1 : never;`
11974-
// contains a union in the `true` branch, and so while we can't confirm assignability to `P1`, we _could_ confirm assignability
11975-
// to `T`.
11971+
// contains a union in the `true` branch, and so while we can't confirm assignability to `P1`, we can confirm assignability to `T`.
11972+
// A lenient version could be made by replacing `getintersectionType([instantiateType(root.trueType, combinedMapper), instantiateType(root.trueType, mapper)])`
11973+
// with `instantiateType(root.trueType, combinedMapper)` which would skip checking aginst the type-parametery-ness of the check;
11974+
// but such a change introduces quite a bit of unsoundness as we stop checking against the type-parameteryness of the `infer` type,
11975+
// which in turn prevents us from erroring on, eg, unsafe write-position assignments of the constraint of the type.
11976+
// To be correct here, we'd need to track the implied variance of the infer parameters and _infer_ appropriately (in addition to checking appropriately)
11977+
// Specifically, we'd need to infer with `InferencePriority.NoConstraint` (or ideally a hypothetical `InferencePriority.SuperConstraint`) for contravariant types,
11978+
// but continue using the constraints for covariant ones.
1197611979
inferTypes(context.inferences, checkConstraint, instantiatedExtends, InferencePriority.AlwaysStrict);
1197711980
const combinedMapper = combineTypeMappers(mapper, context);
1197811981
if (isRelatedTo(checkConstraint, instantiateType(root.extendsType, combinedMapper))) {
11979-
if (result = isRelatedTo(source, instantiateType(root.trueType, combinedMapper), reportErrors)) {
11982+
if (result = isRelatedTo(source, getIntersectionType([instantiateType(root.trueType, combinedMapper), instantiateType(root.trueType, mapper)]), reportErrors)) {
1198011983
errorInfo = saveErrorInfo;
1198111984
return result;
1198211985
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
tests/cases/compiler/thisConditionalInferenceInClassBody.ts(10,27): error TS2345: Argument of type '"hi"' is not assignable to parameter of type 'Unwrap<this["prop"]>'.
2+
Type '"hi"' is not assignable to type 'string & U'.
3+
Type '"hi"' is not assignable to type 'U'.
4+
5+
6+
==== tests/cases/compiler/thisConditionalInferenceInClassBody.ts (1 errors) ====
7+
type Wrapped<T> = { ___secret: T };
8+
type Unwrap<T> = T extends Wrapped<infer U> ? U : T;
9+
10+
declare function set<T, K extends keyof T>(obj: T, key: K, value: Unwrap<T[K]>): Unwrap<T[K]>;
11+
12+
class Foo {
13+
prop: Wrapped<string>;
14+
15+
method() {
16+
set(this, 'prop', 'hi'); // <-- type error
17+
~~~~
18+
!!! error TS2345: Argument of type '"hi"' is not assignable to parameter of type 'Unwrap<this["prop"]>'.
19+
!!! error TS2345: Type '"hi"' is not assignable to type 'string & U'.
20+
!!! error TS2345: Type '"hi"' is not assignable to type 'U'.
21+
}
22+
}
23+
24+
set(new Foo(), 'prop', 'hi'); // <-- typechecks
25+
26+
type InferBecauseWhyNot<T> = T extends (p: infer P1) => any ? P1 | T : never;
27+
28+
function f<Q extends (arg: any) => any>(x: Q): InferBecauseWhyNot<Q> {
29+
return x;
30+
}
31+

tests/baselines/reference/thisConditionalInferenceInClassBody.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class Foo {
2222
>method : () => void
2323

2424
set(this, 'prop', 'hi'); // <-- type error
25-
>set(this, 'prop', 'hi') : Unwrap<this["prop"]>
25+
>set(this, 'prop', 'hi') : any
2626
>set : <T, K extends keyof T>(obj: T, key: K, value: Unwrap<T[K]>) => Unwrap<T[K]>
2727
>this : this
2828
>'prop' : "prop"

0 commit comments

Comments
 (0)