Skip to content

Commit 73bef22

Browse files
andrewbranchandrewbranch
authored
A merged interface with an inherited member should satisfy an abstract base class member (microsoft#32539)
* A merged interface with an inherited member should satisfy an abstract base class member * Tighten up comments and names
1 parent b377e99 commit 73bef22

File tree

6 files changed

+236
-46
lines changed

6 files changed

+236
-46
lines changed

‎src/compiler/checker.ts

Lines changed: 56 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -29329,7 +29329,7 @@ namespace ts {
2932929329

2933029330
// NOTE: assignability is checked in checkClassDeclaration
2933129331
const baseProperties = getPropertiesOfType(baseType);
29332-
for (const baseProperty of baseProperties) {
29332+
basePropertyCheck: for (const baseProperty of baseProperties) {
2933329333
const base = getTargetSymbol(baseProperty);
2933429334

2933529335
if (base.flags & SymbolFlags.Prototype) {
@@ -29341,60 +29341,70 @@ namespace ts {
2934129341

2934229342
Debug.assert(!!derived, "derived should point to something, even if it is the base class' declaration.");
2934329343

29344-
if (derived) {
29345-
// In order to resolve whether the inherited method was overridden in the base class or not,
29346-
// we compare the Symbols obtained. Since getTargetSymbol returns the symbol on the *uninstantiated*
29347-
// type declaration, derived and base resolve to the same symbol even in the case of generic classes.
29348-
if (derived === base) {
29349-
// derived class inherits base without override/redeclaration
29350-
29351-
const derivedClassDecl = getClassLikeDeclarationOfSymbol(type.symbol)!;
29352-
29353-
// It is an error to inherit an abstract member without implementing it or being declared abstract.
29354-
// If there is no declaration for the derived class (as in the case of class expressions),
29355-
// then the class cannot be declared abstract.
29356-
if (baseDeclarationFlags & ModifierFlags.Abstract && (!derivedClassDecl || !hasModifier(derivedClassDecl, ModifierFlags.Abstract))) {
29357-
if (derivedClassDecl.kind === SyntaxKind.ClassExpression) {
29358-
error(derivedClassDecl, Diagnostics.Non_abstract_class_expression_does_not_implement_inherited_abstract_member_0_from_class_1,
29359-
symbolToString(baseProperty), typeToString(baseType));
29360-
}
29361-
else {
29362-
error(derivedClassDecl, Diagnostics.Non_abstract_class_0_does_not_implement_inherited_abstract_member_1_from_class_2,
29363-
typeToString(type), symbolToString(baseProperty), typeToString(baseType));
29344+
// In order to resolve whether the inherited method was overridden in the base class or not,
29345+
// we compare the Symbols obtained. Since getTargetSymbol returns the symbol on the *uninstantiated*
29346+
// type declaration, derived and base resolve to the same symbol even in the case of generic classes.
29347+
if (derived === base) {
29348+
// derived class inherits base without override/redeclaration
29349+
29350+
const derivedClassDecl = getClassLikeDeclarationOfSymbol(type.symbol)!;
29351+
29352+
// It is an error to inherit an abstract member without implementing it or being declared abstract.
29353+
// If there is no declaration for the derived class (as in the case of class expressions),
29354+
// then the class cannot be declared abstract.
29355+
if (baseDeclarationFlags & ModifierFlags.Abstract && (!derivedClassDecl || !hasModifier(derivedClassDecl, ModifierFlags.Abstract))) {
29356+
// Searches other base types for a declaration that would satisfy the inherited abstract member.
29357+
// (The class may have more than one base type via declaration merging with an interface with the
29358+
// same name.)
29359+
for (const otherBaseType of getBaseTypes(type)) {
29360+
if (otherBaseType === baseType) continue;
29361+
const baseSymbol = getPropertyOfObjectType(otherBaseType, base.escapedName);
29362+
const derivedElsewhere = baseSymbol && getTargetSymbol(baseSymbol);
29363+
if (derivedElsewhere && derivedElsewhere !== base) {
29364+
continue basePropertyCheck;
2936429365
}
2936529366
}
29366-
}
29367-
else {
29368-
// derived overrides base.
29369-
const derivedDeclarationFlags = getDeclarationModifierFlagsFromSymbol(derived);
29370-
if (baseDeclarationFlags & ModifierFlags.Private || derivedDeclarationFlags & ModifierFlags.Private) {
29371-
// either base or derived property is private - not override, skip it
29372-
continue;
29373-
}
2937429367

29375-
if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) {
29376-
// method is overridden with method or property/accessor is overridden with property/accessor - correct case
29377-
continue;
29368+
if (derivedClassDecl.kind === SyntaxKind.ClassExpression) {
29369+
error(derivedClassDecl, Diagnostics.Non_abstract_class_expression_does_not_implement_inherited_abstract_member_0_from_class_1,
29370+
symbolToString(baseProperty), typeToString(baseType));
2937829371
}
29379-
29380-
let errorMessage: DiagnosticMessage;
29381-
if (isPrototypeProperty(base)) {
29382-
if (derived.flags & SymbolFlags.Accessor) {
29383-
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor;
29384-
}
29385-
else {
29386-
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_property;
29387-
}
29372+
else {
29373+
error(derivedClassDecl, Diagnostics.Non_abstract_class_0_does_not_implement_inherited_abstract_member_1_from_class_2,
29374+
typeToString(type), symbolToString(baseProperty), typeToString(baseType));
2938829375
}
29389-
else if (base.flags & SymbolFlags.Accessor) {
29390-
errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function;
29376+
}
29377+
}
29378+
else {
29379+
// derived overrides base.
29380+
const derivedDeclarationFlags = getDeclarationModifierFlagsFromSymbol(derived);
29381+
if (baseDeclarationFlags & ModifierFlags.Private || derivedDeclarationFlags & ModifierFlags.Private) {
29382+
// either base or derived property is private - not override, skip it
29383+
continue;
29384+
}
29385+
29386+
if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) {
29387+
// method is overridden with method or property/accessor is overridden with property/accessor - correct case
29388+
continue;
29389+
}
29390+
29391+
let errorMessage: DiagnosticMessage;
29392+
if (isPrototypeProperty(base)) {
29393+
if (derived.flags & SymbolFlags.Accessor) {
29394+
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor;
2939129395
}
2939229396
else {
29393-
errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function;
29397+
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_property;
2939429398
}
29395-
29396-
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, typeToString(baseType), symbolToString(base), typeToString(type));
2939729399
}
29400+
else if (base.flags & SymbolFlags.Accessor) {
29401+
errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function;
29402+
}
29403+
else {
29404+
errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function;
29405+
}
29406+
29407+
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, typeToString(baseType), symbolToString(base), typeToString(type));
2939829408
}
2939929409
}
2940029410
}

‎tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.errors.txt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts(19,11): error TS2320: Interface 'IncorrectlyExtends' cannot simultaneously extend types 'BaseClass' and 'IncorrectGetters'.
2+
Named property 'bar' of types 'BaseClass' and 'IncorrectGetters' are not identical.
3+
4+
5+
==== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts (1 errors) ====
6+
abstract class BaseClass {
7+
abstract bar: number;
8+
}
9+
10+
class Broken extends BaseClass {}
11+
12+
// declaration merging should satisfy abstract bar
13+
interface IGetters {
14+
bar: number;
15+
}
16+
interface Broken extends IGetters {}
17+
18+
new Broken().bar
19+
20+
class IncorrectlyExtends extends BaseClass {}
21+
interface IncorrectGetters {
22+
bar: string;
23+
}
24+
interface IncorrectlyExtends extends IncorrectGetters {}
25+
~~~~~~~~~~~~~~~~~~
26+
!!! error TS2320: Interface 'IncorrectlyExtends' cannot simultaneously extend types 'BaseClass' and 'IncorrectGetters'.
27+
!!! error TS2320: Named property 'bar' of types 'BaseClass' and 'IncorrectGetters' are not identical.
28+

‎tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//// [mergedInheritedMembersSatisfyAbstractBase.ts]
2+
abstract class BaseClass {
3+
abstract bar: number;
4+
}
5+
6+
class Broken extends BaseClass {}
7+
8+
// declaration merging should satisfy abstract bar
9+
interface IGetters {
10+
bar: number;
11+
}
12+
interface Broken extends IGetters {}
13+
14+
new Broken().bar
15+
16+
class IncorrectlyExtends extends BaseClass {}
17+
interface IncorrectGetters {
18+
bar: string;
19+
}
20+
interface IncorrectlyExtends extends IncorrectGetters {}
21+
22+
23+
//// [mergedInheritedMembersSatisfyAbstractBase.js]
24+
var __extends = (this && this.__extends) || (function () {
25+
var extendStatics = function (d, b) {
26+
extendStatics = Object.setPrototypeOf ||
27+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
28+
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
29+
return extendStatics(d, b);
30+
};
31+
return function (d, b) {
32+
extendStatics(d, b);
33+
function __() { this.constructor = d; }
34+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
35+
};
36+
})();
37+
var BaseClass = /** @class */ (function () {
38+
function BaseClass() {
39+
}
40+
return BaseClass;
41+
}());
42+
var Broken = /** @class */ (function (_super) {
43+
__extends(Broken, _super);
44+
function Broken() {
45+
return _super !== null && _super.apply(this, arguments) || this;
46+
}
47+
return Broken;
48+
}(BaseClass));
49+
new Broken().bar;
50+
var IncorrectlyExtends = /** @class */ (function (_super) {
51+
__extends(IncorrectlyExtends, _super);
52+
function IncorrectlyExtends() {
53+
return _super !== null && _super.apply(this, arguments) || this;
54+
}
55+
return IncorrectlyExtends;
56+
}(BaseClass));

‎tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.symbols

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
=== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts ===
2+
abstract class BaseClass {
3+
>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0))
4+
5+
abstract bar: number;
6+
>bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26))
7+
}
8+
9+
class Broken extends BaseClass {}
10+
>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1))
11+
>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0))
12+
13+
// declaration merging should satisfy abstract bar
14+
interface IGetters {
15+
>IGetters : Symbol(IGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 4, 33))
16+
17+
bar: number;
18+
>bar : Symbol(IGetters.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 7, 20))
19+
}
20+
interface Broken extends IGetters {}
21+
>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1))
22+
>IGetters : Symbol(IGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 4, 33))
23+
24+
new Broken().bar
25+
>new Broken().bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26))
26+
>Broken : Symbol(Broken, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 2, 1), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 9, 1))
27+
>bar : Symbol(BaseClass.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 26))
28+
29+
class IncorrectlyExtends extends BaseClass {}
30+
>IncorrectlyExtends : Symbol(IncorrectlyExtends, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 12, 16), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 17, 1))
31+
>BaseClass : Symbol(BaseClass, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 0, 0))
32+
33+
interface IncorrectGetters {
34+
>IncorrectGetters : Symbol(IncorrectGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 14, 45))
35+
36+
bar: string;
37+
>bar : Symbol(IncorrectGetters.bar, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 15, 28))
38+
}
39+
interface IncorrectlyExtends extends IncorrectGetters {}
40+
>IncorrectlyExtends : Symbol(IncorrectlyExtends, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 12, 16), Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 17, 1))
41+
>IncorrectGetters : Symbol(IncorrectGetters, Decl(mergedInheritedMembersSatisfyAbstractBase.ts, 14, 45))
42+

