-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix partial type relations #17382
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
Fix partial type relations #17382
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5765,12 +5765,13 @@ namespace ts { | |
return type.modifiersType; | ||
} | ||
|
||
function isPartialMappedType(type: Type) { | ||
return getObjectFlags(type) & ObjectFlags.Mapped && !!(<MappedType>type).declaration.questionToken; | ||
} | ||
|
||
function isGenericMappedType(type: Type) { | ||
if (getObjectFlags(type) & ObjectFlags.Mapped) { | ||
const constraintType = getConstraintTypeFromMappedType(<MappedType>type); | ||
return maybeTypeOfKind(constraintType, TypeFlags.TypeVariable | TypeFlags.Index); | ||
} | ||
return false; | ||
return getObjectFlags(type) & ObjectFlags.Mapped && | ||
maybeTypeOfKind(getConstraintTypeFromMappedType(<MappedType>type), TypeFlags.TypeVariable | TypeFlags.Index); | ||
} | ||
|
||
function resolveStructuredTypeMembers(type: StructuredType): ResolvedType { | ||
|
@@ -9254,8 +9255,12 @@ namespace ts { | |
if (source.flags & (TypeFlags.Object | TypeFlags.Intersection) && target.flags & TypeFlags.Object) { | ||
// Report structural errors only if we haven't reported any errors yet | ||
const reportStructuralErrors = reportErrors && errorInfo === saveErrorInfo && !sourceIsPrimitive; | ||
if (isGenericMappedType(source) || isGenericMappedType(target)) { | ||
result = mappedTypeRelatedTo(source, target, reportStructuralErrors); | ||
// An empty object type is related to any mapped type that includes a '?' modifier. | ||
if (isPartialMappedType(target) && !isGenericMappedType(source) && isEmptyObjectType(source)) { | ||
result = Ternary.True; | ||
} | ||
else if (isGenericMappedType(target)) { | ||
result = isGenericMappedType(source) ? mappedTypeRelatedTo(<MappedType>source, <MappedType>target, reportStructuralErrors) : Ternary.False; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you made There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, because something for which |
||
} | ||
else { | ||
result = propertiesRelatedTo(source, target, reportStructuralErrors); | ||
|
@@ -9284,33 +9289,19 @@ namespace ts { | |
// A type [P in S]: X is related to a type [Q in T]: Y if T is related to S and X' is | ||
// related to Y, where X' is an instantiation of X in which P is replaced with Q. Notice | ||
// that S and T are contra-variant whereas X and Y are co-variant. | ||
function mappedTypeRelatedTo(source: Type, target: Type, reportErrors: boolean): Ternary { | ||
if (isGenericMappedType(target)) { | ||
if (isGenericMappedType(source)) { | ||
const sourceReadonly = !!(<MappedType>source).declaration.readonlyToken; | ||
const sourceOptional = !!(<MappedType>source).declaration.questionToken; | ||
const targetReadonly = !!(<MappedType>target).declaration.readonlyToken; | ||
const targetOptional = !!(<MappedType>target).declaration.questionToken; | ||
const modifiersRelated = relation === identityRelation ? | ||
sourceReadonly === targetReadonly && sourceOptional === targetOptional : | ||
relation === comparableRelation || !sourceOptional || targetOptional; | ||
if (modifiersRelated) { | ||
let result: Ternary; | ||
if (result = isRelatedTo(getConstraintTypeFromMappedType(<MappedType>target), getConstraintTypeFromMappedType(<MappedType>source), reportErrors)) { | ||
const mapper = createTypeMapper([getTypeParameterFromMappedType(<MappedType>source)], [getTypeParameterFromMappedType(<MappedType>target)]); | ||
return result & isRelatedTo(instantiateType(getTemplateTypeFromMappedType(<MappedType>source), mapper), getTemplateTypeFromMappedType(<MappedType>target), reportErrors); | ||
} | ||
} | ||
} | ||
else if ((<MappedType>target).declaration.questionToken && isEmptyObjectType(source)) { | ||
return Ternary.True; | ||
|
||
} | ||
} | ||
else if (relation !== identityRelation) { | ||
const resolved = resolveStructuredTypeMembers(<ObjectType>target); | ||
if (isEmptyResolvedType(resolved) || resolved.stringIndexInfo && resolved.stringIndexInfo.type.flags & TypeFlags.Any) { | ||
return Ternary.True; | ||
function mappedTypeRelatedTo(source: MappedType, target: MappedType, reportErrors: boolean): Ternary { | ||
const sourceReadonly = !!source.declaration.readonlyToken; | ||
const sourceOptional = !!source.declaration.questionToken; | ||
const targetReadonly = !!target.declaration.readonlyToken; | ||
const targetOptional = !!target.declaration.questionToken; | ||
const modifiersRelated = relation === identityRelation ? | ||
sourceReadonly === targetReadonly && sourceOptional === targetOptional : | ||
relation === comparableRelation || !sourceOptional || targetOptional; | ||
if (modifiersRelated) { | ||
let result: Ternary; | ||
if (result = isRelatedTo(getConstraintTypeFromMappedType(<MappedType>target), getConstraintTypeFromMappedType(<MappedType>source), reportErrors)) { | ||
const mapper = createTypeMapper([getTypeParameterFromMappedType(<MappedType>source)], [getTypeParameterFromMappedType(<MappedType>target)]); | ||
return result & isRelatedTo(instantiateType(getTemplateTypeFromMappedType(<MappedType>source), mapper), getTemplateTypeFromMappedType(<MappedType>target), reportErrors); | ||
} | ||
} | ||
return Ternary.False; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
//// [mappedTypePartialConstraints.ts] | ||
// Repro from #16985 | ||
|
||
interface MyInterface { | ||
something: number; | ||
} | ||
|
||
class MyClass<T extends MyInterface> { | ||
doIt(data : Partial<T>) {} | ||
} | ||
|
||
class MySubClass extends MyClass<MyInterface> {} | ||
|
||
function fn(arg: typeof MyClass) {}; | ||
|
||
fn(MySubClass); | ||
|
||
|
||
//// [mappedTypePartialConstraints.js] | ||
// Repro from #16985 | ||
var __extends = (this && this.__extends) || (function () { | ||
var extendStatics = Object.setPrototypeOf || | ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || | ||
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; | ||
return function (d, b) { | ||
extendStatics(d, b); | ||
function __() { this.constructor = d; } | ||
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); | ||
}; | ||
})(); | ||
var MyClass = (function () { | ||
function MyClass() { | ||
} | ||
MyClass.prototype.doIt = function (data) { }; | ||
return MyClass; | ||
}()); | ||
var MySubClass = (function (_super) { | ||
__extends(MySubClass, _super); | ||
function MySubClass() { | ||
return _super !== null && _super.apply(this, arguments) || this; | ||
} | ||
return MySubClass; | ||
}(MyClass)); | ||
function fn(arg) { } | ||
; | ||
fn(MySubClass); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
=== tests/cases/compiler/mappedTypePartialConstraints.ts === | ||
// Repro from #16985 | ||
|
||
interface MyInterface { | ||
>MyInterface : Symbol(MyInterface, Decl(mappedTypePartialConstraints.ts, 0, 0)) | ||
|
||
something: number; | ||
>something : Symbol(MyInterface.something, Decl(mappedTypePartialConstraints.ts, 2, 23)) | ||
} | ||
|
||
class MyClass<T extends MyInterface> { | ||
>MyClass : Symbol(MyClass, Decl(mappedTypePartialConstraints.ts, 4, 1)) | ||
>T : Symbol(T, Decl(mappedTypePartialConstraints.ts, 6, 14)) | ||
>MyInterface : Symbol(MyInterface, Decl(mappedTypePartialConstraints.ts, 0, 0)) | ||
|
||
doIt(data : Partial<T>) {} | ||
>doIt : Symbol(MyClass.doIt, Decl(mappedTypePartialConstraints.ts, 6, 38)) | ||
>data : Symbol(data, Decl(mappedTypePartialConstraints.ts, 7, 7)) | ||
>Partial : Symbol(Partial, Decl(lib.d.ts, --, --)) | ||
>T : Symbol(T, Decl(mappedTypePartialConstraints.ts, 6, 14)) | ||
} | ||
|
||
class MySubClass extends MyClass<MyInterface> {} | ||
>MySubClass : Symbol(MySubClass, Decl(mappedTypePartialConstraints.ts, 8, 1)) | ||
>MyClass : Symbol(MyClass, Decl(mappedTypePartialConstraints.ts, 4, 1)) | ||
>MyInterface : Symbol(MyInterface, Decl(mappedTypePartialConstraints.ts, 0, 0)) | ||
|
||
function fn(arg: typeof MyClass) {}; | ||
>fn : Symbol(fn, Decl(mappedTypePartialConstraints.ts, 10, 48)) | ||
>arg : Symbol(arg, Decl(mappedTypePartialConstraints.ts, 12, 12)) | ||
>MyClass : Symbol(MyClass, Decl(mappedTypePartialConstraints.ts, 4, 1)) | ||
|
||
fn(MySubClass); | ||
>fn : Symbol(fn, Decl(mappedTypePartialConstraints.ts, 10, 48)) | ||
>MySubClass : Symbol(MySubClass, Decl(mappedTypePartialConstraints.ts, 8, 1)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
=== tests/cases/compiler/mappedTypePartialConstraints.ts === | ||
// Repro from #16985 | ||
|
||
interface MyInterface { | ||
>MyInterface : MyInterface | ||
|
||
something: number; | ||
>something : number | ||
} | ||
|
||
class MyClass<T extends MyInterface> { | ||
>MyClass : MyClass<T> | ||
>T : T | ||
>MyInterface : MyInterface | ||
|
||
doIt(data : Partial<T>) {} | ||
>doIt : (data: Partial<T>) => void | ||
>data : Partial<T> | ||
>Partial : Partial<T> | ||
>T : T | ||
} | ||
|
||
class MySubClass extends MyClass<MyInterface> {} | ||
>MySubClass : MySubClass | ||
>MyClass : MyClass<MyInterface> | ||
>MyInterface : MyInterface | ||
|
||
function fn(arg: typeof MyClass) {}; | ||
>fn : (arg: typeof MyClass) => void | ||
>arg : typeof MyClass | ||
>MyClass : typeof MyClass | ||
|
||
fn(MySubClass); | ||
>fn(MySubClass) : void | ||
>fn : (arg: typeof MyClass) => void | ||
>MySubClass : typeof MySubClass | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Repro from #16985 | ||
|
||
interface MyInterface { | ||
something: number; | ||
} | ||
|
||
class MyClass<T extends MyInterface> { | ||
doIt(data : Partial<T>) {} | ||
} | ||
|
||
class MySubClass extends MyClass<MyInterface> {} | ||
|
||
function fn(arg: typeof MyClass) {}; | ||
|
||
fn(MySubClass); |
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 seems like an awfully specific check - does it work on homomorphic mapped types in which all properties of the type being mapped are already optional? For example:
The same question applies to if you used
Readonly<T>
or others.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.
I'm not sure what you're getting at. The only way we can be certain that all properties of the type that results from a mapping operation are optional is if the mapped type includes a
?
in the template specification. That's what we're checking here.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.
As we discussed offline, that example doesn't apply because a constraint being all-optional (e.g.
MyInterface
) doesn't guarantee thatT
itself will be all-optional.