Code review

Author: Andy Hanson

src/compiler/checker.ts

@@ -7429,7 +7429,7 @@ namespace ts {
 
         // Add the given types to the given type set. Order is preserved, duplicates are removed,
         // and nested types of the given kind are flattened into the set.
-        function addTypesToUnion(typeSet: TypeSet, types: Type[]) {
+        function addTypesToUnion(typeSet: TypeSet, types: ReadonlyArray<Type>) {
             for (const type of types) {
                 addTypeToUnion(typeSet, type);
             }
@@ -7504,7 +7504,7 @@ namespace ts {
         // expression constructs such as array literals and the || and ?: operators). Named types can
         // circularly reference themselves and therefore cannot be subtype reduced during their declaration.
         // For example, "type Item = string | (() => Item" is a named type that circularly references itself.
-        function getUnionType(types: Type[], subtypeReduction?: boolean, aliasSymbol?: Symbol, aliasTypeArguments?: Type[]): Type {
+        function getUnionType(types: ReadonlyArray<Type>, subtypeReduction?: boolean, aliasSymbol?: Symbol, aliasTypeArguments?: Type[]): Type {
             if (types.length === 0) {
                 return neverType;
             }
@@ -10434,11 +10434,6 @@ namespace ts {
             return symbol;
         }
 
-        function createCombinedSymbolWithType(sources: ReadonlyArray<Symbol>, type: Type): Symbol {
-            // This function is currently only used for erroneous overloads, so it's good enough to just use the first source.
-            return createSymbolWithType(first(sources), type);
-        }
-
         function transformTypeOfMembers(type: Type, f: (propertyType: Type) => Type) {
             const members = createSymbolTable();
             for (const property of getPropertiesOfObjectType(type)) {
@@ -16526,49 +16521,22 @@ namespace ts {
             hasCandidatesOutArray: boolean,
         ): Signature {
             Debug.assert(candidates.length > 0); // Else should not have called this.
-
             // Normally we will combine overloads. Skip this if they have type parameters since that's hard to combine.
             // Don't do this if there is a `candidatesOutArray`,
             // because then we want the chosen best candidate to be one of the overloads, not a combination.
-            if (!hasCandidatesOutArray && candidates.length > 1 && !candidates.some(c => !!c.typeParameters)) {
-                return createUnionOfSignaturesForOverloadFailure(candidates);
-            }
-
-            // Pick the longest signature. This way we can get a contextual type for cases like:
-            //     declare function f(a: { xa: number; xb: number; }, b: number);
-            //     f({ |
-            // Also, use explicitly-supplied type arguments if they are provided, so we can get a contextual signature in cases like:
-            //     declare function f<T>(k: keyof T);
-            //     f<Foo>("
-            const bestIndex = getLongestCandidateIndex(candidates, apparentArgumentCount === undefined ? args.length : apparentArgumentCount);
-            const candidate = candidates[bestIndex];
-
-            const { typeParameters } = candidate;
-            if (typeParameters && callLikeExpressionMayHaveTypeArguments(node) && node.typeArguments) {
-                const typeArguments = node.typeArguments.map(getTypeOfNode);
-                while (typeArguments.length > typeParameters.length) {
-                    typeArguments.pop();
-                }
-                while (typeArguments.length < typeParameters.length) {
-                    typeArguments.push(getDefaultTypeArgumentType(isInJavaScriptFile(node)));
-                }
-
-                const instantiated = createSignatureInstantiation(candidate, typeArguments);
-                candidates[bestIndex] = instantiated;
-                return instantiated;
-            }
-
-            return candidate;
+            return hasCandidatesOutArray || candidates.length === 1 || candidates.some(c => !!c.typeParameters)
+                ? pickLongestCandidateSignature(node, candidates, args)
+                : createUnionOfSignaturesForOverloadFailure(candidates);
         }
 
         function createUnionOfSignaturesForOverloadFailure(candidates: ReadonlyArray<Signature>): Signature {
             const thisParameters = mapDefined(candidates, c => c.thisParameter);
             let thisParameter: Symbol | undefined;
             if (thisParameters.length) {
-                thisParameter = createCombinedSymbolWithType(thisParameters, getUnionType(thisParameters.map(getTypeOfParameter), /*subtypeReduction*/ true));
+                thisParameter = createCombinedSymbolFromTypes(thisParameters, thisParameters.map(getTypeOfParameter));
             }
 
-            const { min: minArgumentCount, max: maxNonRestParam }  = minAndMax(candidates, getNumNonRestParameters);
+            const { min: minArgumentCount, max: maxNonRestParam } = minAndMax(candidates, getNumNonRestParameters);
             const hasRestParameter = candidates.some(c => c.hasRestParameter);
             const hasLiteralTypes = candidates.some(c => c.hasLiteralTypes);
             const parameters: ts.Symbol[] = [];
@@ -16577,15 +16545,14 @@ namespace ts {
                     i < parameters.length - 1 ? parameters[i] : last(parameters) :
                     i < parameters.length ? parameters[i] : undefined);
                 Debug.assert(symbols.length !== 0);
-                const types = mapDefined(candidates, candidate => tryGetTypeAtPosition(candidate, i));
-                parameters.push(createCombinedSymbolWithType(symbols, getUnionType(types, /*subtypeReduction*/ true)));
+                parameters.push(createCombinedSymbolFromTypes(symbols, mapDefined(candidates, candidate => tryGetTypeAtPosition(candidate, i))));
             }
 
             if (hasRestParameter) {
                 const symbols = mapDefined(candidates, c => c.hasRestParameter ? last(c.parameters) : undefined);
                 Debug.assert(symbols.length !== 0);
                 const type = createArrayType(getUnionType(mapDefined(candidates, tryGetRestTypeOfSignature), /*subtypeReduction*/ true));
-                parameters.push(createCombinedSymbolWithType(symbols, type));
+                parameters.push(createCombinedSymbolForOverloadFailure(symbols, type));
             }
 
             return createSignature(
@@ -16600,7 +16567,44 @@ namespace ts {
                 hasLiteralTypes);
         }
 
-        function getLongestCandidateIndex(candidates: Signature[], argsCount: number): number {
+        function createCombinedSymbolFromTypes(sources: ReadonlyArray<Symbol>, types: ReadonlyArray<Type>): Symbol {
+            return createCombinedSymbolForOverloadFailure(sources, getUnionType(types, /*subtypeReduction*/ true));
+        }
+
+        function createCombinedSymbolForOverloadFailure(sources: ReadonlyArray<Symbol>, type: Type): Symbol {
+            // This function is currently only used for erroneous overloads, so it's good enough to just use the first source.
+            return createSymbolWithType(first(sources), type);
+        }
+
+        function pickLongestCandidateSignature(node: CallLikeExpression, candidates: Signature[], args: ReadonlyArray<Expression>): Signature {
+            // Pick the longest signature. This way we can get a contextual type for cases like:
+            //     declare function f(a: { xa: number; xb: number; }, b: number);
+            //     f({ |
+            // Also, use explicitly-supplied type arguments if they are provided, so we can get a contextual signature in cases like:
+            //     declare function f<T>(k: keyof T);
+            //     f<Foo>("
+            const bestIndex = getLongestCandidateIndex(candidates, apparentArgumentCount === undefined ? args.length : apparentArgumentCount);
+            const candidate = candidates[bestIndex];
+
+            const { typeParameters } = candidate;
+            if (typeParameters && callLikeExpressionMayHaveTypeArguments(node) && node.typeArguments) {
+                const typeArguments = node.typeArguments.map(getTypeOfNode);
+                while (typeArguments.length > typeParameters.length) {
+                    typeArguments.pop();
+                }
+                while (typeArguments.length < typeParameters.length) {
+                    typeArguments.push(getDefaultTypeArgumentType(isInJavaScriptFile(node)));
+                }
+
+                const instantiated = createSignatureInstantiation(candidate, typeArguments);
+                candidates[bestIndex] = instantiated;
+                return instantiated;
+            }
+
+            return candidate;
+        }
+
+        function getLongestCandidateIndex(candidates: ReadonlyArray<Signature>, argsCount: number): number {
             let maxParamsIndex = -1;
             let maxParams = -1;