Skip to content

Improve handling of corner cases in narrowTypeByInstanceof #52592

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

Merged
merged 5 commits into from
Feb 6, 2023
Merged

Improve handling of corner cases in narrowTypeByInstanceof #52592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
31d335f
Improve handling of corner cases in `narrowTypeByInstanceof`
ahejlsberg Feb 3, 2023
1337f28
Add regression tests
ahejlsberg Feb 3, 2023
59fae7f
Keep existing handling of `any` on right hand side
ahejlsberg Feb 3, 2023
13bd783
Add extra test
ahejlsberg Feb 3, 2023
2577f94
Merge branch 'main' into fix52571
ahejlsberg Feb 3, 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
44 changes: 17 additions & 27 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27039,42 +27039,32 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
return type;
}

// Check that right operand is a function type with a prototype property
const rightType = getTypeOfExpression(expr.right);
if (!isTypeDerivedFrom(rightType, globalFunctionType)) {
return type;
}

let targetType: Type | undefined;
const prototypeProperty = getPropertyOfType(rightType, "prototype" as __String);
if (prototypeProperty) {
// Target type is type of the prototype property
const prototypePropertyType = getTypeOfSymbol(prototypeProperty);
if (!isTypeAny(prototypePropertyType)) {
targetType = prototypePropertyType;
}
}

// Don't narrow from 'any' if the target type is exactly 'Object' or 'Function'
if (isTypeAny(type) && (targetType === globalObjectType || targetType === globalFunctionType)) {
const instanceType = mapType(rightType, getInstanceType);
// Don't narrow from `any` if the target type is exactly `Object` or `Function`, and narrow
// in the false branch only if the target is a non-empty object type.
if (isTypeAny(type) && (instanceType === globalObjectType || instanceType === globalFunctionType) ||
!assumeTrue && !(instanceType.flags & TypeFlags.Object && !isEmptyAnonymousObjectType(instanceType))) {
return type;
}
return getNarrowedType(type, instanceType, assumeTrue, /*checkDerived*/ true);
}

