From 73bef22f0bfd904b5ffbba03136ced954ff67ff8 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 1 Aug 2019 09:34:11 -0700 Subject: [PATCH] A merged interface with an inherited member should satisfy an abstract base class member (#32539) * A merged interface with an inherited member should satisfy an abstract base class member * Tighten up comments and names --- src/compiler/checker.ts | 102 ++++++++++-------- ...ritedMembersSatisfyAbstractBase.errors.txt | 28 +++++ ...rgedInheritedMembersSatisfyAbstractBase.js | 56 ++++++++++ ...nheritedMembersSatisfyAbstractBase.symbols | 42 ++++++++ ...dInheritedMembersSatisfyAbstractBase.types | 35 ++++++ ...rgedInheritedMembersSatisfyAbstractBase.ts | 19 ++++ 6 files changed, 236 insertions(+), 46 deletions(-) create mode 100644 tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.errors.txt create mode 100644 tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.js create mode 100644 tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.symbols create mode 100644 tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.types create mode 100644 tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 340c8c739a..92d026cd3d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -29329,7 +29329,7 @@ namespace ts { // NOTE: assignability is checked in checkClassDeclaration const baseProperties = getPropertiesOfType(baseType); - for (const baseProperty of baseProperties) { + basePropertyCheck: for (const baseProperty of baseProperties) { const base = getTargetSymbol(baseProperty); if (base.flags & SymbolFlags.Prototype) { @@ -29341,61 +29341,71 @@ namespace ts { Debug.assert(!!derived, "derived should point to something, even if it is the base class' declaration."); - if (derived) { - // In order to resolve whether the inherited method was overridden in the base class or not, - // we compare the Symbols obtained. Since getTargetSymbol returns the symbol on the *uninstantiated* - // type declaration, derived and base resolve to the same symbol even in the case of generic classes. - if (derived === base) { - // derived class inherits base without override/redeclaration + // In order to resolve whether the inherited method was overridden in the base class or not, + // we compare the Symbols obtained. Since getTargetSymbol returns the symbol on the *uninstantiated* + // type declaration, derived and base resolve to the same symbol even in the case of generic classes. + if (derived === base) { + // derived class inherits base without override/redeclaration - const derivedClassDecl = getClassLikeDeclarationOfSymbol(type.symbol)!; + const derivedClassDecl = getClassLikeDeclarationOfSymbol(type.symbol)!; - // It is an error to inherit an abstract member without implementing it or being declared abstract. - // If there is no declaration for the derived class (as in the case of class expressions), - // then the class cannot be declared abstract. - if (baseDeclarationFlags & ModifierFlags.Abstract && (!derivedClassDecl || !hasModifier(derivedClassDecl, ModifierFlags.Abstract))) { - if (derivedClassDecl.kind === SyntaxKind.ClassExpression) { - error(derivedClassDecl, Diagnostics.Non_abstract_class_expression_does_not_implement_inherited_abstract_member_0_from_class_1, - symbolToString(baseProperty), typeToString(baseType)); - } - else { - error(derivedClassDecl, Diagnostics.Non_abstract_class_0_does_not_implement_inherited_abstract_member_1_from_class_2, - typeToString(type), symbolToString(baseProperty), typeToString(baseType)); + // It is an error to inherit an abstract member without implementing it or being declared abstract. + // If there is no declaration for the derived class (as in the case of class expressions), + // then the class cannot be declared abstract. + if (baseDeclarationFlags & ModifierFlags.Abstract && (!derivedClassDecl || !hasModifier(derivedClassDecl, ModifierFlags.Abstract))) { + // Searches other base types for a declaration that would satisfy the inherited abstract member. + // (The class may have more than one base type via declaration merging with an interface with the + // same name.) + for (const otherBaseType of getBaseTypes(type)) { + if (otherBaseType === baseType) continue; + const baseSymbol = getPropertyOfObjectType(otherBaseType, base.escapedName); + const derivedElsewhere = baseSymbol && getTargetSymbol(baseSymbol); + if (derivedElsewhere && derivedElsewhere !== base) { + continue basePropertyCheck; } } - } - else { - // derived overrides base. - const derivedDeclarationFlags = getDeclarationModifierFlagsFromSymbol(derived); - if (baseDeclarationFlags & ModifierFlags.Private || derivedDeclarationFlags & ModifierFlags.Private) { - // either base or derived property is private - not override, skip it - continue; - } - if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) { - // method is overridden with method or property/accessor is overridden with property/accessor - correct case - continue; - } - - let errorMessage: DiagnosticMessage; - if (isPrototypeProperty(base)) { - if (derived.flags & SymbolFlags.Accessor) { - errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor; - } - else { - errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_property; - } - } - else if (base.flags & SymbolFlags.Accessor) { - errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function; + if (derivedClassDecl.kind === SyntaxKind.ClassExpression) { + error(derivedClassDecl, Diagnostics.Non_abstract_class_expression_does_not_implement_inherited_abstract_member_0_from_class_1, + symbolToString(baseProperty), typeToString(baseType)); } else { - errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function; + error(derivedClassDecl, Diagnostics.Non_abstract_class_0_does_not_implement_inherited_abstract_member_1_from_class_2, + typeToString(type), symbolToString(baseProperty), typeToString(baseType)); } - - error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, typeToString(baseType), symbolToString(base), typeToString(type)); } } + else { + // derived overrides base. + const derivedDeclarationFlags = getDeclarationModifierFlagsFromSymbol(derived); + if (baseDeclarationFlags & ModifierFlags.Private || derivedDeclarationFlags & ModifierFlags.Private) { + // either base or derived property is private - not override, skip it + continue; + } + + if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) { + // method is overridden with method or property/accessor is overridden with property/accessor - correct case + continue; + } + + let errorMessage: DiagnosticMessage; + if (isPrototypeProperty(base)) { + if (derived.flags & SymbolFlags.Accessor) { + errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor; + } + else { + errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_property; + } + } + else if (base.flags & SymbolFlags.Accessor) { + errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function; + } + else { + errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function; + } + + error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, typeToString(baseType), symbolToString(base), typeToString(type)); + } } } diff --git a/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.errors.txt b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.errors.txt new file mode 100644 index 0000000000..25e647f565 --- /dev/null +++ b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.errors.txt @@ -0,0 +1,28 @@ +tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts(19,11): error TS2320: Interface 'IncorrectlyExtends' cannot simultaneously extend types 'BaseClass' and 'IncorrectGetters'. + Named property 'bar' of types 'BaseClass' and 'IncorrectGetters' are not identical. + + +==== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts (1 errors) ==== + abstract class BaseClass { + abstract bar: number; + } + + class Broken extends BaseClass {} + + // declaration merging should satisfy abstract bar + interface IGetters { + bar: number; + } + interface Broken extends IGetters {} + + new Broken().bar + + class IncorrectlyExtends extends BaseClass {} + interface IncorrectGetters { + bar: string; + } + interface IncorrectlyExtends extends IncorrectGetters {} + ~~~~~~~~~~~~~~~~~~ +!!! error TS2320: Interface 'IncorrectlyExtends' cannot simultaneously extend types 'BaseClass' and 'IncorrectGetters'. +!!! error TS2320: Named property 'bar' of types 'BaseClass' and 'IncorrectGetters' are not identical. + \ No newline at end of file diff --git a/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.js b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.js new file mode 100644 index 0000000000..4f6814b752 --- /dev/null +++ b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.js @@ -0,0 +1,56 @@ +//// [mergedInheritedMembersSatisfyAbstractBase.ts] +abstract class BaseClass { + abstract bar: number; +} + +class Broken extends BaseClass {} + +// declaration merging should satisfy abstract bar +interface IGetters { + bar: number; +} +interface Broken extends IGetters {} + +new Broken().bar + +class IncorrectlyExtends extends BaseClass {} +interface IncorrectGetters { + bar: string; +} +interface IncorrectlyExtends extends IncorrectGetters {} + + +//// [mergedInheritedMembersSatisfyAbstractBase.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 BaseClass = /** @class */ (function () { + function BaseClass() { + } + return BaseClass; +}()); +var Broken = /** @class */ (function (_super) { + __extends(Broken, _super); + function Broken() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Broken; +}(BaseClass)); +new Broken().bar; +var IncorrectlyExtends = /** @class */ (function (_super) { + __extends(IncorrectlyExtends, _super); + function IncorrectlyExtends() { + return _super !== null && _super.apply(this, arguments) || this; + } + return IncorrectlyExtends; +}(BaseClass)); diff --git a/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.symbols b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.symbols new file mode 100644 index 0000000000..d44e0e03b0 --- /dev/null +++ b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.symbols @@ -0,0 +1,42 @@ +=== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts === +abstract class BaseClass { +>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0)) + + abstract bar: number; +>bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26)) +} + +class Broken extends BaseClass {} +>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1)) +>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0)) + +// declaration merging should satisfy abstract bar +interface IGetters { +>IGetters : Symbol(IGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 4, 33)) + + bar: number; +>bar : Symbol(IGetters.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 7, 20)) +} +interface Broken extends IGetters {} +>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1)) +>IGetters : Symbol(IGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 4, 33)) + +new Broken().bar +>new Broken().bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26)) +>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1)) +>bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26)) + +class IncorrectlyExtends extends BaseClass {} +>IncorrectlyExtends : Symbol(IncorrectlyExtends, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 12, 16), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 17, 1)) +>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0)) + +interface IncorrectGetters { +>IncorrectGetters : Symbol(IncorrectGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 14, 45)) + + bar: string; +>bar : Symbol(IncorrectGetters.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 15, 28)) +} +interface IncorrectlyExtends extends IncorrectGetters {} +>IncorrectlyExtends : Symbol(IncorrectlyExtends, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 12, 16), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 17, 1)) +>IncorrectGetters : Symbol(IncorrectGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 14, 45)) + diff --git a/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.types b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.types new file mode 100644 index 0000000000..03d0cdaa06 --- /dev/null +++ b/tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.types @@ -0,0 +1,35 @@ +=== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts === +abstract class BaseClass { +>BaseClass : BaseClass + + abstract bar: number; +>bar : number +} + +class Broken extends BaseClass {} +>Broken : Broken +>BaseClass : BaseClass + +// declaration merging should satisfy abstract bar +interface IGetters { + bar: number; +>bar : number +} +interface Broken extends IGetters {} + +new Broken().bar +>new Broken().bar : number +>new Broken() : Broken +>Broken : typeof Broken +>bar : number + +class IncorrectlyExtends extends BaseClass {} +>IncorrectlyExtends : IncorrectlyExtends +>BaseClass : BaseClass + +interface IncorrectGetters { + bar: string; +>bar : string +} +interface IncorrectlyExtends extends IncorrectGetters {} + diff --git a/tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts b/tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts new file mode 100644 index 0000000000..16b4375e79 --- /dev/null +++ b/tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts @@ -0,0 +1,19 @@ +abstract class BaseClass { + abstract bar: number; +} + +class Broken extends BaseClass {} + +// declaration merging should satisfy abstract bar +interface IGetters { + bar: number; +} +interface Broken extends IGetters {} + +new Broken().bar + +class IncorrectlyExtends extends BaseClass {} +interface IncorrectGetters { + bar: string; +} +interface IncorrectlyExtends extends IncorrectGetters {}