Merge pull request #2356 from Microsoft/typeParameterFixing

Make sure type parameters stay fixed throughout the inference process

Author: JsonFreeman

src/compiler/checker.ts

@@ -79,8 +79,7 @@ module ts {
         let emptyObjectType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
         let anyFunctionType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
         let noConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
-        let inferenceFailureType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
-
+        
         let anySignature = createSignature(undefined, undefined, emptyArray, anyType, 0, false, false);
         let unknownSignature = createSignature(undefined, undefined, emptyArray, unknownType, 0, false, false);
 
@@ -3514,6 +3513,7 @@ module ts {
             return t => {
                 for (let i = 0; i < context.typeParameters.length; i++) {
                     if (t === context.typeParameters[i]) {
+                        context.inferences[i].isFixed = true;
                         return getInferredType(context, i);
                     }
                 }
@@ -4372,8 +4372,11 @@ module ts {
         }
 
         function reportNoCommonSupertypeError(types: Type[], errorLocation: Node, errorMessageChainHead: DiagnosticMessageChain): void {
+            // The downfallType/bestSupertypeDownfallType is the first type that caused a particular candidate
+            // to not be the common supertype. So if it weren't for this one downfallType (and possibly others),
+            // the type in question could have been the common supertype.
             let bestSupertype: Type;
-            let bestSupertypeDownfallType: Type; // The type that caused bestSupertype not to be the common supertype
+            let bestSupertypeDownfallType: Type;
             let bestSupertypeScore = 0;
 
             for (let i = 0; i < types.length; i++) {
@@ -4388,6 +4391,8 @@ module ts {
                     }
                 }
 
+                Debug.assert(!!downfallType, "If there is no common supertype, each type should have a downfallType");
+
                 if (score > bestSupertypeScore) {
                     bestSupertype = types[i];
                     bestSupertypeDownfallType = downfallType;
@@ -4570,13 +4575,12 @@ module ts {
         function createInferenceContext(typeParameters: TypeParameter[], inferUnionTypes: boolean): InferenceContext {
             let inferences: TypeInferences[] = [];
             for (let unused of typeParameters) {
-                inferences.push({ primary: undefined, secondary: undefined });
+                inferences.push({ primary: undefined, secondary: undefined, isFixed: false });
             }
             return {
-                typeParameters: typeParameters,
-                inferUnionTypes: inferUnionTypes,
-                inferenceCount: 0,
-                inferences: inferences,
+                typeParameters,
+                inferUnionTypes,
+                inferences,
                 inferredTypes: new Array(typeParameters.length),
             };
         }
@@ -4622,11 +4626,21 @@ module ts {
                     for (let i = 0; i < typeParameters.length; i++) {
                         if (target === typeParameters[i]) {
                             let inferences = context.inferences[i];
-                            let candidates = inferiority ?
-                                inferences.secondary || (inferences.secondary = []) :
-                                inferences.primary || (inferences.primary = []);
-                            if (!contains(candidates, source)) candidates.push(source);
-                            break;
+                            if (!inferences.isFixed) {
+                                // Any inferences that are made to a type parameter in a union type are inferior
+                                // to inferences made to a flat (non-union) type. This is because if we infer to
+                                // T | string[], we really don't know if we should be inferring to T or not (because
+                                // the correct constituent on the target side could be string[]). Therefore, we put
+                                // such inferior inferences into a secondary bucket, and only use them if the primary
+                                // bucket is empty.
+                                let candidates = inferiority ?
+                                    inferences.secondary || (inferences.secondary = []) :
+                                    inferences.primary || (inferences.primary = []);
+                                if (!contains(candidates, source)) {
+                                    candidates.push(source);
+                                }
+                            }
+                            return;
                         }
                     }
                 }
@@ -4732,21 +4746,35 @@ module ts {
 
         function getInferredType(context: InferenceContext, index: number): Type {
             let inferredType = context.inferredTypes[index];
+            let inferenceSucceeded: boolean;
             if (!inferredType) {
                 let inferences = getInferenceCandidates(context, index);
                 if (inferences.length) {
-                    // Infer widened union or supertype, or the undefined type for no common supertype
+                    // Infer widened union or supertype, or the unknown type for no common supertype
                     let unionOrSuperType = context.inferUnionTypes ? getUnionType(inferences) : getCommonSupertype(inferences);
-                    inferredType = unionOrSuperType ? getWidenedType(unionOrSuperType) : inferenceFailureType;
+                    inferredType = unionOrSuperType ? getWidenedType(unionOrSuperType) : unknownType;
+                    inferenceSucceeded = !!unionOrSuperType;
                 }
                 else {
-                    // Infer the empty object type when no inferences were made
+                    // Infer the empty object type when no inferences were made. It is important to remember that
+                    // in this case, inference still succeeds, meaning there is no error for not having inference
+                    // candidates. An inference error only occurs when there are *conflicting* candidates, i.e.
+                    // candidates with no common supertype.
                     inferredType = emptyObjectType;
+                    inferenceSucceeded = true;
                 }
-                if (inferredType !== inferenceFailureType) {
+
+                // Only do the constraint check if inference succeeded (to prevent cascading errors)
+                if (inferenceSucceeded) {
                     let constraint = getConstraintOfTypeParameter(context.typeParameters[index]);
                     inferredType = constraint && !isTypeAssignableTo(inferredType, constraint) ? constraint : inferredType;
                 }
+                else if (context.failedTypeParameterIndex === undefined || context.failedTypeParameterIndex > index) {
+                    // If inference failed, it is necessary to record the index of the failed type parameter (the one we are on).
+                    // It might be that inference has already failed on a later type parameter on a previous call to inferTypeArguments.
+                    // So if this failure is on preceding type parameter, this type parameter is the new failure index.
+                    context.failedTypeParameterIndex = index;
+                }
                 context.inferredTypes[index] = inferredType;
             }
             return inferredType;
@@ -6343,11 +6371,32 @@ module ts {
             return getSignatureInstantiation(signature, getInferredTypes(context));
         }
 
-        function inferTypeArguments(signature: Signature, args: Expression[], excludeArgument: boolean[]): InferenceContext {
+        function inferTypeArguments(signature: Signature, args: Expression[], excludeArgument: boolean[], context: InferenceContext): void {
             let typeParameters = signature.typeParameters;
-            let context = createInferenceContext(typeParameters, /*inferUnionTypes*/ false);
             let inferenceMapper = createInferenceMapper(context);
 
+            // Clear out all the inference results from the last time inferTypeArguments was called on this context
+            for (let i = 0; i < typeParameters.length; i++) {
+                // As an optimization, we don't have to clear (and later recompute) inferred types
+                // for type parameters that have already been fixed on the previous call to inferTypeArguments.
+                // It would be just as correct to reset all of them. But then we'd be repeating the same work
+                // for the type parameters that were fixed, namely the work done by getInferredType.
+                if (!context.inferences[i].isFixed) {
+                    context.inferredTypes[i] = undefined;
+                }
+            }
+
+            // On this call to inferTypeArguments, we may get more inferences for certain type parameters that were not
+            // fixed last time. This means that a type parameter that failed inference last time may succeed this time,
+            // or vice versa. Therefore, the failedTypeParameterIndex is useless if it points to an unfixed type parameter,
+            // because it may change. So here we reset it. However, getInferredType will not revisit any type parameters
+            // that were previously fixed. So if a fixed type parameter failed previously, it will fail again because
+            // it will contain the exact same set of inferences. So if we reset the index from a fixed type parameter,
+            // we will lose information that we won't recover this time around.
+            if (context.failedTypeParameterIndex !== undefined && !context.inferences[context.failedTypeParameterIndex].isFixed) {
+                context.failedTypeParameterIndex = undefined;
+            }
+
             // We perform two passes over the arguments. In the first pass we infer from all arguments, but use
             // wildcards for all context sensitive function expressions.
             for (let i = 0; i < args.length; i++) {
@@ -6382,18 +6431,7 @@ module ts {
                 }
             }
 
-            let inferredTypes = getInferredTypes(context);
-            // Inference has failed if the inferenceFailureType type is in list of inferences
-            context.failedTypeParameterIndex = indexOf(inferredTypes, inferenceFailureType);
-
-            // Wipe out the inferenceFailureType from the array so that error recovery can work properly
-            for (let i = 0; i < inferredTypes.length; i++) {
-                if (inferredTypes[i] === inferenceFailureType) {
-                    inferredTypes[i] = unknownType;
-                }
-            }
-
-            return context;
+            getInferredTypes(context);
         }
 
         function checkTypeArguments(signature: Signature, typeArguments: TypeNode[], typeArgumentResultTypes: Type[], reportErrors: boolean): boolean {
@@ -6627,15 +6665,17 @@ module ts {
             return resolveErrorCall(node);
 
             function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>) {
-                for (let current of candidates) {
-                    if (!hasCorrectArity(node, args, current)) {
+                for (let originalCandidate of candidates) {
+                    if (!hasCorrectArity(node, args, originalCandidate)) {
                         continue;
                     }
-
-                    let originalCandidate = current;
-                    let inferenceResult: InferenceContext;
+                    
                     let candidate: Signature;
                     let typeArgumentsAreValid: boolean;
+                    let inferenceContext = originalCandidate.typeParameters
+                        ? createInferenceContext(originalCandidate.typeParameters, /*inferUnionTypes*/ false)
+                        : undefined;
+
                     while (true) {
                         candidate = originalCandidate;
                         if (candidate.typeParameters) {
@@ -6645,9 +6685,9 @@ module ts {
                                 typeArgumentsAreValid = checkTypeArguments(candidate, typeArguments, typeArgumentTypes, /*reportErrors*/ false)
                             }
                             else {
-                                inferenceResult = inferTypeArguments(candidate, args, excludeArgument);
-                                typeArgumentsAreValid = inferenceResult.failedTypeParameterIndex < 0;
-                                typeArgumentTypes = inferenceResult.inferredTypes;
+                                inferTypeArguments(candidate, args, excludeArgument, inferenceContext);
+                                typeArgumentsAreValid = inferenceContext.failedTypeParameterIndex === undefined;
+                                typeArgumentTypes = inferenceContext.inferredTypes;
                             }
                             if (!typeArgumentsAreValid) {
                                 break;
@@ -6677,7 +6717,7 @@ module ts {
                         else {
                             candidateForTypeArgumentError = originalCandidate;
                             if (!typeArguments) {
-                                resultOfFailedInference = inferenceResult;
+                                resultOfFailedInference = inferenceContext;
                             }
                         }
                     }

src/compiler/types.ts

@@ -1487,11 +1487,15 @@ module ts {
         (t: Type): Type;
     }
 
+    // @internal
     export interface TypeInferences {
         primary: Type[];    // Inferences made directly to a type parameter
         secondary: Type[];  // Inferences made to a type parameter in a union type
+        isFixed: boolean;   // Whether the type parameter is fixed, as defined in section 4.12.2 of the TypeScript spec
+                            // If a type parameter is fixed, no more inferences can be made for the type parameter
     }
 
+    // @internal
     export interface InferenceContext {
         typeParameters: TypeParameter[];    // Type parameters for which inferences are made
         inferUnionTypes: boolean;           // Infer union types for disjoint candidates (otherwise undefinedType)

tests/baselines/reference/APISample_compile.js

@@ -1176,17 +1176,6 @@ declare module "typescript" {
     interface TypeMapper {
         (t: Type): Type;
     }
-    interface TypeInferences {
-        primary: Type[];
-        secondary: Type[];
-    }
-    interface InferenceContext {
-        typeParameters: TypeParameter[];
-        inferUnionTypes: boolean;
-        inferences: TypeInferences[];
-        inferredTypes: Type[];
-        failedTypeParameterIndex?: number;
-    }
     interface DiagnosticMessage {
         key: string;
         category: DiagnosticCategory;

tests/baselines/reference/APISample_compile.types

@@ -3769,38 +3769,6 @@ declare module "typescript" {
 >t : Type
 >Type : Type
 >Type : Type
-    }
-    interface TypeInferences {
->TypeInferences : TypeInferences
-
-        primary: Type[];
->primary : Type[]
->Type : Type
-
-        secondary: Type[];
->secondary : Type[]
->Type : Type
-    }
-    interface InferenceContext {
->InferenceContext : InferenceContext
-
-        typeParameters: TypeParameter[];
->typeParameters : TypeParameter[]
->TypeParameter : TypeParameter
-
-        inferUnionTypes: boolean;
->inferUnionTypes : boolean
-
-        inferences: TypeInferences[];
->inferences : TypeInferences[]
->TypeInferences : TypeInferences
-
-        inferredTypes: Type[];
->inferredTypes : Type[]
->Type : Type
-
-        failedTypeParameterIndex?: number;
->failedTypeParameterIndex : number
     }
     interface DiagnosticMessage {
 >DiagnosticMessage : DiagnosticMessage

tests/baselines/reference/APISample_linter.js

@@ -1207,17 +1207,6 @@ declare module "typescript" {
     interface TypeMapper {
         (t: Type): Type;
     }
-    interface TypeInferences {
-        primary: Type[];
-        secondary: Type[];
-    }
-    interface InferenceContext {
-        typeParameters: TypeParameter[];
-        inferUnionTypes: boolean;
-        inferences: TypeInferences[];
-        inferredTypes: Type[];
-        failedTypeParameterIndex?: number;
-    }
     interface DiagnosticMessage {
         key: string;
         category: DiagnosticCategory;

tests/baselines/reference/APISample_linter.types

@@ -3915,38 +3915,6 @@ declare module "typescript" {
 >t : Type
 >Type : Type
 >Type : Type
-    }
-    interface TypeInferences {
->TypeInferences : TypeInferences
-
-        primary: Type[];
->primary : Type[]
->Type : Type
-
-        secondary: Type[];
->secondary : Type[]
->Type : Type
-    }
-    interface InferenceContext {
->InferenceContext : InferenceContext
-
-        typeParameters: TypeParameter[];
->typeParameters : TypeParameter[]
->TypeParameter : TypeParameter
-
-        inferUnionTypes: boolean;
->inferUnionTypes : boolean
-
-        inferences: TypeInferences[];
->inferences : TypeInferences[]
->TypeInferences : TypeInferences
-
-        inferredTypes: Type[];
->inferredTypes : Type[]
->Type : Type
-
-        failedTypeParameterIndex?: number;
->failedTypeParameterIndex : number
     }
     interface DiagnosticMessage {
 >DiagnosticMessage : DiagnosticMessage

tests/baselines/reference/APISample_transform.js

@@ -1208,17 +1208,6 @@ declare module "typescript" {
     interface TypeMapper {
         (t: Type): Type;
     }
-    interface TypeInferences {
-        primary: Type[];
-        secondary: Type[];
-    }
-    interface InferenceContext {
-        typeParameters: TypeParameter[];
-        inferUnionTypes: boolean;
-        inferences: TypeInferences[];
-        inferredTypes: Type[];
-        failedTypeParameterIndex?: number;
-    }
     interface DiagnosticMessage {
         key: string;
         category: DiagnosticCategory;