‎tests/baselines/reference/mergedInheritedMembersSatisfyAbstractBase.types

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
=== tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts ===
2+
abstract class BaseClass {
3+
>BaseClass : BaseClass
4+
5+
abstract bar: number;
6+
>bar : number
7+
}
8+
9+
class Broken extends BaseClass {}
10+
>Broken : Broken
11+
>BaseClass : BaseClass
12+
13+
// declaration merging should satisfy abstract bar
14+
interface IGetters {
15+
bar: number;
16+
>bar : number
17+
}
18+
interface Broken extends IGetters {}
19+
20+
new Broken().bar
21+
>new Broken().bar : number
22+
>new Broken() : Broken
23+
>Broken : typeof Broken
24+
>bar : number
25+
26+
class IncorrectlyExtends extends BaseClass {}
27+
>IncorrectlyExtends : IncorrectlyExtends
28+
>BaseClass : BaseClass
29+
30+
interface IncorrectGetters {
31+
bar: string;
32+
>bar : string
33+
}
34+
interface IncorrectlyExtends extends IncorrectGetters {}
35+

‎tests/cases/conformance/interfaces/declarationMerging/mergedInheritedMembersSatisfyAbstractBase.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
abstract class BaseClass {
2+
abstract bar: number;
3+
}
4+
5+
class Broken extends BaseClass {}
6+
7+
// declaration merging should satisfy abstract bar
8+
interface IGetters {
9+
bar: number;
10+
}
11+
interface Broken extends IGetters {}
12+
13+
new Broken().bar
14+
15+
class IncorrectlyExtends extends BaseClass {}
16+
interface IncorrectGetters {
17+
bar: string;
18+
}
19+
interface IncorrectlyExtends extends IncorrectGetters {}

0 commit comments

Comments
 (0)