Improve inference for context sensitive functions in object and array literal arguments #48538
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21729,6 +21729,9 @@ namespace ts { | |
| const inference = inferences[i]; | ||
| if (t === inference.typeParameter) { | ||
| if (fix && !inference.isFixed) { | ||
| // Before we commit to a particular inference (and thus lock out any further inferences), | ||
| // we infer from any intra-expression inference sites we have collected. | ||
| inferFromIntraExpressionSites(context); | ||
| clearCachedInferences(inferences); | ||
| inference.isFixed = true; | ||
| } | ||
|
|
@@ -21746,6 +21749,37 @@ namespace ts { | |
| } | ||
| } | ||
|
|
||
| function addIntraExpressionInferenceSite(context: InferenceContext, node: Expression | MethodDeclaration, type: Type) { | ||
| (context.intraExpressionInferenceSites ??= []).push({ node, type }); | ||
| } | ||
|
|
||
| // We collect intra-expression inference sites within object and array literals to handle cases where | ||
| // inferred types flow between context sensitive element expressions. For example: | ||
| // | ||
| // declare function foo<T>(arg: [(n: number) => T, (x: T) => void]): void; | ||
| // foo([_a => 0, n => n.toFixed()]); | ||
| // | ||
| // Above, both arrow functions in the tuple argument are context sensitive, thus both are omitted from the | ||
| // pass that collects inferences from the non-context sensitive parts of the arguments. In the subsequent | ||
| // pass where nothing is omitted, we need to commit to an inference for T in order to contextually type the | ||
| // parameter in the second arrow function, but we want to first infer from the return type of the first | ||
| // arrow function. This happens automatically when the arrow functions are discrete arguments (because we | ||
| // infer from each argument before processing the next), but when the arrow functions are elements of an | ||
| // object or array literal, we need to perform intra-expression inferences early. | ||
| function inferFromIntraExpressionSites(context: InferenceContext) { | ||
| if (context.intraExpressionInferenceSites) { | ||
| for (const { node, type } of context.intraExpressionInferenceSites) { | ||
| const contextualType = node.kind === SyntaxKind.MethodDeclaration ? | ||
| getContextualTypeForObjectLiteralMethod(node as MethodDeclaration, ContextFlags.NoConstraints) : | ||
| getContextualType(node, ContextFlags.NoConstraints); | ||
| if (contextualType) { | ||
| inferTypes(context.inferences, type, contextualType); | ||
|
There was a problem hiding this comment. So this makes me think that we'll be redoing a lot of inferences - for a given inference set, we'll infer from each of the inner properties to their matching contextual type, then we'll do the outer inference for the object as a whole, which, in so doing, will redo these inferences again (and hopefully come to the same results). It's making me think that maybe we should have some kind of visited list for inferences in a given inference context, something that says "we've already inferred from A to B in this context, no need to do it again" - essentially There was a problem hiding this comment.
I think (see my comment here) that that's why this is only done when we're about to fix (which are cases that are broken today). So in the cases which don't work today, I think we'll effectively do inference twice in the worst-case (cases where you need to fix a type parameter on every context-sensitive expression - should be rare). Otherwise, it should be the same amount of inference as before, the only new work is collecting the context-sensitive inner expressions). There was a problem hiding this comment. I mean, storing these locations and doing the inference later will always result in inference occurring - that inner inference occurring simply changes the order of inferences (and allows some inferences to be locked in before sibling member inferences begin). We'll still ultimately back out to the expression as a whole and do inferences on its type, which will now sometimes have more inferences locked in from these inner inferences, and still do inference on that type and all its members, which includes all these inferences that we've already done. There was a problem hiding this comment. There will be some amount of redoing of inferences, but deferring the work at least ensures we only do it if there's a chance it's needed and the whole scenario is generally pretty rare. I'm not too concerned with the repetition that occurs when we infer from the entire expression, nor am I too concerned with the ordering (we already have out-of-sequence ordering because we first infer from context insensitive arguments). |
||
| } | ||
| } | ||
| context.intraExpressionInferenceSites = undefined; | ||
| } | ||
| } | ||
|
|
||
| function createInferenceInfo(typeParameter: TypeParameter): InferenceInfo { | ||
| return { | ||
| typeParameter, | ||
|
|
@@ -27408,6 +27442,11 @@ namespace ts { | |
| const type = checkExpressionForMutableLocation(e, checkMode, elementContextualType, forceTuple); | ||
| elementTypes.push(addOptionality(type, /*isProperty*/ true, hasOmittedExpression)); | ||
| elementFlags.push(hasOmittedExpression ? ElementFlags.Optional : ElementFlags.Required); | ||
| if (contextualType && someType(contextualType, isTupleLikeType) && checkMode && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) && isContextSensitive(e)) { | ||
| const inferenceContext = getInferenceContext(node); | ||
| Debug.assert(inferenceContext); // In CheckMode.Inferential we should always have an inference context | ||
| addIntraExpressionInferenceSite(inferenceContext, e, type); | ||
| } | ||
| } | ||
| } | ||
| if (inDestructuringPattern) { | ||
|
|
@@ -27625,6 +27664,14 @@ namespace ts { | |
| prop.target = member; | ||
| member = prop; | ||
| allPropertiesTable?.set(prop.escapedName, prop); | ||
|
|
||
| if (contextualType && checkMode && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) && | ||
| (memberDecl.kind === SyntaxKind.PropertyAssignment || memberDecl.kind === SyntaxKind.MethodDeclaration) && isContextSensitive(memberDecl)) { | ||
| const inferenceContext = getInferenceContext(node); | ||
| Debug.assert(inferenceContext); // In CheckMode.Inferential we should always have an inference context | ||
| const inferenceNode = memberDecl.kind === SyntaxKind.PropertyAssignment ? memberDecl.initializer : memberDecl; | ||
| addIntraExpressionInferenceSite(inferenceContext, inferenceNode, type); | ||
| } | ||
| } | ||
| else if (memberDecl.kind === SyntaxKind.SpreadAssignment) { | ||
| if (languageVersion < ScriptTarget.ES2015) { | ||
|
|
@@ -29727,34 +29774,36 @@ namespace ts { | |
| if (node.kind !== SyntaxKind.Decorator) { | ||
| const contextualType = getContextualType(node, every(signature.typeParameters, p => !!getDefaultFromTypeParameter(p)) ? ContextFlags.SkipBindingPatterns : ContextFlags.None); | ||
| if (contextualType) { | ||
| const inferenceTargetType = getReturnTypeOfSignature(signature); | ||
| if (couldContainTypeVariables(inferenceTargetType)) { | ||
| // We clone the inference context to avoid disturbing a resolution in progress for an | ||
| // outer call expression. Effectively we just want a snapshot of whatever has been | ||
| // inferred for any outer call expression so far. | ||
| const outerContext = getInferenceContext(node); | ||
| const outerMapper = getMapperFromContext(cloneInferenceContext(outerContext, InferenceFlags.NoDefault)); | ||
| const instantiatedType = instantiateType(contextualType, outerMapper); | ||
| // If the contextual type is a generic function type with a single call signature, we | ||
| // instantiate the type with its own type parameters and type arguments. This ensures that | ||
| // the type parameters are not erased to type any during type inference such that they can | ||
| // be inferred as actual types from the contextual type. For example: | ||
| // declare function arrayMap<T, U>(f: (x: T) => U): (a: T[]) => U[]; | ||
| // const boxElements: <A>(a: A[]) => { value: A }[] = arrayMap(value => ({ value })); | ||
| // Above, the type of the 'value' parameter is inferred to be 'A'. | ||
| const contextualSignature = getSingleCallSignature(instantiatedType); | ||
| const inferenceSourceType = contextualSignature && contextualSignature.typeParameters ? | ||
| getOrCreateTypeFromSignature(getSignatureInstantiationWithoutFillingInTypeArguments(contextualSignature, contextualSignature.typeParameters)) : | ||
| instantiatedType; | ||
| // Inferences made from return types have lower priority than all other inferences. | ||
| inferTypes(context.inferences, inferenceSourceType, inferenceTargetType, InferencePriority.ReturnType); | ||
| // Create a type mapper for instantiating generic contextual types using the inferences made | ||
| // from the return type. We need a separate inference pass here because (a) instantiation of | ||
| // the source type uses the outer context's return mapper (which excludes inferences made from | ||
| // outer arguments), and (b) we don't want any further inferences going into this context. | ||
| const returnContext = createInferenceContext(signature.typeParameters!, signature, context.flags); | ||
| const returnSourceType = instantiateType(contextualType, outerContext && outerContext.returnMapper); | ||
| inferTypes(returnContext.inferences, returnSourceType, inferenceTargetType); | ||
| context.returnMapper = some(returnContext.inferences, hasInferenceCandidates) ? getMapperFromContext(cloneInferredPartOfContext(returnContext)) : undefined; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -29768,7 +29817,7 @@ namespace ts { | |
| } | ||
|
|
||
| const thisType = getThisTypeOfSignature(signature); | ||
| if (thisType && couldContainTypeVariables(thisType)) { | ||
|
There was a problem hiding this comment. Are these new guards on There was a problem hiding this comment. Purely an optimization. |
||
| const thisArgumentNode = getThisArgumentOfCall(node); | ||
| inferTypes(context.inferences, getThisArgumentType(thisArgumentNode), thisType); | ||
| } | ||
|
|
@@ -29777,12 +29826,14 @@ namespace ts { | |
| const arg = args[i]; | ||
| if (arg.kind !== SyntaxKind.OmittedExpression && !(checkMode & CheckMode.IsForStringLiteralArgumentCompletions && hasSkipDirectInferenceFlag(arg))) { | ||
| const paramType = getTypeAtPosition(signature, i); | ||
| if (couldContainTypeVariables(paramType)) { | ||
| const argType = checkExpressionWithContextualType(arg, paramType, context, checkMode); | ||
| inferTypes(context.inferences, argType, paramType); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (restType && couldContainTypeVariables(restType)) { | ||
| const spreadType = getSpreadArgumentType(args, argCount, args.length, restType, context, checkMode); | ||
| inferTypes(context.inferences, spreadType, restType); | ||
| } | ||
|
|
@@ -34141,6 +34192,11 @@ namespace ts { | |
| context.contextualType = contextualType; | ||
| context.inferenceContext = inferenceContext; | ||
| const type = checkExpression(node, checkMode | CheckMode.Contextual | (inferenceContext ? CheckMode.Inferential : 0)); | ||
| // In CheckMode.Inferential we collect intra-expression inference sites to process before fixing any type | ||
| // parameters. This information is no longer needed after the call to checkExpression. | ||
| if (inferenceContext && inferenceContext.intraExpressionInferenceSites) { | ||
| inferenceContext.intraExpressionInferenceSites = undefined; | ||
| } | ||
| // We strip literal freshness when an appropriate contextual type is present such that contextually typed | ||
| // literals always preserve their literal types (otherwise they might widen during type inference). An alternative | ||
| // here would be to not mark contextually typed literals as fresh in the first place. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are multiple nested active inference contexts (eg,
foo(..., bar(..., baz(..., {...})))might we not want to add the expression to all inference contexts? This way if a type parameter flows fromfoointobarand thenbaz, but doesn't flow back out on construction (eg, because it's conditional'd away), we could still pick up the inner inference site.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, context sensitive arguments to an inner call might cause an outer type parameter to become fixed, but inferences from the inner argument expressions only occur through the return type of the inner argument expressions.