if (!targetType) {
const constructSignatures = getSignaturesOfType(rightType, SignatureKind.Construct);
targetType = constructSignatures.length ?
getUnionType(map(constructSignatures, signature => getReturnTypeOfSignature(getErasedSignature(signature)))) :
emptyObjectType;
function getInstanceType(constructorType: Type) {
const prototypePropertyType = getTypeOfPropertyOfType(constructorType, "prototype" as __String);
if (prototypePropertyType && !isTypeAny(prototypePropertyType)) {
return prototypePropertyType;
}

// We can't narrow a union based off instanceof without negated types see #31576 for more info
if (!assumeTrue && rightType.flags & TypeFlags.Union) {
const nonConstructorTypeInUnion = find((rightType as UnionType).types, (t) => !isConstructorType(t));
if (!nonConstructorTypeInUnion) return type;
const constructSignatures = getSignaturesOfType(constructorType, SignatureKind.Construct);
if (constructSignatures.length) {
return getUnionType(map(constructSignatures, signature => getReturnTypeOfSignature(getErasedSignature(signature))));
}

return getNarrowedType(type, targetType, assumeTrue, /*checkDerived*/ true);
// We use the empty object type to indicate we don't know the type of objects created by
// this constructor function.
return emptyObjectType;
}

function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean, checkDerived: boolean) {
Expand Down
149 changes: 149 additions & 0 deletions tests/baselines/reference/narrowByInstanceof.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
//// [narrowByInstanceof.ts]
interface A { a: string }
interface B { b: string }
interface C { c: string }

type AA = {
(): void;
prototype: A;
}

type BB = {
new(): B;
}

function foo(x: A | B | C, A: AA, B: BB, AB: AA | BB) {
if (x instanceof A) {
x; // A
}
else {
x; // B | C
}
if (x instanceof B) {
x; // B
}
else {
x; // A | C
}
if (x instanceof AB) {
x; // A | B
}
else {
x; // A | B | C
}
}

function bar(target: any, Promise: any) {
if (target instanceof Promise) {
target.__then();
}
}

// Repro from #52571

class PersonMixin extends Function {
public check(o: any) {
return typeof o === "object" && o !== null && o instanceof Person;
}
}

const cls = new PersonMixin();

class Person {
work(): void { console.log("work") }
sayHi(): void { console.log("Hi") }
}

class Car {
sayHi(): void { console.log("Wof Wof") }
}

function test(o: Person | Car) {
if (o instanceof cls) {
console.log("Is Person");
(o as Person).work()
}
else {
console.log("Is Car")
o.sayHi();
}
}


//// [narrowByInstanceof.js]
"use strict";
var __extends = (this && this.__extends) || (function () {
var extendStatics = function (d, b) {
extendStatics = Object.setPrototypeOf ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
function (d, b) { for (var p in b) if (Object.prototype.hasOwnProperty.call(b, p)) d[p] = b[p]; };
return extendStatics(d, b);
};
return function (d, b) {
if (typeof b !== "function" && b !== null)
throw new TypeError("Class extends value " + String(b) + " is not a constructor or null");
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
function foo(x, A, B, AB) {
if (x instanceof A) {
x; // A
}
else {
x; // B | C
}
if (x instanceof B) {
x; // B
}
else {
x; // A | C
}
if (x instanceof AB) {
x; // A | B
}
else {
x; // A | B | C
}
}
function bar(target, Promise) {
if (target instanceof Promise) {
target.__then();
}
}
// Repro from #52571
var PersonMixin = /** @class */ (function (_super) {
__extends(PersonMixin, _super);
function PersonMixin() {
return _super !== null && _super.apply(this, arguments) || this;
}
PersonMixin.prototype.check = function (o) {
return typeof o === "object" && o !== null && o instanceof Person;
};
return PersonMixin;
}(Function));
var cls = new PersonMixin();
var Person = /** @class */ (function () {
function Person() {
}
Person.prototype.work = function () { console.log("work"); };
Person.prototype.sayHi = function () { console.log("Hi"); };
return Person;
}());
var Car = /** @class */ (function () {
function Car() {
}
Car.prototype.sayHi = function () { console.log("Wof Wof"); };
return Car;
}());
function test(o) {
if (o instanceof cls) {
console.log("Is Person");
o.work();
}
else {
console.log("Is Car");
o.sayHi();
}
}
174 changes: 174 additions & 0 deletions tests/baselines/reference/narrowByInstanceof.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
=== tests/cases/compiler/narrowByInstanceof.ts ===
interface A { a: string }
>A : Symbol(A, Decl(narrowByInstanceof.ts, 0, 0))
>a : Symbol(A.a, Decl(narrowByInstanceof.ts, 0, 13))

interface B { b: string }
>B : Symbol(B, Decl(narrowByInstanceof.ts, 0, 25))
>b : Symbol(B.b, Decl(narrowByInstanceof.ts, 1, 13))

interface C { c: string }
>C : Symbol(C, Decl(narrowByInstanceof.ts, 1, 25))
>c : Symbol(C.c, Decl(narrowByInstanceof.ts, 2, 13))

type AA = {
>AA : Symbol(AA, Decl(narrowByInstanceof.ts, 2, 25))

(): void;
prototype: A;
>prototype : Symbol(prototype, Decl(narrowByInstanceof.ts, 5, 13))
>A : Symbol(A, Decl(narrowByInstanceof.ts, 0, 0))
}

type BB = {
>BB : Symbol(BB, Decl(narrowByInstanceof.ts, 7, 1))

new(): B;
>B : Symbol(B, Decl(narrowByInstanceof.ts, 0, 25))
}

function foo(x: A | B | C, A: AA, B: BB, AB: AA | BB) {
>foo : Symbol(foo, Decl(narrowByInstanceof.ts, 11, 1))
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
>A : Symbol(A, Decl(narrowByInstanceof.ts, 0, 0))
>B : Symbol(B, Decl(narrowByInstanceof.ts, 0, 25))
>C : Symbol(C, Decl(narrowByInstanceof.ts, 1, 25))
>A : Symbol(A, Decl(narrowByInstanceof.ts, 13, 26))
>AA : Symbol(AA, Decl(narrowByInstanceof.ts, 2, 25))
>B : Symbol(B, Decl(narrowByInstanceof.ts, 13, 33))
>BB : Symbol(BB, Decl(narrowByInstanceof.ts, 7, 1))
>AB : Symbol(AB, Decl(narrowByInstanceof.ts, 13, 40))
>AA : Symbol(AA, Decl(narrowByInstanceof.ts, 2, 25))
>BB : Symbol(BB, Decl(narrowByInstanceof.ts, 7, 1))

if (x instanceof A) {
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
>A : Symbol(A, Decl(narrowByInstanceof.ts, 13, 26))

x; // A
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
else {
x; // B | C
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
if (x instanceof B) {
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
>B : Symbol(B, Decl(narrowByInstanceof.ts, 13, 33))

x; // B
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
else {
x; // A | C
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
if (x instanceof AB) {
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
>AB : Symbol(AB, Decl(narrowByInstanceof.ts, 13, 40))

x; // A | B
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
else {
x; // A | B | C
>x : Symbol(x, Decl(narrowByInstanceof.ts, 13, 13))
}
}

function bar(target: any, Promise: any) {
>bar : Symbol(bar, Decl(narrowByInstanceof.ts, 32, 1))
>target : Symbol(target, Decl(narrowByInstanceof.ts, 34, 13))
>Promise : Symbol(Promise, Decl(narrowByInstanceof.ts, 34, 25))

if (target instanceof Promise) {
>target : Symbol(target, Decl(narrowByInstanceof.ts, 34, 13))
>Promise : Symbol(Promise, Decl(narrowByInstanceof.ts, 34, 25))

target.__then();
>target : Symbol(target, Decl(narrowByInstanceof.ts, 34, 13))
}
}

// Repro from #52571

class PersonMixin extends Function {
>PersonMixin : Symbol(PersonMixin, Decl(narrowByInstanceof.ts, 38, 1))
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

public check(o: any) {
>check : Symbol(PersonMixin.check, Decl(narrowByInstanceof.ts, 42, 36))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 43, 17))

return typeof o === "object" && o !== null && o instanceof Person;
>o : Symbol(o, Decl(narrowByInstanceof.ts, 43, 17))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 43, 17))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 43, 17))
>Person : Symbol(Person, Decl(narrowByInstanceof.ts, 48, 30))
}
}

const cls = new PersonMixin();
>cls : Symbol(cls, Decl(narrowByInstanceof.ts, 48, 5))
>PersonMixin : Symbol(PersonMixin, Decl(narrowByInstanceof.ts, 38, 1))

class Person {
>Person : Symbol(Person, Decl(narrowByInstanceof.ts, 48, 30))

work(): void { console.log("work") }
>work : Symbol(Person.work, Decl(narrowByInstanceof.ts, 50, 14))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))

sayHi(): void { console.log("Hi") }
>sayHi : Symbol(Person.sayHi, Decl(narrowByInstanceof.ts, 51, 40))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
}

class Car {
>Car : Symbol(Car, Decl(narrowByInstanceof.ts, 53, 1))

sayHi(): void { console.log("Wof Wof") }
>sayHi : Symbol(Car.sayHi, Decl(narrowByInstanceof.ts, 55, 11))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
}

function test(o: Person | Car) {
>test : Symbol(test, Decl(narrowByInstanceof.ts, 57, 1))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 59, 14))
>Person : Symbol(Person, Decl(narrowByInstanceof.ts, 48, 30))
>Car : Symbol(Car, Decl(narrowByInstanceof.ts, 53, 1))

if (o instanceof cls) {
>o : Symbol(o, Decl(narrowByInstanceof.ts, 59, 14))
>cls : Symbol(cls, Decl(narrowByInstanceof.ts, 48, 5))

console.log("Is Person");
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))

(o as Person).work()
>(o as Person).work : Symbol(Person.work, Decl(narrowByInstanceof.ts, 50, 14))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 59, 14))
>Person : Symbol(Person, Decl(narrowByInstanceof.ts, 48, 30))
>work : Symbol(Person.work, Decl(narrowByInstanceof.ts, 50, 14))
}
else {
console.log("Is Car")
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))

o.sayHi();
>o.sayHi : Symbol(sayHi, Decl(narrowByInstanceof.ts, 51, 40), Decl(narrowByInstanceof.ts, 55, 11))
>o : Symbol(o, Decl(narrowByInstanceof.ts, 59, 14))
>sayHi : Symbol(sayHi, Decl(narrowByInstanceof.ts, 51, 40), Decl(narrowByInstanceof.ts, 55, 11))
}
}

Loading