Skip to content
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 isTypeDerivedFrom to properly handle {} and intersections #51631

Merged
merged 2 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18759,16 +18759,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// 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
// S is a type variable with a base constraint that is derived from T,
// S is an intersection type and some constituent of S is derived from T, or
// S is a type variable with a base constraint that is derived from T, or
// T is {} and S is an object-like type (ensuring {} is less derived than Object), or
Copy link
Contributor

Choose a reason for hiding this comment

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

q: what does "less derived" mean? how something can be more or less derived?

Copy link
Member Author

Choose a reason for hiding this comment

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

It means that any object-like type, including Object, appears to be derived from {}. The effect of this addition is that x instanceof Object will narrow {} to Object, which in turn means that x qualifies as the right hand operand of an in check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably reword this like this then:

Suggested change
// T is {} and S is an object-like type (ensuring {} is less derived than Object), or
// T is {} and S is an object-like type (ensuring that object-likes, including Object, are derived from {}), or

Maybe I'm just missing the appropriate background and lingo but this explanation is more readable to me.

Copy link

Choose a reason for hiding this comment

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

In the context of type theory, "less derived" is definitely a thing. e.g. Animal is less derived than Dog which is less derived than PitBull, while Cat and Dog are equally derived, being sister types. These relationships are obvious under nominal typing, but probably get pretty hairy under structural typing.

// 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((source as UnionType).types, t => isTypeDerivedFrom(t, target)) :
target.flags & TypeFlags.Union ? some((target as UnionType).types, t => isTypeDerivedFrom(source, t)) :
source.flags & TypeFlags.Intersection ? some((source as IntersectionType).types, t => isTypeDerivedFrom(t, target)) :
source.flags & TypeFlags.InstantiableNonPrimitive ? isTypeDerivedFrom(getBaseConstraintOfType(source) || unknownType, target) :
target === globalObjectType ? !!(source.flags & (TypeFlags.Object | TypeFlags.NonPrimitive)) :
isEmptyAnonymousObjectType(target) ? !!(source.flags & (TypeFlags.Object | TypeFlags.NonPrimitive)) :
target === globalObjectType ? !!(source.flags & (TypeFlags.Object | TypeFlags.NonPrimitive)) && !isEmptyAnonymousObjectType(source) :
target === globalFunctionType ? !!(source.flags & TypeFlags.Object) && isFunctionObjectType(source as ObjectType) :
hasBaseType(source, getTargetType(target)) || (isArrayType(target) && !isReadonlyArrayType(target) && isTypeDerivedFrom(source, globalReadonlyArrayType));
}
Expand Down
59 changes: 59 additions & 0 deletions tests/baselines/reference/inKeywordAndIntersection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//// [inKeywordAndIntersection.ts]
class A { a = 0 }
class B { b = 0 }

function f10(obj: A & { x: string } | B) {
if (obj instanceof Object) {
obj; // A & { x: string } | B
}
else {
obj; // Error
}
}

// Repro from #50844

interface InstanceOne {
one(): void
}

interface InstanceTwo {
two(): void
}

const instance = {} as InstanceOne | InstanceTwo

const ClassOne = {} as { new(): InstanceOne } & { foo: true };

if (instance instanceof ClassOne) {
instance.one();
}


//// [inKeywordAndIntersection.js]
"use strict";
var A = /** @class */ (function () {
function A() {
this.a = 0;
}
return A;
}());
var B = /** @class */ (function () {
function B() {
this.b = 0;
}
return B;
}());
function f10(obj) {
if (obj instanceof Object) {
obj; // A & { x: string } | B
}
else {
obj; // Error
}
}
var instance = {};
var ClassOne = {};
if (instance instanceof ClassOne) {
instance.one();
}
65 changes: 65 additions & 0 deletions tests/baselines/reference/inKeywordAndIntersection.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
=== tests/cases/compiler/inKeywordAndIntersection.ts ===
class A { a = 0 }
>A : Symbol(A, Decl(inKeywordAndIntersection.ts, 0, 0))
>a : Symbol(A.a, Decl(inKeywordAndIntersection.ts, 0, 9))

class B { b = 0 }
>B : Symbol(B, Decl(inKeywordAndIntersection.ts, 0, 17))
>b : Symbol(B.b, Decl(inKeywordAndIntersection.ts, 1, 9))

function f10(obj: A & { x: string } | B) {
>f10 : Symbol(f10, Decl(inKeywordAndIntersection.ts, 1, 17))
>obj : Symbol(obj, Decl(inKeywordAndIntersection.ts, 3, 13))
>A : Symbol(A, Decl(inKeywordAndIntersection.ts, 0, 0))
>x : Symbol(x, Decl(inKeywordAndIntersection.ts, 3, 23))
>B : Symbol(B, Decl(inKeywordAndIntersection.ts, 0, 17))

if (obj instanceof Object) {
>obj : Symbol(obj, Decl(inKeywordAndIntersection.ts, 3, 13))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

obj; // A & { x: string } | B
>obj : Symbol(obj, Decl(inKeywordAndIntersection.ts, 3, 13))
}
else {
obj; // Error
>obj : Symbol(obj, Decl(inKeywordAndIntersection.ts, 3, 13))
}
}

// Repro from #50844

interface InstanceOne {
>InstanceOne : Symbol(InstanceOne, Decl(inKeywordAndIntersection.ts, 10, 1))

one(): void
>one : Symbol(InstanceOne.one, Decl(inKeywordAndIntersection.ts, 14, 23))
}

interface InstanceTwo {
>InstanceTwo : Symbol(InstanceTwo, Decl(inKeywordAndIntersection.ts, 16, 1))

two(): void
>two : Symbol(InstanceTwo.two, Decl(inKeywordAndIntersection.ts, 18, 23))
}

