Skip to content

Fixed inferred type widening with contextual types created by binding patterns #56875

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed inferred type widening with contextual types created by binding patterns #56875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40905d4
Fixed inferred type widening with contextual types created by binding…
Andarist Dec 23, 2023
8f0a2c9
simplify one of the checks
Andarist Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 54 additions & 17 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1968,10 +1968,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
var blockedStringType = createIntrinsicType(TypeFlags.Any, "any", /*objectFlags*/ undefined, "blocked string");
var errorType = createIntrinsicType(TypeFlags.Any, "error");
var unresolvedType = createIntrinsicType(TypeFlags.Any, "unresolved");
var nonInferrableAnyType = createIntrinsicType(TypeFlags.Any, "any", ObjectFlags.ContainsWideningType, "non-inferrable");
var intrinsicMarkerType = createIntrinsicType(TypeFlags.Any, "intrinsic");
var unknownType = createIntrinsicType(TypeFlags.Unknown, "unknown");
var nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown", /*objectFlags*/ undefined, "non-null");
var nonInferrableUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown", ObjectFlags.ContainsWideningType, "non-inferrable");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely getIntersectionType should reduce nonInferrableUnknown & unknown to unknown. Most of such reductions are performed based on includes coming from addTypesToIntersection. It would be cool to introduce TypeFlags.IncludesNonInferrableUnknown but those Includes* flags are mostly reusing (?) other existing flags and I'm not super comfortable with touching this on my own.

var undefinedType = createIntrinsicType(TypeFlags.Undefined, "undefined");
var undefinedWideningType = strictNullChecks ? undefinedType : createIntrinsicType(TypeFlags.Undefined, "undefined", ObjectFlags.ContainsWideningType, "widening");
var missingType = createIntrinsicType(TypeFlags.Undefined, "undefined", /*objectFlags*/ undefined, "missing");
Expand Down Expand Up @@ -11444,10 +11444,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
reportImplicitAny(element, anyType);
}
// When we're including the pattern in the type (an indication we're obtaining a contextual type), we
// use a non-inferrable any type. Inference will never directly infer this type, but it is possible
// use a non-inferrable unknown type. Inference will never directly infer this type, but it is possible
// to infer a type that contains it, e.g. for a binding pattern like [foo] or { foo }. In such cases,
// widening of the binding pattern type substitutes a regular any for the non-inferrable any.
return includePatternInType ? nonInferrableAnyType : anyType;
// widening of the binding pattern type substitutes a regular any for the non-inferrable unknown.
return includePatternInType ? nonInferrableUnknownType : anyType;
Comment on lines +11449 to +11450
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure when "widening of the binding pattern type substitutes a regular any for the non-inferrable any" is happening and if this part of the comment is even relevant today.

}

