From 3d64b9d7ac2bd443b9ffbb924361b426f58c5688 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 9 Jul 2018 16:58:55 -0700 Subject: [PATCH] Handle intersection types when looking up base types for visibility (#25418) * Handle intersection types when looking up base types for visibility * Extract protected constructor check to function and recur on intersections * Remove unneeded cast --- src/compiler/checker.ts | 44 ++++++++--- .../reference/noCrashOnMixin.errors.txt | 29 +++++++ tests/baselines/reference/noCrashOnMixin.js | 75 +++++++++++++++++++ .../reference/noCrashOnMixin.symbols | 48 ++++++++++++ .../baselines/reference/noCrashOnMixin.types | 51 +++++++++++++ tests/cases/compiler/noCrashOnMixin.ts | 23 ++++++ 6 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/noCrashOnMixin.errors.txt create mode 100644 tests/baselines/reference/noCrashOnMixin.js create mode 100644 tests/baselines/reference/noCrashOnMixin.symbols create mode 100644 tests/baselines/reference/noCrashOnMixin.types create mode 100644 tests/cases/compiler/noCrashOnMixin.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a72367a4cda..b8b215e32f9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -19345,6 +19345,38 @@ namespace ts { return resolveErrorCall(node); } + function typeHasProtectedAccessibleBase(target: Symbol, type: InterfaceType): boolean { + const baseTypes = getBaseTypes(type); + if (!length(baseTypes)) { + return false; + } + const firstBase = baseTypes[0]; + if (firstBase.flags & TypeFlags.Intersection) { + const types = (firstBase as IntersectionType).types; + const mixinCount = countWhere(types, isMixinConstructorType); + let i = 0; + for (const intersectionMember of (firstBase as IntersectionType).types) { + i++; + // We want to ignore mixin ctors + if (mixinCount === 0 || mixinCount === types.length && i === 0 || !isMixinConstructorType(intersectionMember)) { + if (getObjectFlags(intersectionMember) & (ObjectFlags.Class | ObjectFlags.Interface)) { + if (intersectionMember.symbol === target) { + return true; + } + if (typeHasProtectedAccessibleBase(target, intersectionMember as InterfaceType)) { + return true; + } + } + } + } + return false; + } + if (firstBase.symbol === target) { + return true; + } + return typeHasProtectedAccessibleBase(target, firstBase as InterfaceType); + } + function isConstructorAccessible(node: NewExpression, signature: Signature) { if (!signature || !signature.declaration) { return true; @@ -19364,16 +19396,10 @@ namespace ts { // A private or protected constructor can only be instantiated within its own class (or a subclass, for protected) if (!isNodeWithinClass(node, declaringClassDeclaration)) { const containingClass = getContainingClass(node); - if (containingClass) { + if (containingClass && modifiers & ModifierFlags.Protected) { const containingType = getTypeOfNode(containingClass); - let baseTypes = getBaseTypes(containingType as InterfaceType); - while (baseTypes.length) { - const baseType = baseTypes[0]; - if (modifiers & ModifierFlags.Protected && - baseType.symbol === declaration.parent.symbol) { - return true; - } - baseTypes = getBaseTypes(baseType as InterfaceType); + if (typeHasProtectedAccessibleBase(declaration.parent.symbol, containingType as InterfaceType)) { + return true; } } if (modifiers & ModifierFlags.Private) { diff --git a/tests/baselines/reference/noCrashOnMixin.errors.txt b/tests/baselines/reference/noCrashOnMixin.errors.txt new file mode 100644 index 00000000000..01baff88417 --- /dev/null +++ b/tests/baselines/reference/noCrashOnMixin.errors.txt @@ -0,0 +1,29 @@ +tests/cases/compiler/noCrashOnMixin.ts(21,9): error TS2674: Constructor of class 'Abstract' is protected and only accessible within the class declaration. + + +==== tests/cases/compiler/noCrashOnMixin.ts (1 errors) ==== + class Abstract { + protected constructor() { + } + } + + class Concrete extends Abstract { + } + + type Constructor = new (...args: any[]) => T; + + function Mixin(Base: TBase) { + return class extends Base { + }; + } + + class Empty { + } + + class CrashTrigger extends Mixin(Empty) { + public trigger() { + new Concrete(); + ~~~~~~~~~~~~~~ +!!! error TS2674: Constructor of class 'Abstract' is protected and only accessible within the class declaration. + } + } \ No newline at end of file diff --git a/tests/baselines/reference/noCrashOnMixin.js b/tests/baselines/reference/noCrashOnMixin.js new file mode 100644 index 00000000000..049b61e8a1c --- /dev/null +++ b/tests/baselines/reference/noCrashOnMixin.js @@ -0,0 +1,75 @@ +//// [noCrashOnMixin.ts] +class Abstract { + protected constructor() { + } +} + +class Concrete extends Abstract { +} + +type Constructor = new (...args: any[]) => T; + +function Mixin(Base: TBase) { + return class extends Base { + }; +} + +class Empty { +} + +class CrashTrigger extends Mixin(Empty) { + public trigger() { + new Concrete(); + } +} + +//// [noCrashOnMixin.js] +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 Abstract = /** @class */ (function () { + function Abstract() { + } + return Abstract; +}()); +var Concrete = /** @class */ (function (_super) { + __extends(Concrete, _super); + function Concrete() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Concrete; +}(Abstract)); +function Mixin(Base) { + return /** @class */ (function (_super) { + __extends(class_1, _super); + function class_1() { + return _super !== null && _super.apply(this, arguments) || this; + } + return class_1; + }(Base)); +} +var Empty = /** @class */ (function () { + function Empty() { + } + return Empty; +}()); +var CrashTrigger = /** @class */ (function (_super) { + __extends(CrashTrigger, _super); + function CrashTrigger() { + return _super !== null && _super.apply(this, arguments) || this; + } + CrashTrigger.prototype.trigger = function () { + new Concrete(); + }; + return CrashTrigger; +}(Mixin(Empty))); diff --git a/tests/baselines/reference/noCrashOnMixin.symbols b/tests/baselines/reference/noCrashOnMixin.symbols new file mode 100644 index 00000000000..3f99f99aa49 --- /dev/null +++ b/tests/baselines/reference/noCrashOnMixin.symbols @@ -0,0 +1,48 @@ +=== tests/cases/compiler/noCrashOnMixin.ts === +class Abstract { +>Abstract : Symbol(Abstract, Decl(noCrashOnMixin.ts, 0, 0)) + + protected constructor() { + } +} + +class Concrete extends Abstract { +>Concrete : Symbol(Concrete, Decl(noCrashOnMixin.ts, 3, 1)) +>Abstract : Symbol(Abstract, Decl(noCrashOnMixin.ts, 0, 0)) +} + +type Constructor = new (...args: any[]) => T; +>Constructor : Symbol(Constructor, Decl(noCrashOnMixin.ts, 6, 1)) +>T : Symbol(T, Decl(noCrashOnMixin.ts, 8, 17)) +>args : Symbol(args, Decl(noCrashOnMixin.ts, 8, 32)) +>T : Symbol(T, Decl(noCrashOnMixin.ts, 8, 17)) + +function Mixin(Base: TBase) { +>Mixin : Symbol(Mixin, Decl(noCrashOnMixin.ts, 8, 53)) +>TBase : Symbol(TBase, Decl(noCrashOnMixin.ts, 10, 15)) +>Constructor : Symbol(Constructor, Decl(noCrashOnMixin.ts, 6, 1)) +>Base : Symbol(Base, Decl(noCrashOnMixin.ts, 10, 42)) +>TBase : Symbol(TBase, Decl(noCrashOnMixin.ts, 10, 15)) + + return class extends Base { +>Base : Symbol(Base, Decl(noCrashOnMixin.ts, 10, 42)) + + }; +} + +class Empty { +>Empty : Symbol(Empty, Decl(noCrashOnMixin.ts, 13, 1)) +} + +class CrashTrigger extends Mixin(Empty) { +>CrashTrigger : Symbol(CrashTrigger, Decl(noCrashOnMixin.ts, 16, 1)) +>Mixin : Symbol(Mixin, Decl(noCrashOnMixin.ts, 8, 53)) +>Empty : Symbol(Empty, Decl(noCrashOnMixin.ts, 13, 1)) + + public trigger() { +>trigger : Symbol(CrashTrigger.trigger, Decl(noCrashOnMixin.ts, 18, 41)) + + new Concrete(); +>Concrete : Symbol(Concrete, Decl(noCrashOnMixin.ts, 3, 1)) + } +} diff --git a/tests/baselines/reference/noCrashOnMixin.types b/tests/baselines/reference/noCrashOnMixin.types new file mode 100644 index 00000000000..4d31e483b90 --- /dev/null +++ b/tests/baselines/reference/noCrashOnMixin.types @@ -0,0 +1,51 @@ +=== tests/cases/compiler/noCrashOnMixin.ts === +class Abstract { +>Abstract : Abstract + + protected constructor() { + } +} + +class Concrete extends Abstract { +>Concrete : Concrete +>Abstract : Abstract +} + +type Constructor = new (...args: any[]) => T; +>Constructor : Constructor +>T : T +>args : any[] +>T : T + +function Mixin(Base: TBase) { +>Mixin : >(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Mixin.(Anonymous class); } & TBase +>TBase : TBase +>Constructor : Constructor +>Base : TBase +>TBase : TBase + + return class extends Base { +>class extends Base { } : { new (...args: any[]): (Anonymous class); prototype: Mixin.(Anonymous class); } & TBase +>Base : {} + + }; +} + +class Empty { +>Empty : Empty +} + +class CrashTrigger extends Mixin(Empty) { +>CrashTrigger : CrashTrigger +>Mixin(Empty) : Mixin.(Anonymous class) & Empty +>Mixin : >(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Mixin.(Anonymous class); } & TBase +>Empty : typeof Empty + + public trigger() { +>trigger : () => void + + new Concrete(); +>new Concrete() : any +>Concrete : typeof Concrete + } +} diff --git a/tests/cases/compiler/noCrashOnMixin.ts b/tests/cases/compiler/noCrashOnMixin.ts new file mode 100644 index 00000000000..900e827fa46 --- /dev/null +++ b/tests/cases/compiler/noCrashOnMixin.ts @@ -0,0 +1,23 @@ +class Abstract { + protected constructor() { + } +} + +class Concrete extends Abstract { +} + +type Constructor = new (...args: any[]) => T; + +function Mixin(Base: TBase) { + return class extends Base { + }; +} + +class Empty { +} + +class CrashTrigger extends Mixin(Empty) { + public trigger() { + new Concrete(); + } +} \ No newline at end of file