const instance = {} as InstanceOne | InstanceTwo
>instance : Symbol(instance, Decl(inKeywordAndIntersection.ts, 22, 5))
>InstanceOne : Symbol(InstanceOne, Decl(inKeywordAndIntersection.ts, 10, 1))
>InstanceTwo : Symbol(InstanceTwo, Decl(inKeywordAndIntersection.ts, 16, 1))

const ClassOne = {} as { new(): InstanceOne } & { foo: true };
>ClassOne : Symbol(ClassOne, Decl(inKeywordAndIntersection.ts, 24, 5))
>InstanceOne : Symbol(InstanceOne, Decl(inKeywordAndIntersection.ts, 10, 1))
>foo : Symbol(foo, Decl(inKeywordAndIntersection.ts, 24, 49))

if (instance instanceof ClassOne) {
>instance : Symbol(instance, Decl(inKeywordAndIntersection.ts, 22, 5))
>ClassOne : Symbol(ClassOne, Decl(inKeywordAndIntersection.ts, 24, 5))

instance.one();
>instance.one : Symbol(InstanceOne.one, Decl(inKeywordAndIntersection.ts, 14, 23))
>instance : Symbol(instance, Decl(inKeywordAndIntersection.ts, 22, 5))
>one : Symbol(InstanceOne.one, Decl(inKeywordAndIntersection.ts, 14, 23))
}

66 changes: 66 additions & 0 deletions tests/baselines/reference/inKeywordAndIntersection.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
=== tests/cases/compiler/inKeywordAndIntersection.ts ===
class A { a = 0 }
>A : A
>a : number
>0 : 0

class B { b = 0 }
>B : B
>b : number
>0 : 0

function f10(obj: A & { x: string } | B) {
>f10 : (obj: (A & { x: string;}) | B) => void
>obj : B | (A & { x: string; })
>x : string

if (obj instanceof Object) {
>obj instanceof Object : boolean
>obj : B | (A & { x: string; })
>Object : ObjectConstructor

obj; // A & { x: string } | B
>obj : B | (A & { x: string; })
}
else {
obj; // Error
>obj : never
}
}

// Repro from #50844

interface InstanceOne {
one(): void
>one : () => void
}

interface InstanceTwo {
two(): void
>two : () => void
}

const instance = {} as InstanceOne | InstanceTwo
>instance : InstanceOne | InstanceTwo
>{} as InstanceOne | InstanceTwo : InstanceOne | InstanceTwo
>{} : {}

const ClassOne = {} as { new(): InstanceOne } & { foo: true };
>ClassOne : (new () => InstanceOne) & { foo: true; }
>{} as { new(): InstanceOne } & { foo: true } : (new () => InstanceOne) & { foo: true; }
>{} : {}
>foo : true
>true : true

if (instance instanceof ClassOne) {
>instance instanceof ClassOne : boolean
>instance : InstanceOne | InstanceTwo
>ClassOne : (new () => InstanceOne) & { foo: true; }

instance.one();
>instance.one() : void
>instance.one : () => void
>instance : InstanceOne
>one : () => void
}

34 changes: 34 additions & 0 deletions tests/baselines/reference/inKeywordAndUnknown.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,38 @@ tests/cases/compiler/inKeywordAndUnknown.ts(12,18): error TS2638: Type '{}' may
}
y; // {}
}

// Repro from #51007

function isHTMLTable(table: unknown): boolean {
return !!table && table instanceof Object && 'html' in table;
}

function f1(x: unknown) {
return x && x instanceof Object && 'a' in x;
}

function f2<T>(x: T) {
return x && x instanceof Object && 'a' in x;
}

function f3(x: {}) {
return x instanceof Object && 'a' in x;
}

function f4<T extends {}>(x: T) {
return x instanceof Object && 'a' in x;
}

function f5<T>(x: T & {}) {
return x instanceof Object && 'a' in x;
}

function f6<T extends {}>(x: T & {}) {
return x instanceof Object && 'a' in x;
}

function f7<T extends object>(x: T & {}) {
return x instanceof Object && 'a' in x;
}

59 changes: 59 additions & 0 deletions tests/baselines/reference/inKeywordAndUnknown.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,40 @@ function f(x: {}, y: unknown) {
}
y; // {}
}

// Repro from #51007

function isHTMLTable(table: unknown): boolean {
return !!table && table instanceof Object && 'html' in table;
}

function f1(x: unknown) {
return x && x instanceof Object && 'a' in x;
}

function f2<T>(x: T) {
return x && x instanceof Object && 'a' in x;
}

function f3(x: {}) {
return x instanceof Object && 'a' in x;
}

function f4<T extends {}>(x: T) {
return x instanceof Object && 'a' in x;
}

function f5<T>(x: T & {}) {
return x instanceof Object && 'a' in x;
}

function f6<T extends {}>(x: T & {}) {
return x instanceof Object && 'a' in x;
}

function f7<T extends object>(x: T & {}) {
return x instanceof Object && 'a' in x;
}


//// [inKeywordAndUnknown.js]
Expand All @@ -34,3 +68,28 @@ function f(x, y) {
}
y; // {}
}
// Repro from #51007
function isHTMLTable(table) {
return !!table && table instanceof Object && 'html' in table;
}
function f1(x) {
return x && x instanceof Object && 'a' in x;
}
function f2(x) {
return x && x instanceof Object && 'a' in x;
}
function f3(x) {
return x instanceof Object && 'a' in x;
}
function f4(x) {
return x instanceof Object && 'a' in x;
}
function f5(x) {
return x instanceof Object && 'a' in x;
}
function f6(x) {
return x instanceof Object && 'a' in x;
}
function f7(x) {
return x instanceof Object && 'a' in x;
}