// Return the type implied by an object binding pattern
Expand Down Expand Up @@ -20905,7 +20905,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
function isSimpleTypeRelatedTo(source: Type, target: Type, relation: Map<string, RelationComparisonResult>, errorReporter?: ErrorReporter) {
const s = source.flags;
const t = target.flags;
if (t & TypeFlags.Any || s & TypeFlags.Never || source === wildcardType) return true;
if (t & TypeFlags.Any || s & TypeFlags.Never || source === wildcardType || source === nonInferrableUnknownType) return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This likely isn't strictly needed - at least not for the test cases I have here now. While iterating on this, I planned to make the created pattern type pass the constraint check in getInferredType. This line would allow for that in some scenarios. Later I realized that it wouldn't allow for that in all scenarios though - and that's why I just avoid doing the constraint check in getInferredType. The idea is still that the nonInferrableUnknown should act like any for assignability purposes though - I just replaced its type with unknown to benefit from the T & unknown // T.

if (t & TypeFlags.Unknown && !(relation === strictSubtypeRelation && s & TypeFlags.Any)) return true;
if (t & TypeFlags.Never) return false;
if (s & TypeFlags.StringLike && t & TypeFlags.String) return true;
Expand Down Expand Up @@ -24603,6 +24603,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
else if (isObjectLiteralType(type)) {
result = getWidenedTypeOfObjectLiteral(type, context);
if (type.pattern) {
result.pattern = type.pattern;
}
}
else if (type.flags & TypeFlags.Union) {
const unionContext = context || createWideningContext(/*parent*/ undefined, /*propertyName*/ undefined, (type as UnionType).types);
Expand All @@ -24617,6 +24620,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
else if (isArrayOrTupleType(type)) {
result = createTypeReference(type.target, sameMap(getTypeArguments(type), getWidenedType));
if (type.pattern) {
result.pattern = type.pattern;
}
}
if (result && context === undefined) {
type.widened = result;
Expand Down Expand Up @@ -25025,21 +25031,34 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// For arrays and tuples we infer new arrays and tuples where the reverse mapping has been
// applied to the element type(s).
if (isArrayType(source)) {
return createArrayType(inferReverseMappedType(getTypeArguments(source)[0], target, constraint), isReadonlyArrayType(source));
let reversed = createArrayType(inferReverseMappedType(getTypeArguments(source)[0], target, constraint), isReadonlyArrayType(source)) as TypeReference;
if (source.pattern) {
reversed = cloneTypeReference(reversed);
reversed.pattern = source.pattern;
}
return reversed;
}
if (isTupleType(source)) {
const elementTypes = map(getElementTypes(source), t => inferReverseMappedType(t, target, constraint));
const elementFlags = getMappedTypeModifiers(target) & MappedTypeModifiers.IncludeOptional ?
sameMap(source.target.elementFlags, f => f & ElementFlags.Optional ? ElementFlags.Required : f) :
source.target.elementFlags;
return createTupleType(elementTypes, elementFlags, source.target.readonly, source.target.labeledElementDeclarations);
let reversed = createTupleType(elementTypes, elementFlags, source.target.readonly, source.target.labeledElementDeclarations) as TypeReference;
if (source.pattern) {
reversed = cloneTypeReference(reversed);
reversed.pattern = source.pattern;
}
return reversed;
}
// For all other object types we infer a new object type where the reverse mapping has been
// applied to the type of each property.
const reversed = createObjectType(ObjectFlags.ReverseMapped | ObjectFlags.Anonymous, /*symbol*/ undefined) as ReverseMappedType;
reversed.source = source;
reversed.mappedType = target;
reversed.constraintType = constraint;
if (source.pattern) {
reversed.pattern = source.pattern;
}
return reversed;
}

Expand All @@ -25056,7 +25075,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const templateType = getTemplateTypeFromMappedType(target);
const inference = createInferenceInfo(typeParameter);
inferTypes([inference], sourceType, templateType);
return getTypeFromInference(inference) || unknownType;
return getTypeFromInference(inference) || (sourceType.pattern ? nonInferrableUnknownType : unknownType);
}

function* getUnmatchedProperties(source: Type, target: Type, requireOptionalProperties: boolean, matchDiscriminantProperties: boolean): IterableIterator<Symbol> {
Expand Down Expand Up @@ -25089,8 +25108,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function tupleTypesDefinitelyUnrelated(source: TupleTypeReference, target: TupleTypeReference) {
return !(target.target.combinedFlags & ElementFlags.Variadic) && target.target.minLength > source.target.minLength ||
!target.target.hasRestElement && (source.target.hasRestElement || target.target.fixedLength < source.target.fixedLength);
return !source.pattern && (!(target.target.combinedFlags & ElementFlags.Variadic) && target.target.minLength > source.target.minLength ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us to infer between tuples of different shapes in those scenarios - since the tuple created by an array binding pattern might be shorter than the target. So far, I haven't managed to break this but I would advise extra caution around this when reviewing.

!target.target.hasRestElement && (source.target.hasRestElement || target.target.fixedLength < source.target.fixedLength));
}

function typesDefinitelyUnrelated(source: Type, target: Type) {
Expand Down Expand Up @@ -25396,9 +25415,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// This flag is infectious; if we produce Box<never> (where never is silentNeverType), Box<never> is
// also non-inferrable.
//
// As a special case, also ignore nonInferrableAnyType, which is a special form of the any type
// used as a stand-in for binding elements when they are being inferred.
if (getObjectFlags(source) & ObjectFlags.NonInferrableType || source === nonInferrableAnyType) {
// As a special case, also ignore nonInferrableUnknownType, which is a special form of the unknown type
// used as a stand-in for binding elements when they are being inferred. It is ignored as a direct inference candidate
// but types containing it can be inferred.
if (getObjectFlags(source) & ObjectFlags.NonInferrableType || source === nonInferrableUnknownType) {
return;
}
if (!inference.isFixed) {
Expand Down Expand Up @@ -26099,12 +26119,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
function getInferredType(context: InferenceContext, index: number): Type {
const inference = context.inferences[index];
if (!inference.inferredType) {
let isFromBindingPattern = false;
let inferredType: Type | undefined;
let fallbackType: Type | undefined;
if (context.signature) {
const inferredCovariantType = inference.candidates ? getCovariantInference(inference, context.signature) : undefined;
const inferredContravariantType = inference.contraCandidates ? getContravariantInference(inference) : undefined;
if (inferredCovariantType || inferredContravariantType) {
if (inferredCovariantType?.pattern) {
isFromBindingPattern = true;
inferredType = inferredCovariantType;
}
else if (inferredCovariantType || inferredContravariantType) {
// If we have both co- and contra-variant inferences, we prefer the co-variant inference if it is not 'never',
// all co-variant inferences are subtypes of it (i.e. it isn't one of a conflicting set of candidates), it is
// a subtype of some contra-variant inference, and no other type parameter is constrained to this type parameter
Expand Down Expand Up @@ -26145,7 +26170,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const constraint = getConstraintOfTypeParameter(inference.typeParameter);
if (constraint) {
const instantiatedConstraint = instantiateType(constraint, context.nonFixingMapper);
if (!inferredType || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) {
if (isFromBindingPattern) {
inference.inferredType = getIntersectionType([inference.inferredType, instantiatedConstraint]);
}
else if (!inferredType || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) {
// If the fallback type satisfies the constraint, we pick it. Otherwise, we pick the constraint.
inference.inferredType = fallbackType && context.compareTypes(fallbackType, getTypeWithThisArgument(instantiatedConstraint, fallbackType)) ? fallbackType : instantiatedConstraint;
}
Expand Down Expand Up @@ -30279,7 +30307,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
else if (t.flags & TypeFlags.StructuredType) {
const prop = getPropertyOfType(t, name);
if (prop) {
return isCircularMappedProperty(prop) ? undefined : removeMissingType(getTypeOfSymbol(prop), !!(prop && prop.flags & SymbolFlags.Optional));
if (isCircularMappedProperty(prop)) {
return undefined;
}
const type = getTypeOfSymbol(prop);
if (type !== nonInferrableUnknownType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since nonInferrableUnknownType doesn't hold any useful information for contextual typing I let it fallthrough to index signatures here. Note that { x: nonInferrableUnknownType } & { x: string } shouldn't return nonInferrableUnknownType for 'x' property here. So if the constraint already defines a property then index signatures won't be consulted.

return removeMissingType(type, !!(prop && prop.flags & SymbolFlags.Optional));
}
if (t.flags & TypeFlags.Intersection) {
t = getIntersectionType((t as IntersectionType).types.filter(t => !t.pattern));
}
Comment on lines +30317 to +30319
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I knew how to do it in a better way... this is mainly to satisfy the isTupleType(t) below (when it should be satisfied)

}
if (isTupleType(t) && isNumericLiteralName(name) && +name >= 0) {
const restType = getElementTypeOfSliceOfTupleType(t, t.target.fixedLength, /*endSkipCount*/ 0, /*writing*/ false, /*noReductions*/ true);
Expand Down Expand Up @@ -31248,7 +31285,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

pushCachedContextualType(node);
const contextualType = getApparentTypeOfContextualType(node, /*contextFlags*/ undefined);
const contextualTypeHasPattern = contextualType && contextualType.pattern &&
const contextualTypeHasPattern = !(checkMode & CheckMode.Inferential) && contextualType && contextualType.pattern &&
(contextualType.pattern.kind === SyntaxKind.ObjectBindingPattern || contextualType.pattern.kind === SyntaxKind.ObjectLiteralExpression);
const inConstContext = isConstContext(node);
const checkFlags = inConstContext ? CheckFlags.Readonly : 0;
Expand Down
11 changes: 0 additions & 11 deletions tests/baselines/reference/bindingPatternContextualTypeDoesNotCauseWidening.js
View file
Open in desktop

This file was deleted.

11 changes: 11 additions & 0 deletions tests/baselines/reference/bindingPatternContextualTypeDoesNotCauseWidening1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//// [tests/cases/compiler/bindingPatternContextualTypeDoesNotCauseWidening1.ts] ////

//// [bindingPatternContextualTypeDoesNotCauseWidening1.ts]
declare function pick<O, T extends keyof O>(keys: T[], obj?: O): Pick<O, T>;
const _ = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
const { } = pick(['b'], { a: 'a', b: 'b' }); // T: "b"


//// [bindingPatternContextualTypeDoesNotCauseWidening1.js]
var _ = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
var _a = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
40 changes: 20 additions & 20 deletions ...ontextualTypeDoesNotCauseWidening.symbols → ...ntextualTypeDoesNotCauseWidening1.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
//// [tests/cases/compiler/bindingPatternContextualTypeDoesNotCauseWidening.ts] ////
//// [tests/cases/compiler/bindingPatternContextualTypeDoesNotCauseWidening1.ts] ////

=== bindingPatternContextualTypeDoesNotCauseWidening.ts ===
=== bindingPatternContextualTypeDoesNotCauseWidening1.ts ===
declare function pick<O, T extends keyof O>(keys: T[], obj?: O): Pick<O, T>;
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 0))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 22))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 24))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 22))
>keys : Symbol(keys, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 44))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 24))
>obj : Symbol(obj, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 54))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 22))
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 0))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 22))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 24))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 22))
>keys : Symbol(keys, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 44))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 24))
>obj : Symbol(obj, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 54))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 22))
>Pick : Symbol(Pick, Decl(lib.es5.d.ts, --, --))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 22))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 24))
>O : Symbol(O, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 22))
>T : Symbol(T, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 24))

