From 343e0f7fd980be44523fc5db9520e21ba4b0c1c4 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 3 Feb 2019 08:19:22 -0800 Subject: [PATCH 1/4] Fix NarrowTypeByTypeof, NarrowTypeByInstanceof, isPropertyInitializedInConstructor --- src/compiler/checker.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2e6259b668..84d4132e61 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15045,11 +15045,6 @@ namespace ts { return false; } - function hasNarrowableDeclaredType(expr: Node) { - const type = getDeclaredTypeOfReference(expr); - return !!(type && type.flags & TypeFlags.Union); - } - function findDiscriminantProperties(sourceProperties: Symbol[], target: Type): Symbol[] | undefined { let result: Symbol[] | undefined; for (const sourceProperty of sourceProperties) { @@ -16107,9 +16102,9 @@ namespace ts { // We have '==', '!=', '====', or !==' operator with 'typeof xxx' and string literal operands const target = getReferenceCandidate(typeOfExpr.expression); if (!isMatchingReference(reference, target)) { - // For a reference of the form 'x.y', where 'x' has a narrowable declared type, a - // 'typeof x === ...' type guard resets the narrowed type of 'y' to its declared type. - if (containsMatchingReference(reference, target) && hasNarrowableDeclaredType(target)) { + // For a reference of the form 'x.y', a 'typeof x === ...' type guard resets the + // narrowed type of 'y' to its declared type. + if (containsMatchingReference(reference, target)) { return declaredType; } return type; @@ -16264,9 +16259,9 @@ namespace ts { function narrowTypeByInstanceof(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type { const left = getReferenceCandidate(expr.left); if (!isMatchingReference(reference, left)) { - // For a reference of the form 'x.y', where 'x' has a narrowable declared type, an - // 'x instanceof T' type guard resets the narrowed type of 'y' to its declared type. - if (containsMatchingReference(reference, left) && hasNarrowableDeclaredType(left)) { + // For a reference of the form 'x.y', an 'x instanceof T' type guard resets the + // narrowed type of 'y' to its declared type. + if (containsMatchingReference(reference, left)) { return declaredType; } return type; @@ -27221,7 +27216,7 @@ namespace ts { reference.expression.parent = reference; reference.parent = constructor; reference.flowNode = constructor.returnFlowNode; - const flowType = getFlowTypeOfReference(reference, propType, getOptionalType(propType)); + const flowType = getFlowTypeOfReference(reference, getOptionalType(propType)); return !(getFalsyFlags(flowType) & TypeFlags.Undefined); } From 415c725bb91606b1446ada82dca6bac5a6052d03 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 3 Feb 2019 15:23:07 -0800 Subject: [PATCH 2/4] Make exception for synthetic 'this.x' in narrowTypeByInstanceof --- src/compiler/checker.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 84d4132e61..d5e4b29d44 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15045,6 +15045,10 @@ namespace ts { return false; } + function isSyntheticThisPropertyAccess(expr: Node) { + return isAccessExpression(expr) && expr.expression.kind === SyntaxKind.ThisKeyword && !!(expr.expression.flags & NodeFlags.Synthesized); + } + function findDiscriminantProperties(sourceProperties: Symbol[], target: Type): Symbol[] | undefined { let result: Symbol[] | undefined; for (const sourceProperty of sourceProperties) { @@ -16260,8 +16264,13 @@ namespace ts { const left = getReferenceCandidate(expr.left); if (!isMatchingReference(reference, left)) { // For a reference of the form 'x.y', an 'x instanceof T' type guard resets the - // narrowed type of 'y' to its declared type. - if (containsMatchingReference(reference, left)) { + // narrowed type of 'y' to its declared type. We do this because preceding 'x.y' + // references might reference a different 'y' property. However, we make an exception + // for property accesses where x is a synthetic 'this' expression, indicating that we + // were called from isPropertyInitializedInConstructor. Without this exception, + // initializations of 'this' properties that occur before a 'this instanceof XXX' + // check would not be considered. + if (containsMatchingReference(reference, left) && !isSyntheticThisPropertyAccess(reference)) { return declaredType; } return type; From 0c5471ba5cb1eab73661b21d9d9a5d19f7286f8a Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 3 Feb 2019 15:23:21 -0800 Subject: [PATCH 3/4] Add regression test --- .../cases/compiler/narrowingOfDottedNames.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/cases/compiler/narrowingOfDottedNames.ts b/tests/cases/compiler/narrowingOfDottedNames.ts index 4e504e4133..299dfff3be 100644 --- a/tests/cases/compiler/narrowingOfDottedNames.ts +++ b/tests/cases/compiler/narrowingOfDottedNames.ts @@ -57,3 +57,36 @@ class Foo2 constructor() { } } + +// Repro from #29513 + +class AInfo { + a_count: number = 1; +} + +class BInfo { + b_count: number = 1; +} + +class Base { + id: number = 0; +} + +class A2 extends Base { + info!: AInfo; +} + +class B2 extends Base { + info!: BInfo; +} + +let target: Base = null as any; + +while (target) { + if (target instanceof A2) { + target.info.a_count = 3; + } + else if (target instanceof B2) { + const j: BInfo = target.info; + } +} From 026225997e54dbaac834472d985d457b7c43dab6 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 3 Feb 2019 15:23:49 -0800 Subject: [PATCH 4/4] Accept new baselines --- .../narrowingOfDottedNames.errors.txt | 33 +++++++ .../reference/narrowingOfDottedNames.js | 88 +++++++++++++++++++ .../reference/narrowingOfDottedNames.symbols | 72 +++++++++++++++ .../reference/narrowingOfDottedNames.types | 77 ++++++++++++++++ 4 files changed, 270 insertions(+) diff --git a/tests/baselines/reference/narrowingOfDottedNames.errors.txt b/tests/baselines/reference/narrowingOfDottedNames.errors.txt index f5bf833c4e..1cc8fa4df8 100644 --- a/tests/baselines/reference/narrowingOfDottedNames.errors.txt +++ b/tests/baselines/reference/narrowingOfDottedNames.errors.txt @@ -64,4 +64,37 @@ tests/cases/compiler/narrowingOfDottedNames.ts(54,5): error TS2564: Property 'x' constructor() { } } + + // Repro from #29513 + + class AInfo { + a_count: number = 1; + } + + class BInfo { + b_count: number = 1; + } + + class Base { + id: number = 0; + } + + class A2 extends Base { + info!: AInfo; + } + + class B2 extends Base { + info!: BInfo; + } + + let target: Base = null as any; + + while (target) { + if (target instanceof A2) { + target.info.a_count = 3; + } + else if (target instanceof B2) { + const j: BInfo = target.info; + } + } \ No newline at end of file diff --git a/tests/baselines/reference/narrowingOfDottedNames.js b/tests/baselines/reference/narrowingOfDottedNames.js index 5d6ef2aaaa..4b4d9ada39 100644 --- a/tests/baselines/reference/narrowingOfDottedNames.js +++ b/tests/baselines/reference/narrowingOfDottedNames.js @@ -56,11 +56,57 @@ class Foo2 constructor() { } } + +// Repro from #29513 + +class AInfo { + a_count: number = 1; +} + +class BInfo { + b_count: number = 1; +} + +class Base { + id: number = 0; +} + +class A2 extends Base { + info!: AInfo; +} + +class B2 extends Base { + info!: BInfo; +} + +let target: Base = null as any; + +while (target) { + if (target instanceof A2) { + target.info.a_count = 3; + } + else if (target instanceof B2) { + const j: BInfo = target.info; + } +} //// [narrowingOfDottedNames.js] "use strict"; // Repro from #8383 +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 (b.hasOwnProperty(p)) d[p] = b[p]; }; + return extendStatics(d, b); + }; + return function (d, b) { + extendStatics(d, b); + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); + }; +})(); var A = /** @class */ (function () { function A() { } @@ -110,3 +156,45 @@ var Foo2 = /** @class */ (function () { } return Foo2; }()); +// Repro from #29513 +var AInfo = /** @class */ (function () { + function AInfo() { + this.a_count = 1; + } + return AInfo; +}()); +var BInfo = /** @class */ (function () { + function BInfo() { + this.b_count = 1; + } + return BInfo; +}()); +var Base = /** @class */ (function () { + function Base() { + this.id = 0; + } + return Base; +}()); +var A2 = /** @class */ (function (_super) { + __extends(A2, _super); + function A2() { + return _super !== null && _super.apply(this, arguments) || this; + } + return A2; +}(Base)); +var B2 = /** @class */ (function (_super) { + __extends(B2, _super); + function B2() { + return _super !== null && _super.apply(this, arguments) || this; + } + return B2; +}(Base)); +var target = null; +while (target) { + if (target instanceof A2) { + target.info.a_count = 3; + } + else if (target instanceof B2) { + var j = target.info; + } +} diff --git a/tests/baselines/reference/narrowingOfDottedNames.symbols b/tests/baselines/reference/narrowingOfDottedNames.symbols index a045c83cae..781c73f68f 100644 --- a/tests/baselines/reference/narrowingOfDottedNames.symbols +++ b/tests/baselines/reference/narrowingOfDottedNames.symbols @@ -129,3 +129,75 @@ class Foo2 } } +// Repro from #29513 + +class AInfo { +>AInfo : Symbol(AInfo, Decl(narrowingOfDottedNames.ts, 56, 1)) + + a_count: number = 1; +>a_count : Symbol(AInfo.a_count, Decl(narrowingOfDottedNames.ts, 60, 13)) +} + +class BInfo { +>BInfo : Symbol(BInfo, Decl(narrowingOfDottedNames.ts, 62, 1)) + + b_count: number = 1; +>b_count : Symbol(BInfo.b_count, Decl(narrowingOfDottedNames.ts, 64, 13)) +} + +class Base { +>Base : Symbol(Base, Decl(narrowingOfDottedNames.ts, 66, 1)) + + id: number = 0; +>id : Symbol(Base.id, Decl(narrowingOfDottedNames.ts, 68, 12)) +} + +class A2 extends Base { +>A2 : Symbol(A2, Decl(narrowingOfDottedNames.ts, 70, 1)) +>Base : Symbol(Base, Decl(narrowingOfDottedNames.ts, 66, 1)) + + info!: AInfo; +>info : Symbol(A2.info, Decl(narrowingOfDottedNames.ts, 72, 23)) +>AInfo : Symbol(AInfo, Decl(narrowingOfDottedNames.ts, 56, 1)) +} + +class B2 extends Base { +>B2 : Symbol(B2, Decl(narrowingOfDottedNames.ts, 74, 1)) +>Base : Symbol(Base, Decl(narrowingOfDottedNames.ts, 66, 1)) + + info!: BInfo; +>info : Symbol(B2.info, Decl(narrowingOfDottedNames.ts, 76, 23)) +>BInfo : Symbol(BInfo, Decl(narrowingOfDottedNames.ts, 62, 1)) +} + +let target: Base = null as any; +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) +>Base : Symbol(Base, Decl(narrowingOfDottedNames.ts, 66, 1)) + +while (target) { +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) + + if (target instanceof A2) { +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) +>A2 : Symbol(A2, Decl(narrowingOfDottedNames.ts, 70, 1)) + + target.info.a_count = 3; +>target.info.a_count : Symbol(AInfo.a_count, Decl(narrowingOfDottedNames.ts, 60, 13)) +>target.info : Symbol(A2.info, Decl(narrowingOfDottedNames.ts, 72, 23)) +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) +>info : Symbol(A2.info, Decl(narrowingOfDottedNames.ts, 72, 23)) +>a_count : Symbol(AInfo.a_count, Decl(narrowingOfDottedNames.ts, 60, 13)) + } + else if (target instanceof B2) { +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) +>B2 : Symbol(B2, Decl(narrowingOfDottedNames.ts, 74, 1)) + + const j: BInfo = target.info; +>j : Symbol(j, Decl(narrowingOfDottedNames.ts, 87, 13)) +>BInfo : Symbol(BInfo, Decl(narrowingOfDottedNames.ts, 62, 1)) +>target.info : Symbol(B2.info, Decl(narrowingOfDottedNames.ts, 76, 23)) +>target : Symbol(target, Decl(narrowingOfDottedNames.ts, 80, 3)) +>info : Symbol(B2.info, Decl(narrowingOfDottedNames.ts, 76, 23)) + } +} + diff --git a/tests/baselines/reference/narrowingOfDottedNames.types b/tests/baselines/reference/narrowingOfDottedNames.types index 5638a349fd..644aea6ba1 100644 --- a/tests/baselines/reference/narrowingOfDottedNames.types +++ b/tests/baselines/reference/narrowingOfDottedNames.types @@ -132,3 +132,80 @@ class Foo2 } } +// Repro from #29513 + +class AInfo { +>AInfo : AInfo + + a_count: number = 1; +>a_count : number +>1 : 1 +} + +class BInfo { +>BInfo : BInfo + + b_count: number = 1; +>b_count : number +>1 : 1 +} + +class Base { +>Base : Base + + id: number = 0; +>id : number +>0 : 0 +} + +class A2 extends Base { +>A2 : A2 +>Base : Base + + info!: AInfo; +>info : AInfo +} + +class B2 extends Base { +>B2 : B2 +>Base : Base + + info!: BInfo; +>info : BInfo +} + +let target: Base = null as any; +>target : Base +>null as any : any +>null : null + +while (target) { +>target : Base + + if (target instanceof A2) { +>target instanceof A2 : boolean +>target : Base +>A2 : typeof A2 + + target.info.a_count = 3; +>target.info.a_count = 3 : 3 +>target.info.a_count : number +>target.info : AInfo +>target : A2 +>info : AInfo +>a_count : number +>3 : 3 + } + else if (target instanceof B2) { +>target instanceof B2 : boolean +>target : Base +>B2 : typeof B2 + + const j: BInfo = target.info; +>j : BInfo +>target.info : BInfo +>target : B2 +>info : BInfo + } +} +