Skip to content

Commit 2c5ba08

Browse files
JsonFreemanJsonFreeman
committed
Merge pull request microsoft#3787 from Microsoft/fixingTypeParameters
Fix type parameters upon subsequent visits to a parameter that caused fixing the first time
2 parents fbe60c9 + 6df0f3d commit 2c5ba08

15 files changed

+512
-33
lines changed

‎src/compiler/checker.ts

+83-17
Original file line numberDiff line numberDiff line change
@@ -4249,15 +4249,18 @@ namespace ts {
42494249
}
42504250

42514251
function createInferenceMapper(context: InferenceContext): TypeMapper {
4252-
return t => {
4252+
let mapper: TypeMapper = t => {
42534253
for (let i = 0; i < context.typeParameters.length; i++) {
42544254
if (t === context.typeParameters[i]) {
42554255
context.inferences[i].isFixed = true;
42564256
return getInferredType(context, i);
42574257
}
42584258
}
4259-
return t;
4259+
return t;
42604260
};
4261+
4262+
mapper.context = context;
4263+
return mapper;
42614264
}
42624265

42634266
function identityMapper(type: Type): Type {
@@ -5468,7 +5471,9 @@ namespace ts {
54685471
function createInferenceContext(typeParameters: TypeParameter[], inferUnionTypes: boolean): InferenceContext {
54695472
let inferences: TypeInferences[] = [];
54705473
for (let unused of typeParameters) {
5471-
inferences.push({ primary: undefined, secondary: undefined, isFixed: false });
5474+
inferences.push({
5475+
primary: undefined, secondary: undefined, isFixed: false
5476+
});
54725477
}
54735478
return {
54745479
typeParameters,
@@ -6769,10 +6774,23 @@ namespace ts {
67696774
return result;
67706775
}
67716776

6772-
// Presence of a contextual type mapper indicates inferential typing, except the identityMapper object is
6773-
// used as a special marker for other purposes.
6777+
/**
6778+
* Detect if the mapper implies an inference context. Specifically, there are 4 possible values
6779+
* for a mapper. Let's go through each one of them:
6780+
*
6781+
* 1. undefined - this means we are not doing inferential typing, but we may do contextual typing,
6782+
* which could cause us to assign a parameter a type
6783+
* 2. identityMapper - means we want to avoid assigning a parameter a type, whether or not we are in
6784+
* inferential typing (context is undefined for the identityMapper)
6785+
* 3. a mapper created by createInferenceMapper - we are doing inferential typing, we want to assign
6786+
* types to parameters and fix type parameters (context is defined)
6787+
* 4. an instantiation mapper created by createTypeMapper or createTypeEraser - this should never be
6788+
* passed as the contextual mapper when checking an expression (context is undefined for these)
6789+
*
6790+
* isInferentialContext is detecting if we are in case 3
6791+
*/
67746792
function isInferentialContext(mapper: TypeMapper) {
6775-
return mapper && mapper !== identityMapper;
6793+
return mapper && mapper.context;
67766794
}
67776795

67786796
// A node is an assignment target if it is on the left hand side of an '=' token, if it is parented by a property
@@ -8861,13 +8879,52 @@ namespace ts {
88618879
let len = signature.parameters.length - (signature.hasRestParameter ? 1 : 0);
88628880
for (let i = 0; i < len; i++) {
88638881
let parameter = signature.parameters[i];
8864-
let links = getSymbolLinks(parameter);
8865-
links.type = instantiateType(getTypeAtPosition(context, i), mapper);
8882+
let contextualParameterType = getTypeAtPosition(context, i);
8883+
assignTypeToParameterAndFixTypeParameters(parameter, contextualParameterType, mapper);
88668884
}
88678885
if (signature.hasRestParameter && context.hasRestParameter && signature.parameters.length >= context.parameters.length) {
88688886
let parameter = lastOrUndefined(signature.parameters);
8869-
let links = getSymbolLinks(parameter);
8870-
links.type = instantiateType(getTypeOfSymbol(lastOrUndefined(context.parameters)), mapper);
8887+
let contextualParameterType = getTypeOfSymbol(lastOrUndefined(context.parameters));
8888+
assignTypeToParameterAndFixTypeParameters(parameter, contextualParameterType, mapper);
8889+
}
8890+
}
8891+
8892+
function assignTypeToParameterAndFixTypeParameters(parameter: Symbol, contextualType: Type, mapper: TypeMapper) {
8893+
let links = getSymbolLinks(parameter);
8894+
if (!links.type) {
8895+
links.type = instantiateType(contextualType, mapper);
8896+
}
8897+
else if (isInferentialContext(mapper)) {
8898+
// Even if the parameter already has a type, it might be because it was given a type while
8899+
// processing the function as an argument to a prior signature during overload resolution.
8900+
// If this was the case, it may have caused some type parameters to be fixed. So here,
8901+
// we need to ensure that type parameters at the same positions get fixed again. This is
8902+
// done by calling instantiateType to attach the mapper to the contextualType, and then
8903+
// calling inferTypes to force a walk of contextualType so that all the correct fixing
8904+
// happens. The choice to pass in links.type may seem kind of arbitrary, but it serves
8905+
// to make sure that all the correct positions in contextualType are reached by the walk.
8906+
// Here is an example:
8907+
//
8908+
// interface Base {
8909+
// baseProp;
8910+
// }
8911+
// interface Derived extends Base {
8912+
// toBase(): Base;
8913+
// }
8914+
//
8915+
// var derived: Derived;
8916+
//
8917+
// declare function foo<T>(x: T, func: (p: T) => T): T;
8918+
// declare function foo<T>(x: T, func: (p: T) => T): T;
8919+
//
8920+
// var result = foo(derived, d => d.toBase());
8921+
//
8922+
// We are typing d while checking the second overload. But we've already given d
8923+
// a type (Derived) from the first overload. However, we still want to fix the
8924+
// T in the second overload so that we do not infer Base as a candidate for T
8925+
// (inferring Base would make type argument inference inconsistent between the two
8926+
// overloads).
8927+
inferTypes(mapper.context, links.type, instantiateType(contextualType, mapper));
88718928
}
88728929
}
88738930

@@ -9087,27 +9144,36 @@ namespace ts {
90879144

90889145
let links = getNodeLinks(node);
90899146
let type = getTypeOfSymbol(node.symbol);
9090-
// Check if function expression is contextually typed and assign parameter types if so
9091-
if (!(links.flags & NodeCheckFlags.ContextChecked)) {
9147+
let contextSensitive = isContextSensitive(node);
9148+
let mightFixTypeParameters = contextSensitive && isInferentialContext(contextualMapper);
9149+
9150+
// Check if function expression is contextually typed and assign parameter types if so.
9151+
// See the comment in assignTypeToParameterAndFixTypeParameters to understand why we need to
9152+
// check mightFixTypeParameters.
9153+
if (mightFixTypeParameters || !(links.flags & NodeCheckFlags.ContextChecked)) {
90929154
let contextualSignature = getContextualSignature(node);
90939155
// If a type check is started at a function expression that is an argument of a function call, obtaining the
90949156
// contextual type may recursively get back to here during overload resolution of the call. If so, we will have
90959157
// already assigned contextual types.
9096-
if (!(links.flags & NodeCheckFlags.ContextChecked)) {
9158+
let contextChecked = !!(links.flags & NodeCheckFlags.ContextChecked);
9159+
if (mightFixTypeParameters || !contextChecked) {
90979160
links.flags |= NodeCheckFlags.ContextChecked;
90989161
if (contextualSignature) {
90999162
let signature = getSignaturesOfType(type, SignatureKind.Call)[0];
9100-
if (isContextSensitive(node)) {
9163+
if (contextSensitive) {
91019164
assignContextualParameterTypes(signature, contextualSignature, contextualMapper || identityMapper);
91029165
}
9103-
if (!node.type && !signature.resolvedReturnType) {
9166+
if (mightFixTypeParameters || !node.type && !signature.resolvedReturnType) {
91049167
let returnType = getReturnTypeFromBody(node, contextualMapper);
91059168
if (!signature.resolvedReturnType) {
91069169
signature.resolvedReturnType = returnType;
91079170
}
91089171
}
91099172
}
9110-
checkSignatureDeclaration(node);
9173+
9174+
if (!contextChecked) {
9175+
checkSignatureDeclaration(node);
9176+
}
91119177
}
91129178
}
91139179

@@ -9797,7 +9863,7 @@ namespace ts {
97979863
}
97989864

97999865
function instantiateTypeWithSingleGenericCallSignature(node: Expression | MethodDeclaration, type: Type, contextualMapper?: TypeMapper) {
9800-
if (contextualMapper && contextualMapper !== identityMapper) {
9866+
if (isInferentialContext(contextualMapper)) {
98019867
let signature = getSingleCallSignature(type);
98029868
if (signature && signature.typeParameters) {
98039869
let contextualType = getContextualType(<Expression>node);

‎src/compiler/types.ts

+3
Original file line numberDiff line numberDiff line change
@@ -1912,6 +1912,9 @@ namespace ts {
19121912
/* @internal */
19131913
export interface TypeMapper {
19141914
(t: TypeParameter): Type;
1915+
context?: InferenceContext; // The inference context this mapper was created from.
1916+
// Only inference mappers have this set (in createInferenceMapper).
1917+
// The identity mapper and regular instantiation mappers do not need it.
19151918
}
19161919

19171920
/* @internal */

‎tests/baselines/reference/fixingTypeParametersRepeatedly1.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//// [fixingTypeParametersRepeatedly1.ts]
2+
declare function f<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
3+
f("", x => null, x => x.toLowerCase());
4+
5+
// First overload of g should type check just like f
6+
declare function g<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
7+
declare function g();
8+
g("", x => null, x => x.toLowerCase());
9+
10+
//// [fixingTypeParametersRepeatedly1.js]
11+
f("", function (x) { return null; }, function (x) { return x.toLowerCase(); });
12+
g("", function (x) { return null; }, function (x) { return x.toLowerCase(); });

‎tests/baselines/reference/fixingTypeParametersRepeatedly1.symbols

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
=== tests/cases/compiler/fixingTypeParametersRepeatedly1.ts ===
2+
declare function f<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
3+
>f : Symbol(f, Decl(fixingTypeParametersRepeatedly1.ts, 0, 0))
4+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
5+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 0, 22))
6+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
7+
>y : Symbol(y, Decl(fixingTypeParametersRepeatedly1.ts, 0, 27))
8+
>p : Symbol(p, Decl(fixingTypeParametersRepeatedly1.ts, 0, 32))
9+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
10+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
11+
>z : Symbol(z, Decl(fixingTypeParametersRepeatedly1.ts, 0, 43))
12+
>p : Symbol(p, Decl(fixingTypeParametersRepeatedly1.ts, 0, 48))
13+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
14+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
15+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 0, 19))
16+
17+
f("", x => null, x => x.toLowerCase());
18+
>f : Symbol(f, Decl(fixingTypeParametersRepeatedly1.ts, 0, 0))
19+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 1, 5))
20+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 1, 16))
21+
>x.toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, 399, 51))
22+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 1, 16))
23+
>toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, 399, 51))
24+
25+
// First overload of g should type check just like f
26+
declare function g<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
27+
>g : Symbol(g, Decl(fixingTypeParametersRepeatedly1.ts, 1, 39), Decl(fixingTypeParametersRepeatedly1.ts, 4, 63))
28+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
29+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 4, 22))
30+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
31+
>y : Symbol(y, Decl(fixingTypeParametersRepeatedly1.ts, 4, 27))
32+
>p : Symbol(p, Decl(fixingTypeParametersRepeatedly1.ts, 4, 32))
33+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
34+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
35+
>z : Symbol(z, Decl(fixingTypeParametersRepeatedly1.ts, 4, 43))
36+
>p : Symbol(p, Decl(fixingTypeParametersRepeatedly1.ts, 4, 48))
37+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
38+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
39+
>T : Symbol(T, Decl(fixingTypeParametersRepeatedly1.ts, 4, 19))
40+
41+
declare function g();
42+
>g : Symbol(g, Decl(fixingTypeParametersRepeatedly1.ts, 1, 39), Decl(fixingTypeParametersRepeatedly1.ts, 4, 63))
43+
44+
g("", x => null, x => x.toLowerCase());
45+
>g : Symbol(g, Decl(fixingTypeParametersRepeatedly1.ts, 1, 39), Decl(fixingTypeParametersRepeatedly1.ts, 4, 63))
46+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 6, 5))
47+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 6, 16))
48+
>x.toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, 399, 51))
49+
>x : Symbol(x, Decl(fixingTypeParametersRepeatedly1.ts, 6, 16))
50+
>toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, 399, 51))
51+

