Skip to content

Commit b217f22

Browse files
authored
Allow identical type parameter lists to merge in union signatures (microsoft#31023)
* Have signature identity checks look at the base constraint of type parameters, allow identical type parameter lists to merge in union signatures * Update text in fourslash test * Add whitespace to fix lint, remove duplicate impl * Consolidate names * Remove comparisons of type parameter defaults, add more test cases
1 parent 675cd4d commit b217f22

File tree

7 files changed

+765
-11
lines changed

7 files changed

+765
-11
lines changed

src/compiler/checker.ts

+49-10
Original file line numberDiff line numberDiff line change
@@ -10274,7 +10274,7 @@ namespace ts {
1027410274
if (signatures !== masterList) {
1027510275
const signature = signatures[0];
1027610276
Debug.assert(!!signature, "getUnionSignatures bails early on empty signature lists and should not have empty lists on second pass");
10277-
results = signature.typeParameters && some(results, s => !!s.typeParameters) ? undefined : map(results, sig => combineSignaturesOfUnionMembers(sig, signature));
10277+
results = signature.typeParameters && some(results, s => !!s.typeParameters && !compareTypeParametersIdentical(signature.typeParameters!, s.typeParameters)) ? undefined : map(results, sig => combineSignaturesOfUnionMembers(sig, signature));
1027810278
if (!results) {
1027910279
break;
1028010280
}
@@ -10285,18 +10285,39 @@ namespace ts {
1028510285
return result || emptyArray;
1028610286
}
1028710287

10288-
function combineUnionThisParam(left: Symbol | undefined, right: Symbol | undefined): Symbol | undefined {
10288+
function compareTypeParametersIdentical(sourceParams: readonly TypeParameter[], targetParams: readonly TypeParameter[]): boolean {
10289+
if (sourceParams.length !== targetParams.length) {
10290+
return false;
10291+
}
10292+
10293+
const mapper = createTypeMapper(targetParams, sourceParams);
10294+
for (let i = 0; i < sourceParams.length; i++) {
10295+
const source = sourceParams[i];
10296+
const target = targetParams[i];
10297+
if (source === target) continue;
10298+
// We instantiate the target type parameter constraints into the source types so we can recognize `<T, U extends T>` as the same as `<A, B extends A>`
10299+
if (!isTypeIdenticalTo(getConstraintFromTypeParameter(source) || unknownType, instantiateType(getConstraintFromTypeParameter(target) || unknownType, mapper))) return false;
10300+
// We don't compare defaults - we just use the type parameter defaults from the first signature that seems to match.
10301+
// It might make sense to combine these defaults in the future, but doing so intelligently requires knowing
10302+
// if the parameter is used covariantly or contravariantly (so we intersect if it's used like a parameter or union if used like a return type)
10303+
// and, since it's just an inference _default_, just picking one arbitrarily works OK.
10304+
}
10305+
10306+
return true;
10307+
}
10308+
10309+
function combineUnionThisParam(left: Symbol | undefined, right: Symbol | undefined, mapper: TypeMapper | undefined): Symbol | undefined {
1028910310
if (!left || !right) {
1029010311
return left || right;
1029110312
}
1029210313
// A signature `this` type might be a read or a write position... It's very possible that it should be invariant
1029310314
// and we should refuse to merge signatures if there are `this` types and they do not match. However, so as to be
1029410315
// permissive when calling, for now, we'll intersect the `this` types just like we do for param types in union signatures.
10295-
const thisType = getIntersectionType([getTypeOfSymbol(left), getTypeOfSymbol(right)]);
10316+
const thisType = getIntersectionType([getTypeOfSymbol(left), instantiateType(getTypeOfSymbol(right), mapper)]);
1029610317
return createSymbolWithType(left, thisType);
1029710318
}
1029810319

10299-
function combineUnionParameters(left: Signature, right: Signature) {
10320+
function combineUnionParameters(left: Signature, right: Signature, mapper: TypeMapper | undefined) {
1030010321
const leftCount = getParameterCount(left);
1030110322
const rightCount = getParameterCount(right);
1030210323
const longest = leftCount >= rightCount ? left : right;
@@ -10306,8 +10327,14 @@ namespace ts {
1030610327
const needsExtraRestElement = eitherHasEffectiveRest && !hasEffectiveRestParameter(longest);
1030710328
const params = new Array<Symbol>(longestCount + (needsExtraRestElement ? 1 : 0));
1030810329
for (let i = 0; i < longestCount; i++) {
10309-
const longestParamType = tryGetTypeAtPosition(longest, i)!;
10310-
const shorterParamType = tryGetTypeAtPosition(shorter, i) || unknownType;
10330+
let longestParamType = tryGetTypeAtPosition(longest, i)!;
10331+
if (longest === right) {
10332+
longestParamType = instantiateType(longestParamType, mapper);
10333+
}
10334+
let shorterParamType = tryGetTypeAtPosition(shorter, i) || unknownType;
10335+
if (shorter === right) {
10336+
shorterParamType = instantiateType(shorterParamType, mapper);
10337+
}
1031110338
const unionParamType = getIntersectionType([longestParamType, shorterParamType]);
1031210339
const isRestParam = eitherHasEffectiveRest && !needsExtraRestElement && i === (longestCount - 1);
1031310340
const isOptional = i >= getMinArgumentCount(longest) && i >= getMinArgumentCount(shorter);
@@ -10328,19 +10355,28 @@ namespace ts {
1032810355
if (needsExtraRestElement) {
1032910356
const restParamSymbol = createSymbol(SymbolFlags.FunctionScopedVariable, "args" as __String);
1033010357
restParamSymbol.type = createArrayType(getTypeAtPosition(shorter, longestCount));
10358+
if (shorter === right) {
10359+
restParamSymbol.type = instantiateType(restParamSymbol.type, mapper);
10360+
}
1033110361
params[longestCount] = restParamSymbol;
1033210362
}
1033310363
return params;
1033410364
}
1033510365

1033610366
function combineSignaturesOfUnionMembers(left: Signature, right: Signature): Signature {
10367+
const typeParams = left.typeParameters || right.typeParameters;
10368+
let paramMapper: TypeMapper | undefined;
10369+
if (left.typeParameters && right.typeParameters) {
10370+
paramMapper = createTypeMapper(right.typeParameters, left.typeParameters);
10371+
// We just use the type parameter defaults from the first signature
10372+
}
1033710373
const declaration = left.declaration;
10338-
const params = combineUnionParameters(left, right);
10339-
const thisParam = combineUnionThisParam(left.thisParameter, right.thisParameter);
10374+
const params = combineUnionParameters(left, right, paramMapper);
10375+
const thisParam = combineUnionThisParam(left.thisParameter, right.thisParameter, paramMapper);
1034010376
const minArgCount = Math.max(left.minArgumentCount, right.minArgumentCount);
1034110377
const result = createSignature(
1034210378
declaration,
10343-
left.typeParameters || right.typeParameters,
10379+
typeParams,
1034410380
thisParam,
1034510381
params,
1034610382
/*resolvedReturnType*/ undefined,
@@ -10349,6 +10385,9 @@ namespace ts {
1034910385
(left.flags | right.flags) & SignatureFlags.PropagatingFlags
1035010386
);
1035110387
result.unionSignatures = concatenate(left.unionSignatures || [left], [right]);
10388+
if (paramMapper) {
10389+
result.mapper = left.mapper && left.unionSignatures ? combineTypeMappers(left.mapper, paramMapper) : paramMapper;
10390+
}
1035210391
return result;
1035310392
}
1035410393

@@ -11882,7 +11921,7 @@ namespace ts {
1188211921
return errorType;
1188311922
}
1188411923
let type = signature.target ? instantiateType(getReturnTypeOfSignature(signature.target), signature.mapper) :
11885-
signature.unionSignatures ? getUnionType(map(signature.unionSignatures, getReturnTypeOfSignature), UnionReduction.Subtype) :
11924+
signature.unionSignatures ? instantiateType(getUnionType(map(signature.unionSignatures, getReturnTypeOfSignature), UnionReduction.Subtype), signature.mapper) :
1188611925
getReturnTypeFromAnnotation(signature.declaration!) ||
1188711926
(nodeIsMissing((<FunctionLikeDeclaration>signature.declaration).body) ? anyType : getReturnTypeFromBody(<FunctionLikeDeclaration>signature.declaration));
1188811927
if (signature.flags & SignatureFlags.IsInnerCallChain) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
tests/cases/compiler/unionOfClassCalls.ts(28,5): error TS2349: This expression is not callable.
2+
Each member of the union type '{ (callbackfn: (previousValue: number, currentValue: number, currentIndex: number, array: number[]) => number): number; (callbackfn: (previousValue: number, currentValue: number, currentIndex: number, array: number[]) => number, initialValue: number): number; <U>(callbackfn: (previousValue: U, currentValue: number, currentIndex: number, array: number[]) => U, initialValue: U): U; } | { (callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string): string; (callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string, initialValue: string): string; <U>(callbackfn: (previousValue: U, currentValue: string, currentIndex: number, array: string[]) => U, initialValue: U): U; }' has signatures, but none of those signatures are compatible with each other.
3+
4+
5+
==== tests/cases/compiler/unionOfClassCalls.ts (1 errors) ====
6+
// from https://github.com/microsoft/TypeScript/issues/30717
7+
declare class Test<T> {
8+
obj: T;
9+
get<K extends keyof T>(k: K): T[K];
10+
}
11+
12+
interface A { t: "A" }
13+
interface B { t: "B" }
14+
15+
declare const tmp: Test<A> | Test<B>;
16+
17+
switch (tmp.get('t')) {
18+
case 'A': break;
19+
case 'B': break;
20+
}
21+
22+
// from https://github.com/microsoft/TypeScript/issues/36390
23+
24+
const arr: number[] | string[] = []; // Works with Array<number | string>
25+
const arr1: number[] = [];
26+
const arr2: string[] = [];
27+
28+
arr.map((a: number | string, index: number) => {
29+
return index
30+
})
31+
32+
// This case still doesn't work because `reduce` has multiple overloads :(
33+
arr.reduce((acc: Array<string>, a: number | string, index: number) => {
34+
~~~~~~
35+
!!! error TS2349: This expression is not callable.
36+
!!! error TS2349: Each member of the union type '{ (callbackfn: (previousValue: number, currentValue: number, currentIndex: number, array: number[]) => number): number; (callbackfn: (previousValue: number, currentValue: number, currentIndex: number, array: number[]) => number, initialValue: number): number; <U>(callbackfn: (previousValue: U, currentValue: number, currentIndex: number, array: number[]) => U, initialValue: U): U; } | { (callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string): string; (callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string, initialValue: string): string; <U>(callbackfn: (previousValue: U, currentValue: string, currentIndex: number, array: string[]) => U, initialValue: U): U; }' has signatures, but none of those signatures are compatible with each other.
37+
return []
38+
}, [])
39+
40+
arr.forEach((a: number | string, index: number) => {
41+
return index
42+
})
43+
44+
arr1.map((a: number, index: number) => {
45+
return index
46+
})
47+
48+
arr1.reduce((acc: number[], a: number, index: number) => {
49+
return [a]
50+
}, [])
51+
52+
arr1.forEach((a: number, index: number) => {
53+
return index
54+
})
55+
arr2.map((a: string, index: number) => {
56+
return index
57+
})
58+
59+
arr2.reduce((acc: string[], a: string, index: number) => {
60+
return []
61+
}, [])
62+
63+
arr2.forEach((a: string, index: number) => {
64+
return index
65+
})
66+
67+
// from https://github.com/microsoft/TypeScript/issues/36307
68+
69+
declare class Foo {
70+
doThing(): Promise<this>
71+
}
72+
73+
declare class Bar extends Foo {
74+
bar: number;
75+
}
76+
declare class Baz extends Foo {
77+
baz: number;
78+
}
79+
80+
declare var a: Bar | Baz;
81+
// note, you must annotate `result` for now
82+
a.doThing().then((result: Bar | Baz) => {
83+
// whatever
84+
});
85+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
//// [unionOfClassCalls.ts]
2+
// from https://github.com/microsoft/TypeScript/issues/30717
3+
declare class Test<T> {
4+
obj: T;
5+
get<K extends keyof T>(k: K): T[K];
6+
}
7+
8+
interface A { t: "A" }
9+
interface B { t: "B" }
10+
11+
declare const tmp: Test<A> | Test<B>;
12+
13+
switch (tmp.get('t')) {
14+
case 'A': break;
15+
case 'B': break;
16+
}
17+
18+
// from https://github.com/microsoft/TypeScript/issues/36390
19+
20+
const arr: number[] | string[] = []; // Works with Array<number | string>
21+
const arr1: number[] = [];
22+
const arr2: string[] = [];
23+
24+
arr.map((a: number | string, index: number) => {
25+
return index
26+
})
27+
28+
// This case still doesn't work because `reduce` has multiple overloads :(
29+
arr.reduce((acc: Array<string>, a: number | string, index: number) => {
30+
return []
31+
}, [])
32+
33+
arr.forEach((a: number | string, index: number) => {
34+
return index
35+
})
36+
37+
arr1.map((a: number, index: number) => {
38+
return index
39+
})
40+
41+
arr1.reduce((acc: number[], a: number, index: number) => {
42+
return [a]
43+
}, [])
44+
45+
arr1.forEach((a: number, index: number) => {
46+
return index
47+
})
48+
arr2.map((a: string, index: number) => {
49+
return index
50+
})
51+
52+
arr2.reduce((acc: string[], a: string, index: number) => {
53+
return []
54+
}, [])
55+
56+
arr2.forEach((a: string, index: number) => {
57+
return index
58+
})
59+
60+
// from https://github.com/microsoft/TypeScript/issues/36307
61+
62+
declare class Foo {
63+
doThing(): Promise<this>
64+
}
65+
66+
declare class Bar extends Foo {
67+
bar: number;
68+
}
69+
declare class Baz extends Foo {
70+
baz: number;
71+
}
72+
73+
declare var a: Bar | Baz;
74+
// note, you must annotate `result` for now
75+
a.doThing().then((result: Bar | Baz) => {
76+
// whatever
77+
});
78+
79+
80+
//// [unionOfClassCalls.js]
81+
"use strict";
82+
switch (tmp.get('t')) {
83+
case 'A': break;
84+
case 'B': break;
85+
}
86+
// from https://github.com/microsoft/TypeScript/issues/36390
87+
var arr = []; // Works with Array<number | string>
88+
var arr1 = [];
89+
var arr2 = [];
90+
arr.map(function (a, index) {
91+
return index;
92+
});
93+
// This case still doesn't work because `reduce` has multiple overloads :(
94+
arr.reduce(function (acc, a, index) {
95+
return [];
96+
}, []);
97+
arr.forEach(function (a, index) {
98+
return index;
99+
});
100+
arr1.map(function (a, index) {
101+
return index;
102+
});
103+
arr1.reduce(function (acc, a, index) {
104+
return [a];
105+
}, []);
106+
arr1.forEach(function (a, index) {
107+
return index;
108+
});
109+
arr2.map(function (a, index) {
110+
return index;
111+
});
112+
arr2.reduce(function (acc, a, index) {
113+
return [];
114+
}, []);
115+
arr2.forEach(function (a, index) {
116+
return index;
117+
});
118+
// note, you must annotate `result` for now
119+
a.doThing().then(function (result) {
120+
// whatever
121+
});

0 commit comments

Comments
 (0)