From 0862c3c6ddea99d3bf1aad288d78d9b274017ba1 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 3 Feb 2021 16:35:43 -0800 Subject: [PATCH 1/2] Accessors can override synthetic properties This is the simplest fix -- I'm going to check whether it's worthwhile to make it stricter next. Fixes #41347 --- src/compiler/checker.ts | 3 +- .../reference/accessorsOverrideProperty8.js | 79 +++++++++++++ .../accessorsOverrideProperty8.symbols | 111 ++++++++++++++++++ .../accessorsOverrideProperty8.types | 89 ++++++++++++++ .../accessorsOverrideProperty8.ts | 49 ++++++++ 5 files changed, 330 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/accessorsOverrideProperty8.js create mode 100644 tests/baselines/reference/accessorsOverrideProperty8.symbols create mode 100644 tests/baselines/reference/accessorsOverrideProperty8.types create mode 100644 tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c4b3d67ad6d96..bfde0cb4d45eb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -36244,8 +36244,9 @@ namespace ts { // property/accessor is overridden with property/accessor if (baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer) || base.valueDeclaration && base.valueDeclaration.parent.kind === SyntaxKind.InterfaceDeclaration + || getCheckFlags(base) & CheckFlags.Synthetic || derived.valueDeclaration && isBinaryExpression(derived.valueDeclaration)) { - // when the base property is abstract or from an interface, base/derived flags don't need to match + // when the base property is abstract, from an interface or from an intersection, base/derived flags don't need to match // same when the derived property is from an assignment continue; } diff --git a/tests/baselines/reference/accessorsOverrideProperty8.js b/tests/baselines/reference/accessorsOverrideProperty8.js new file mode 100644 index 0000000000000..be7b7ef353da5 --- /dev/null +++ b/tests/baselines/reference/accessorsOverrideProperty8.js @@ -0,0 +1,79 @@ +//// [accessorsOverrideProperty8.ts] +// #41347, based on microsoft/rushstack + +// Mixin utilities +export type Constructor = new (...args: any[]) => T; +export type PropertiesOf = { [K in keyof T]: T[K] }; + +interface IApiItemConstructor extends Constructor, PropertiesOf {} + +// Base class +class ApiItem { + public get members(): ReadonlyArray { + return []; + } +} + +// Normal subclass +class ApiEnumMember extends ApiItem { +} + +// Mixin base class +interface ApiItemContainerMixin extends ApiItem { + readonly members: ReadonlyArray; +} + +function ApiItemContainerMixin( + baseClass: TBaseClass +): TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) { + abstract class MixedClass extends baseClass implements ApiItemContainerMixin { + public constructor(...args: any[]) { + super(...args); + } + + public get members(): ReadonlyArray { + return []; + } + } + + return MixedClass; +} + +// Subclass inheriting from mixin +export class ApiEnum extends ApiItemContainerMixin(ApiItem) { + // This worked prior to TypeScript 4.0: + public get members(): ReadonlyArray { + return []; + } +} + + +//// [accessorsOverrideProperty8.js] +// #41347, based on microsoft/rushstack +// Base class +class ApiItem { + get members() { + return []; + } +} +// Normal subclass +class ApiEnumMember extends ApiItem { +} +function ApiItemContainerMixin(baseClass) { + class MixedClass extends baseClass { + constructor(...args) { + super(...args); + } + get members() { + return []; + } + } + return MixedClass; +} +// Subclass inheriting from mixin +export class ApiEnum extends ApiItemContainerMixin(ApiItem) { + // This worked prior to TypeScript 4.0: + get members() { + return []; + } +} diff --git a/tests/baselines/reference/accessorsOverrideProperty8.symbols b/tests/baselines/reference/accessorsOverrideProperty8.symbols new file mode 100644 index 0000000000000..b31c4de731cff --- /dev/null +++ b/tests/baselines/reference/accessorsOverrideProperty8.symbols @@ -0,0 +1,111 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts === +// #41347, based on microsoft/rushstack + +// Mixin utilities +export type Constructor = new (...args: any[]) => T; +>Constructor : Symbol(Constructor, Decl(accessorsOverrideProperty8.ts, 0, 0)) +>T : Symbol(T, Decl(accessorsOverrideProperty8.ts, 3, 24)) +>args : Symbol(args, Decl(accessorsOverrideProperty8.ts, 3, 39)) +>T : Symbol(T, Decl(accessorsOverrideProperty8.ts, 3, 24)) + +export type PropertiesOf = { [K in keyof T]: T[K] }; +>PropertiesOf : Symbol(PropertiesOf, Decl(accessorsOverrideProperty8.ts, 3, 60)) +>T : Symbol(T, Decl(accessorsOverrideProperty8.ts, 4, 25)) +>K : Symbol(K, Decl(accessorsOverrideProperty8.ts, 4, 33)) +>T : Symbol(T, Decl(accessorsOverrideProperty8.ts, 4, 25)) +>T : Symbol(T, Decl(accessorsOverrideProperty8.ts, 4, 25)) +>K : Symbol(K, Decl(accessorsOverrideProperty8.ts, 4, 33)) + +interface IApiItemConstructor extends Constructor, PropertiesOf {} +>IApiItemConstructor : Symbol(IApiItemConstructor, Decl(accessorsOverrideProperty8.ts, 4, 55)) +>Constructor : Symbol(Constructor, Decl(accessorsOverrideProperty8.ts, 0, 0)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) +>PropertiesOf : Symbol(PropertiesOf, Decl(accessorsOverrideProperty8.ts, 3, 60)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + +// Base class +class ApiItem { +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + + public get members(): ReadonlyArray { +>members : Symbol(ApiItem.members, Decl(accessorsOverrideProperty8.ts, 9, 15)) +>ReadonlyArray : Symbol(ReadonlyArray, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2016.array.include.d.ts, --, --)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + + return []; + } +} + +// Normal subclass +class ApiEnumMember extends ApiItem { +>ApiEnumMember : Symbol(ApiEnumMember, Decl(accessorsOverrideProperty8.ts, 13, 1)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) +} + +// Mixin base class +interface ApiItemContainerMixin extends ApiItem { +>ApiItemContainerMixin : Symbol(ApiItemContainerMixin, Decl(accessorsOverrideProperty8.ts, 22, 1), Decl(accessorsOverrideProperty8.ts, 17, 1)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + + readonly members: ReadonlyArray; +>members : Symbol(ApiItemContainerMixin.members, Decl(accessorsOverrideProperty8.ts, 20, 49)) +>ReadonlyArray : Symbol(ReadonlyArray, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2016.array.include.d.ts, --, --)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) +} + +function ApiItemContainerMixin( +>ApiItemContainerMixin : Symbol(ApiItemContainerMixin, Decl(accessorsOverrideProperty8.ts, 22, 1), Decl(accessorsOverrideProperty8.ts, 17, 1)) +>TBaseClass : Symbol(TBaseClass, Decl(accessorsOverrideProperty8.ts, 24, 31)) +>IApiItemConstructor : Symbol(IApiItemConstructor, Decl(accessorsOverrideProperty8.ts, 4, 55)) + + baseClass: TBaseClass +>baseClass : Symbol(baseClass, Decl(accessorsOverrideProperty8.ts, 24, 71)) +>TBaseClass : Symbol(TBaseClass, Decl(accessorsOverrideProperty8.ts, 24, 31)) + +): TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) { +>TBaseClass : Symbol(TBaseClass, Decl(accessorsOverrideProperty8.ts, 24, 31)) +>args : Symbol(args, Decl(accessorsOverrideProperty8.ts, 26, 22)) +>ApiItemContainerMixin : Symbol(ApiItemContainerMixin, Decl(accessorsOverrideProperty8.ts, 22, 1), Decl(accessorsOverrideProperty8.ts, 17, 1)) + + abstract class MixedClass extends baseClass implements ApiItemContainerMixin { +>MixedClass : Symbol(MixedClass, Decl(accessorsOverrideProperty8.ts, 26, 65)) +>baseClass : Symbol(baseClass, Decl(accessorsOverrideProperty8.ts, 24, 71)) +>ApiItemContainerMixin : Symbol(ApiItemContainerMixin, Decl(accessorsOverrideProperty8.ts, 22, 1), Decl(accessorsOverrideProperty8.ts, 17, 1)) + + public constructor(...args: any[]) { +>args : Symbol(args, Decl(accessorsOverrideProperty8.ts, 28, 23)) + + super(...args); +>super : Symbol(TBaseClass, Decl(accessorsOverrideProperty8.ts, 24, 31)) +>args : Symbol(args, Decl(accessorsOverrideProperty8.ts, 28, 23)) + } + + public get members(): ReadonlyArray { +>members : Symbol(MixedClass.members, Decl(accessorsOverrideProperty8.ts, 30, 5)) +>ReadonlyArray : Symbol(ReadonlyArray, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2016.array.include.d.ts, --, --)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + + return []; + } + } + + return MixedClass; +>MixedClass : Symbol(MixedClass, Decl(accessorsOverrideProperty8.ts, 26, 65)) +} + +// Subclass inheriting from mixin +export class ApiEnum extends ApiItemContainerMixin(ApiItem) { +>ApiEnum : Symbol(ApiEnum, Decl(accessorsOverrideProperty8.ts, 38, 1)) +>ApiItemContainerMixin : Symbol(ApiItemContainerMixin, Decl(accessorsOverrideProperty8.ts, 22, 1), Decl(accessorsOverrideProperty8.ts, 17, 1)) +>ApiItem : Symbol(ApiItem, Decl(accessorsOverrideProperty8.ts, 6, 91)) + + // This worked prior to TypeScript 4.0: + public get members(): ReadonlyArray { +>members : Symbol(ApiEnum.members, Decl(accessorsOverrideProperty8.ts, 41, 61)) +>ReadonlyArray : Symbol(ReadonlyArray, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2016.array.include.d.ts, --, --)) +>ApiEnumMember : Symbol(ApiEnumMember, Decl(accessorsOverrideProperty8.ts, 13, 1)) + + return []; + } +} + diff --git a/tests/baselines/reference/accessorsOverrideProperty8.types b/tests/baselines/reference/accessorsOverrideProperty8.types new file mode 100644 index 0000000000000..8e3e862d99ada --- /dev/null +++ b/tests/baselines/reference/accessorsOverrideProperty8.types @@ -0,0 +1,89 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts === +// #41347, based on microsoft/rushstack + +// Mixin utilities +export type Constructor = new (...args: any[]) => T; +>Constructor : Constructor +>args : any[] + +export type PropertiesOf = { [K in keyof T]: T[K] }; +>PropertiesOf : PropertiesOf + +interface IApiItemConstructor extends Constructor, PropertiesOf {} +>ApiItem : typeof ApiItem + +// Base class +class ApiItem { +>ApiItem : ApiItem + + public get members(): ReadonlyArray { +>members : readonly ApiItem[] + + return []; +>[] : never[] + } +} + +// Normal subclass +class ApiEnumMember extends ApiItem { +>ApiEnumMember : ApiEnumMember +>ApiItem : ApiItem +} + +// Mixin base class +interface ApiItemContainerMixin extends ApiItem { + readonly members: ReadonlyArray; +>members : readonly ApiItem[] +} + +function ApiItemContainerMixin( +>ApiItemContainerMixin : (baseClass: TBaseClass) => TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) + + baseClass: TBaseClass +>baseClass : TBaseClass + +): TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) { +>args : any[] + + abstract class MixedClass extends baseClass implements ApiItemContainerMixin { +>MixedClass : MixedClass +>baseClass : ApiItem + + public constructor(...args: any[]) { +>args : any[] + + super(...args); +>super(...args) : void +>super : TBaseClass +>...args : any +>args : any[] + } + + public get members(): ReadonlyArray { +>members : readonly ApiItem[] + + return []; +>[] : never[] + } + } + + return MixedClass; +>MixedClass : ((abstract new (...args: any[]) => MixedClass) & { prototype: ApiItemContainerMixin.MixedClass; }) & TBaseClass +} + +// Subclass inheriting from mixin +export class ApiEnum extends ApiItemContainerMixin(ApiItem) { +>ApiEnum : ApiEnum +>ApiItemContainerMixin(ApiItem) : ApiItem & ApiItemContainerMixin +>ApiItemContainerMixin : (baseClass: TBaseClass) => TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) +>ApiItem : typeof ApiItem + + // This worked prior to TypeScript 4.0: + public get members(): ReadonlyArray { +>members : readonly ApiEnumMember[] + + return []; +>[] : never[] + } +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts new file mode 100644 index 0000000000000..9dfa1a833b0c8 --- /dev/null +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty8.ts @@ -0,0 +1,49 @@ +// @strict: true +// @target: es2017 +// #41347, based on microsoft/rushstack + +// Mixin utilities +export type Constructor = new (...args: any[]) => T; +export type PropertiesOf = { [K in keyof T]: T[K] }; + +interface IApiItemConstructor extends Constructor, PropertiesOf {} + +// Base class +class ApiItem { + public get members(): ReadonlyArray { + return []; + } +} + +// Normal subclass +class ApiEnumMember extends ApiItem { +} + +// Mixin base class +interface ApiItemContainerMixin extends ApiItem { + readonly members: ReadonlyArray; +} + +function ApiItemContainerMixin( + baseClass: TBaseClass +): TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) { + abstract class MixedClass extends baseClass implements ApiItemContainerMixin { + public constructor(...args: any[]) { + super(...args); + } + + public get members(): ReadonlyArray { + return []; + } + } + + return MixedClass; +} + +// Subclass inheriting from mixin +export class ApiEnum extends ApiItemContainerMixin(ApiItem) { + // This worked prior to TypeScript 4.0: + public get members(): ReadonlyArray { + return []; + } +} From 8cd12bb694cdcbaef82c8676bb5dc9c392e841bf Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 3 Feb 2021 17:04:14 -0800 Subject: [PATCH 2/2] use stricter check described in issue --- src/compiler/checker.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index bfde0cb4d45eb..90e40f8aca866 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6899,7 +6899,7 @@ namespace ts { ...!length(baseTypes) ? [] : [factory.createHeritageClause(SyntaxKind.ExtendsKeyword, map(baseTypes, b => serializeBaseType(b, staticBaseType, localName)))], ...!length(implementsExpressions) ? [] : [factory.createHeritageClause(SyntaxKind.ImplementsKeyword, implementsExpressions)] ]; - const symbolProps = getNonInterhitedProperties(classType, baseTypes, getPropertiesOfType(classType)); + const symbolProps = getNonInheritedProperties(classType, baseTypes, getPropertiesOfType(classType)); const publicSymbolProps = filter(symbolProps, s => { // `valueDeclaration` could be undefined if inherited from // a union/intersection base type, but inherited properties @@ -36242,9 +36242,8 @@ namespace ts { const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor; if (basePropertyFlags && derivedPropertyFlags) { // property/accessor is overridden with property/accessor - if (baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer) - || base.valueDeclaration && base.valueDeclaration.parent.kind === SyntaxKind.InterfaceDeclaration - || getCheckFlags(base) & CheckFlags.Synthetic + if (base.valueDeclaration && isPropertyAbstractOrFromInterface(base.valueDeclaration, baseDeclarationFlags) + || getCheckFlags(base) & CheckFlags.Synthetic && base.declarations.some(d => isPropertyAbstractOrFromInterface(d, baseDeclarationFlags)) || derived.valueDeclaration && isBinaryExpression(derived.valueDeclaration)) { // when the base property is abstract, from an interface or from an intersection, base/derived flags don't need to match // same when the derived property is from an assignment @@ -36304,7 +36303,12 @@ namespace ts { } } - function getNonInterhitedProperties(type: InterfaceType, baseTypes: BaseType[], properties: Symbol[]) { + function isPropertyAbstractOrFromInterface(declaration: Declaration, baseDeclarationFlags: ModifierFlags) { + return baseDeclarationFlags & ModifierFlags.Abstract && (!isPropertyDeclaration(declaration) || !declaration.initializer) + || declaration.parent.kind === SyntaxKind.InterfaceDeclaration; + } + + function getNonInheritedProperties(type: InterfaceType, baseTypes: BaseType[], properties: Symbol[]) { if (!length(baseTypes)) { return properties; }