const _ = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
>_ : Symbol(_, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 1, 5))
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 0))
>a : Symbol(a, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 1, 26))
>b : Symbol(b, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 1, 34))
>_ : Symbol(_, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 1, 5))
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 0))
>a : Symbol(a, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 1, 26))
>b : Symbol(b, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 1, 34))

const { } = pick(['b'], { a: 'a', b: 'b' }); // T: "b" | "a" ??? (before fix)
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 0, 0))
>a : Symbol(a, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 2, 26))
>b : Symbol(b, Decl(bindingPatternContextualTypeDoesNotCauseWidening.ts, 2, 34))
const { } = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
>pick : Symbol(pick, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 0, 0))
>a : Symbol(a, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 2, 26))
>b : Symbol(b, Decl(bindingPatternContextualTypeDoesNotCauseWidening1.ts, 2, 34))

6 changes: 3 additions & 3 deletions ...nContextualTypeDoesNotCauseWidening.types → ...ContextualTypeDoesNotCauseWidening1.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//// [tests/cases/compiler/bindingPatternContextualTypeDoesNotCauseWidening.ts] ////
//// [tests/cases/compiler/bindingPatternContextualTypeDoesNotCauseWidening1.ts] ////

=== bindingPatternContextualTypeDoesNotCauseWidening.ts ===
=== bindingPatternContextualTypeDoesNotCauseWidening1.ts ===
declare function pick<O, T extends keyof O>(keys: T[], obj?: O): Pick<O, T>;
>pick : <O, T extends keyof O>(keys: T[], obj?: O) => Pick<O, T>
>keys : T[]
Expand All @@ -18,7 +18,7 @@ const _ = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
>b : string
>'b' : "b"

const { } = pick(['b'], { a: 'a', b: 'b' }); // T: "b" | "a" ??? (before fix)
const { } = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
>pick(['b'], { a: 'a', b: 'b' }) : Pick<{ a: string; b: string; }, "b">
>pick : <O, T extends keyof O>(keys: T[], obj?: O) => Pick<O, T>
>['b'] : "b"[]
Expand Down
Loading