Infer from annotated parameters of context sensitive functions in the first inference pass #56460
Conversation
… first inference pass
…tated-params-of-context-sensitive-functions
…tated-params-of-context-sensitive-functions
…tated-params-of-context-sensitive-functions
|
@jakebailey could you run the extended test suite here? :) |
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
@typescript-bot pack this |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
src/compiler/checker.ts
Outdated
| if (checkMode & CheckMode.Inferential) { | ||
| const contextualSignature = getContextualSignature(node); | ||
| if (contextualSignature) { | ||
| inferFromAnnotatedParameters(node, contextualSignature, getInferenceContext(node)!); | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it not a little weird for getInferenceContext and inferFromAnnotatedParameters to get called here? I'm not finding other examples where we're doing this sort of thing outside of a function intended to contextually check (which this function is not, contextuallyCheckFunctionExpressionOrObjectLiteralMethod is).
There was a problem hiding this comment.
hm, this might be the first time this is called like it but the placement of those calls here is what fixes the bug conceptually - as in, for the added test cases it's important to infer before contextually checking those functions
Contextual check of functions can fix "requested" inference type parameters and that creates an ordering issue for #56459 as the parameter type "request" can happen before inferFromAnnotatedParameters even has a chance to be called on the other function that comes later in the source code.
And the similar problem happens when it comes to #60047 . The conditional type asks for the inferred type of the type parameter and since by that time the inferFromAnnotatedParameters was not called it resolves to its constraint. Then the arguments fail to be typechecked against current parameter types in getSignatureApplicabilityError so the algorithm returns, never reaching the second inference pass that starts to include context-sensitive parameters.
So if those issues are meant to be fixed it's absolutely necessary to call inferFromAnnotatedParameters within the first inference pass and that doesn't check context-sensitive functions, that's deferred to the second inference pass. I don't see how to even attempt to fix this in any other way given the known limitations around context-sensitive expressions and their associated implementation details
…tated-params-of-context-sensitive-functions
fixes #56459
fixes #60047
fixes #60648