‎tests/baselines/reference/fixingTypeParametersRepeatedly1.types

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
=== tests/cases/compiler/fixingTypeParametersRepeatedly1.ts ===
2+
declare function f<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
3+
>f : <T>(x: T, y: (p: T) => T, z: (p: T) => T) => T
4+
>T : T
5+
>x : T
6+
>T : T
7+
>y : (p: T) => T
8+
>p : T
9+
>T : T
10+
>T : T
11+
>z : (p: T) => T
12+
>p : T
13+
>T : T
14+
>T : T
15+
>T : T
16+
17+
f("", x => null, x => x.toLowerCase());
18+
>f("", x => null, x => x.toLowerCase()) : string
19+
>f : <T>(x: T, y: (p: T) => T, z: (p: T) => T) => T
20+
>"" : string
21+
>x => null : (x: string) => any
22+
>x : string
23+
>null : null
24+
>x => x.toLowerCase() : (x: string) => string
25+
>x : string
26+
>x.toLowerCase() : string
27+
>x.toLowerCase : () => string
28+
>x : string
29+
>toLowerCase : () => string
30+
31+
// First overload of g should type check just like f
32+
declare function g<T>(x: T, y: (p: T) => T, z: (p: T) => T): T;
33+
>g : { <T>(x: T, y: (p: T) => T, z: (p: T) => T): T; (): any; }
34+
>T : T
35+
>x : T
36+
>T : T
37+
>y : (p: T) => T
38+
>p : T
39+
>T : T
40+
>T : T
41+
>z : (p: T) => T
42+
>p : T
43+
>T : T
44+
>T : T
45+
>T : T
46+
47+
declare function g();
48+
>g : { <T>(x: T, y: (p: T) => T, z: (p: T) => T): T; (): any; }
49+
50+
g("", x => null, x => x.toLowerCase());
51+
>g("", x => null, x => x.toLowerCase()) : string
52+
>g : { <T>(x: T, y: (p: T) => T, z: (p: T) => T): T; (): any; }
53+
>"" : string
54+
>x => null : (x: string) => any
55+
>x : string
56+
>null : null
57+
>x => x.toLowerCase() : (x: string) => string
58+
>x : string
59+
>x.toLowerCase() : string
60+
>x.toLowerCase : () => string
61+
>x : string
62+
>toLowerCase : () => string
63+

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

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
tests/cases/compiler/fixingTypeParametersRepeatedly2.ts(11,27): error TS2345: Argument of type '(d: Derived) => Base' is not assignable to parameter of type '(p: Derived) => Derived'.
2+
Type 'Base' is not assignable to type 'Derived'.
3+
Property 'toBase' is missing in type 'Base'.
4+
tests/cases/compiler/fixingTypeParametersRepeatedly2.ts(17,27): error TS2345: Argument of type '(d: Derived) => Base' is not assignable to parameter of type '(p: Derived) => Derived'.
5+
Type 'Base' is not assignable to type 'Derived'.
6+
7+
8+
==== tests/cases/compiler/fixingTypeParametersRepeatedly2.ts (2 errors) ====
9+
interface Base {
10+
baseProp;
11+
}
12+
interface Derived extends Base {
13+
toBase(): Base;
14+
}
15+
16+
var derived: Derived;
17+
18+
declare function foo<T>(x: T, func: (p: T) => T): T;
19+
var result = foo(derived, d => d.toBase());
20+
~~~~~~~~~~~~~~~
21+
!!! error TS2345: Argument of type '(d: Derived) => Base' is not assignable to parameter of type '(p: Derived) => Derived'.
22+
!!! error TS2345: Type 'Base' is not assignable to type 'Derived'.
23+
!!! error TS2345: Property 'toBase' is missing in type 'Base'.
24+
25+
// bar should type check just like foo.
26+
// The same error should be observed in both cases.
27+
declare function bar<T>(x: T, func: (p: T) => T): T;
28+
declare function bar<T>(x: T, func: (p: T) => T): T;
29+
var result = bar(derived, d => d.toBase());
30+
~~~~~~~~~~~~~~~
31+
!!! error TS2345: Argument of type '(d: Derived) => Base' is not assignable to parameter of type '(p: Derived) => Derived'.
32+
!!! error TS2345: Type 'Base' is not assignable to type 'Derived'.

