Revert #54477 but keep the tests #57160
Conversation
Discovered in microsoft#57117 The implementation should not `couldContainTypeVariables`--it's not intended a fast path, and should not be used in places where its unreliability can be observed. The tests stay, but with a note added that they should pass but do not.
typescript-bot
added
Author: Team
For Uncommitted Bug
There was a problem hiding this comment.
This is a minimal repro for the break, right?
| const signature = getSingleCallSignature(funcType); | ||
| if (signature) { | ||
| const returnType = getReturnTypeOfSignature(signature); | ||
| if (!signature.typeParameters || !couldContainTypeVariables(returnType)) { |
There was a problem hiding this comment.
I try to understand how to use couldContainTypeVariables appropriately and I wonder how this was different from how checkFunctionExpressionOrObjectLiteralMethod uses it here?
// The identityMapper object is used to indicate that function expressions are wildcards
if (checkMode && checkMode & CheckMode.SkipContextSensitive && isContextSensitive(node)) {
// Skip parameters, return signature with return type that retains noncontextual parts so inferences can still be drawn in an early stage
if (!getEffectiveReturnTypeNode(node) && !hasContextSensitiveParameters(node)) {
// Return plain anyFunctionType if there is no possibility we'll make inferences from the return type
const contextualSignature = getContextualSignature(node);
if (contextualSignature && couldContainTypeVariables(getReturnTypeOfSignature(contextualSignature))) {There was a problem hiding this comment.
I see how this is part of the inference algorithm and the check here was kinda not - but I still fail to understand how those 2 situations are fundamentally different so it's OK to call it within one context and not OK to call it within another.
There was a problem hiding this comment.
I'm not sure that other use is correct either...
There was a problem hiding this comment.
I also tried to repro the regression caused by this cause I wanted to add a test case for it but I failed. It has been mentioned that it was only visible in the IDE but I assume that this particular report was reported by a bot reporting tsc-related changes. I couldn't repro with either though.
5 participants
Discovered in #57117 for the 5.4 beta.
The implementation should not use
couldContainTypeVariables--it's intended as a fast path, and should not be used in places where its unreliability can be observed.The tests stay, but with a note added that they should pass but do not.