-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Don't obtain apparent type of contextual types being optional type variables #61635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Don't obtain apparent type of contextual types being optional type variables #61635
Conversation
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.
Pull Request Overview
This PR addresses #61633 by fixing an incorrect treatment of contextual types that are optional type variables. The changes include updating the type checker logic to use a non‑nullable version of the instantiated type when verifying type variable status, and adding corresponding tests.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/cases/compiler/inferenceFromGenericClass1.ts | Adds tests to verify proper inference when using generic classes with optional type variables. |
tests/baselines/reference/inferenceFromGenericClass1.js | Updates baseline output corresponding to the new test behavior. |
src/compiler/checker.ts | Adjusts the type checking logic to use getNonNullableType and maybeTypeOfKind in the constraint check. |
Files not reviewed (2)
- tests/baselines/reference/inferenceFromGenericClass1.symbols: Language not supported
- tests/baselines/reference/inferenceFromGenericClass1.types: Language not supported
@@ -32412,7 +32412,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
getContextualTypeForObjectLiteralMethod(node, contextFlags) : | |||
getContextualType(node, contextFlags); | |||
const instantiatedType = instantiateContextualType(contextualType, node, contextFlags); | |||
if (instantiatedType && !(contextFlags && contextFlags & ContextFlags.NoConstraints && instantiatedType.flags & TypeFlags.TypeVariable)) { | |||
if (instantiatedType && !(contextFlags && contextFlags & ContextFlags.NoConstraints && maybeTypeOfKind(getNonNullableType(instantiatedType), TypeFlags.TypeVariable))) { |
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.
This change replaces the direct check on instantiatedType.flags with a call to maybeTypeOfKind on a non-nullable version of instantiatedType. Please double-check that using getNonNullableType ensures that optional type variables are not incorrectly inferred as type variables.
Copilot uses AI. Check for mistakes.
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.
- Without this
instantiateTypeWithSingleGenericCallSignature
computedcontextualType
asAnyConstructor | undefined
- then eliminated
| undefined
withgetNonNullableType
- and obtained a generic
contextualSignature
from that - and returned
anyFunctionType
because this happened withCheckMode.SkipGenericFunctions
- given
anyFunctionType
is non-inferrable,getInferredType
picked up the default type argument (undefined
) - and
getSignatureApplicabilityError
returned with diagnostics because it was called with a check candidate that was a product of signature instantiation with currently inferred type arguments
f29693d
to
529fc9e
Compare
fixes #61633