‎tests/baselines/reference/fixingTypeParametersRepeatedly2.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [fixingTypeParametersRepeatedly2.ts]
2+
interface Base {
3+
baseProp;
4+
}
5+
interface Derived extends Base {
6+
toBase(): Base;
7+
}
8+
9+
var derived: Derived;
10+
11+
declare function foo<T>(x: T, func: (p: T) => T): T;
12+
var result = foo(derived, d => d.toBase());
13+
14+
// bar should type check just like foo.
15+
// The same error should be observed in both cases.
16+
declare function bar<T>(x: T, func: (p: T) => T): T;
17+
declare function bar<T>(x: T, func: (p: T) => T): T;
18+
var result = bar(derived, d => d.toBase());
19+
20+
//// [fixingTypeParametersRepeatedly2.js]
21+
var derived;
22+
var result = foo(derived, function (d) { return d.toBase(); });
23+
var result = bar(derived, function (d) { return d.toBase(); });

‎tests/baselines/reference/fixingTypeParametersRepeatedly3.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [fixingTypeParametersRepeatedly3.ts]
2+
interface Base {
3+
baseProp;
4+
}
5+
interface Derived extends Base {
6+
toBase?(): Base;
7+
}
8+
9+
var derived: Derived;
10+
11+
declare function foo<T>(x: T, func: (p: T) => T): T;
12+
var result = foo(derived, d => d.toBase());
13+
14+
// bar should type check just like foo.
15+
// result2 should have the same type as result
16+
declare function bar<T>(x: T, func: (p: T) => T): T;
17+
declare function bar<T>(x: T, func: (p: T) => T): T;
18+
var result2 = bar(derived, d => d.toBase());
19+
20+
//// [fixingTypeParametersRepeatedly3.js]
21+
var derived;
22+
var result = foo(derived, function (d) { return d.toBase(); });
23+
var result2 = bar(derived, function (d) { return d.toBase(); });

0 commit comments

Comments
 (0)