Skip to content

Commit

Permalink
Merge pull request #19671 from Microsoft/nominalInstanceof
Browse files Browse the repository at this point in the history
Improved handling of structurally identical classes
  • Loading branch information
ahejlsberg committed Nov 3, 2017
2 parents f67a9ba + 81326ac commit add8b49
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 40 deletions.
50 changes: 28 additions & 22 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7440,9 +7440,12 @@ namespace ts {
return false;
}

function isSubtypeOfAny(candidate: Type, types: Type[]): boolean {
for (const type of types) {
if (candidate !== type && isTypeSubtypeOf(candidate, type)) {
function isSubtypeOfAny(source: Type, targets: Type[]): boolean {
for (const target of targets) {
if (source !== target && isTypeSubtypeOf(source, target) && (
!(getObjectFlags(getTargetType(source)) & ObjectFlags.Class) ||
!(getObjectFlags(getTargetType(target)) & ObjectFlags.Class) ||
isTypeDerivedFrom(source, target))) {
return true;
}
}
Expand Down Expand Up @@ -8565,12 +8568,19 @@ namespace ts {
return isTypeRelatedTo(source, target, assignableRelation);
}

// A type S is considered to be an instance of a type T if S and T are the same type or if S is a
// subtype of T but not structurally identical to T. This specifically means that two distinct but
// structurally identical types (such as two classes) are not considered instances of each other.
function isTypeInstanceOf(source: Type, target: Type): boolean {
return getTargetType(source) === getTargetType(target) || isTypeSubtypeOf(source, target) && !isTypeIdenticalTo(source, target);
}
// An object type S is considered to be derived from an object type T if
// S is a union type and every constituent of S is derived from T,
// T is a union type and S is derived from at least one constituent of T, or
// T is one of the global types Object and Function and S is a subtype of T, or
// T occurs directly or indirectly in an 'extends' clause of S.
// Note that this check ignores type parameters and only considers the
// inheritance hierarchy.
function isTypeDerivedFrom(source: Type, target: Type): boolean {
return source.flags & TypeFlags.Union ? every((<UnionType>source).types, t => isTypeDerivedFrom(t, target)) :
target.flags & TypeFlags.Union ? some((<UnionType>target).types, t => isTypeDerivedFrom(source, t)) :
target === globalObjectType || target === globalFunctionType ? isTypeSubtypeOf(source, target) :
hasBaseType(source, getTargetType(target));
}

/**
* This is *not* a bi-directional relationship.
Expand Down Expand Up @@ -9604,7 +9614,7 @@ namespace ts {
if (relation === identityRelation) {
return propertiesIdenticalTo(source, target);
}
const requireOptionalProperties = relation === subtypeRelation && !isObjectLiteralType(source);
const requireOptionalProperties = relation === subtypeRelation && !isObjectLiteralType(source) && !isEmptyArrayLiteralType(source);
const unmatchedProperty = getUnmatchedProperty(source, target, requireOptionalProperties);
if (unmatchedProperty) {
if (reportErrors) {
Expand Down Expand Up @@ -10312,6 +10322,11 @@ namespace ts {
!(type.flags & TypeFlags.Nullable) && isTypeAssignableTo(type, anyReadonlyArrayType);
}

function isEmptyArrayLiteralType(type: Type): boolean {
const elementType = isArrayType(type) ? (<TypeReference>type).typeArguments[0] : undefined;
return elementType === undefinedWideningType || elementType === neverType;
}

function isTupleLikeType(type: Type): boolean {
return !!getPropertyOfType(type, "0" as __String);
}
Expand Down Expand Up @@ -12415,7 +12430,7 @@ namespace ts {
}

if (targetType) {
return getNarrowedType(type, targetType, assumeTrue, isTypeInstanceOf);
return getNarrowedType(type, targetType, assumeTrue, isTypeDerivedFrom);
}

return type;
Expand Down Expand Up @@ -13883,7 +13898,6 @@ namespace ts {
type.pattern = node;
return type;
}
const contextualType = getApparentTypeOfContextualType(node);
if (contextualType && contextualTypeIsTupleLikeType(contextualType)) {
const pattern = contextualType.pattern;
// If array literal is contextually typed by a binding pattern or an assignment pattern, pad the resulting
Expand Down Expand Up @@ -18087,14 +18101,6 @@ namespace ts {
return (target.flags & TypeFlags.Nullable) !== 0 || isTypeComparableTo(source, target);
}

function getBestChoiceType(type1: Type, type2: Type): Type {
const firstAssignableToSecond = isTypeAssignableTo(type1, type2);
const secondAssignableToFirst = isTypeAssignableTo(type2, type1);
return secondAssignableToFirst && !firstAssignableToSecond ? type1 :
firstAssignableToSecond && !secondAssignableToFirst ? type2 :
getUnionType([type1, type2], /*subtypeReduction*/ true);
}

function checkBinaryExpression(node: BinaryExpression, checkMode?: CheckMode) {
return checkBinaryLikeExpression(node.left, node.operatorToken, node.right, checkMode, node);
}
Expand Down Expand Up @@ -18231,7 +18237,7 @@ namespace ts {
leftType;
case SyntaxKind.BarBarToken:
return getTypeFacts(leftType) & TypeFacts.Falsy ?
getBestChoiceType(removeDefinitelyFalsyTypes(leftType), rightType) :
getUnionType([removeDefinitelyFalsyTypes(leftType), rightType], /*subtypeReduction*/ true) :
leftType;
case SyntaxKind.EqualsToken:
checkAssignmentOperator(rightType);
Expand Down Expand Up @@ -18391,7 +18397,7 @@ namespace ts {
checkExpression(node.condition);
const type1 = checkExpression(node.whenTrue, checkMode);
const type2 = checkExpression(node.whenFalse, checkMode);
return getBestChoiceType(type1, type2);
return getUnionType([type1, type2], /*subtypeReduction*/ true);
}

function checkTemplateExpression(node: TemplateExpression): Type {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2641,7 +2641,7 @@ namespace ts {
/**
* Gets a custom text range to use when emitting source maps.
*/
export function getSourceMapRange(node: Node) {
export function getSourceMapRange(node: Node): SourceMapRange {
const emitNode = node.emitNode;
return (emitNode && emitNode.sourceMapRange) || node;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ var myList: MyList<number>;
>MyList : MyList<T>

var xs = [list, myList]; // {}[]
>xs : List<number>[]
>[list, myList] : List<number>[]
>xs : (List<number> | MyList<number>)[]
>[list, myList] : (List<number> | MyList<number>)[]
>list : List<number>
>myList : MyList<number>

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/fixSignatureCaching.types
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,9 @@ define(function () {
>'UnknownMobile' : "UnknownMobile"

isArray = ('isArray' in Array) ?
>isArray = ('isArray' in Array) ? Array.isArray : function (value) { return Object.prototype.toString.call(value) === '[object Array]'; } : (value: any) => boolean
>isArray = ('isArray' in Array) ? Array.isArray : function (value) { return Object.prototype.toString.call(value) === '[object Array]'; } : (arg: any) => arg is any[]
>isArray : any
>('isArray' in Array) ? Array.isArray : function (value) { return Object.prototype.toString.call(value) === '[object Array]'; } : (value: any) => boolean
>('isArray' in Array) ? Array.isArray : function (value) { return Object.prototype.toString.call(value) === '[object Array]'; } : (arg: any) => arg is any[]
>('isArray' in Array) : boolean
>'isArray' in Array : boolean
>'isArray' : "isArray"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
tests/cases/conformance/types/typeRelationships/instanceOf/narrowingGenericTypeFromInstanceof01.ts(13,17): error TS2345: Argument of type 'A<T> | B<T>' is not assignable to parameter of type 'A<T>'.
Type 'B<T>' is not assignable to type 'A<T>'.
Property 'a' is missing in type 'B<T>'.
tests/cases/conformance/types/typeRelationships/instanceOf/narrowingGenericTypeFromInstanceof01.ts(13,17): error TS2345: Argument of type 'B<T>' is not assignable to parameter of type 'A<{}>'.
Property 'a' is missing in type 'B<T>'.


==== tests/cases/conformance/types/typeRelationships/instanceOf/narrowingGenericTypeFromInstanceof01.ts (1 errors) ====
Expand All @@ -18,9 +17,8 @@ tests/cases/conformance/types/typeRelationships/instanceOf/narrowingGenericTypeF
if (x instanceof B) {
acceptA(x);
~
!!! error TS2345: Argument of type 'A<T> | B<T>' is not assignable to parameter of type 'A<T>'.
!!! error TS2345: Type 'B<T>' is not assignable to type 'A<T>'.
!!! error TS2345: Property 'a' is missing in type 'B<T>'.
!!! error TS2345: Argument of type 'B<T>' is not assignable to parameter of type 'A<{}>'.
!!! error TS2345: Property 'a' is missing in type 'B<T>'.
}

if (x instanceof A) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function test<T>(x: A<T> | B<T>) {
acceptA(x);
>acceptA(x) : any
>acceptA : <T>(a: A<T>) => void
>x : A<T> | B<T>
>x : B<T>
}

if (x instanceof A) {
Expand All @@ -65,7 +65,7 @@ function test<T>(x: A<T> | B<T>) {
acceptB(x);
>acceptB(x) : void
>acceptB : <T>(b: B<T>) => void
>x : A<T> | B<T>
>x : B<T>
}

if (x instanceof B) {
Expand All @@ -76,6 +76,6 @@ function test<T>(x: A<T> | B<T>) {
acceptB(x);
>acceptB(x) : void
>acceptB : <T>(b: B<T>) => void
>x : A<T> | B<T>
>x : B<T>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ var e: Ellement;
>Ellement : Symbol(Ellement, Decl(nonContextuallyTypedLogicalOr.ts, 3, 1))

(c || e).dummy;
>(c || e).dummy : Symbol(Contextual.dummy, Decl(nonContextuallyTypedLogicalOr.ts, 0, 22))
>(c || e).dummy : Symbol(dummy, Decl(nonContextuallyTypedLogicalOr.ts, 0, 22), Decl(nonContextuallyTypedLogicalOr.ts, 5, 20))
>c : Symbol(c, Decl(nonContextuallyTypedLogicalOr.ts, 10, 3))
>e : Symbol(e, Decl(nonContextuallyTypedLogicalOr.ts, 11, 3))
>dummy : Symbol(Contextual.dummy, Decl(nonContextuallyTypedLogicalOr.ts, 0, 22))
>dummy : Symbol(dummy, Decl(nonContextuallyTypedLogicalOr.ts, 0, 22), Decl(nonContextuallyTypedLogicalOr.ts, 5, 20))

4 changes: 2 additions & 2 deletions tests/baselines/reference/nonContextuallyTypedLogicalOr.types
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ var e: Ellement;

(c || e).dummy;
>(c || e).dummy : any
>(c || e) : Contextual
>c || e : Contextual
>(c || e) : Contextual | Ellement
>c || e : Contextual | Ellement
>c : Contextual
>e : Ellement
>dummy : any
Expand Down

0 comments on commit add8b49

Please sign in to comment.