Skip to content

Commit c4f334a

Browse files
sandersnsandersn
committed
Updates from design review + fix ancient bug
1. Don't error when overriding properties from interfaces. 2. Fix error when overriding methods with other things. This had no tests so I assume that the code was always dead and never worked.
1 parent c26785e commit c4f334a

19 files changed

+361
-7
lines changed

‎src/compiler/checker.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29513,19 +29513,20 @@ namespace ts {
2951329513
// either base or derived property is private - not override, skip it
2951429514
continue;
2951529515
}
29516-
if (isPrototypeProperty(base)) {
29517-
// method is overridden with method - correct case
29518-
continue;
29519-
}
2952029516
let errorMessage: DiagnosticMessage;
2952129517
const basePropertyFlags = base.flags & SymbolFlags.PropertyOrAccessor;
2952229518
const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor;
2952329519
if (basePropertyFlags && derivedPropertyFlags) {
2952429520
// property/accessor is overridden with property/accessor
29525-
if (!(baseDeclarationFlags & ModifierFlags.Abstract) && basePropertyFlags !== SymbolFlags.Property && derivedPropertyFlags === SymbolFlags.Property) {
29521+
if (baseDeclarationFlags & ModifierFlags.Abstract
29522+
|| getObjectFlags(getTargetType(baseType)) & ObjectFlags.Interface) {
29523+
// when the base property is abstract or from an interface, base/derived flags don't need to match
29524+
continue;
29525+
}
29526+
if (basePropertyFlags !== SymbolFlags.Property && derivedPropertyFlags === SymbolFlags.Property) {
2952629527
errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_property;
2952729528
}
29528-
else if (!(baseDeclarationFlags & ModifierFlags.Abstract) && basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property) {
29529+
else if (basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property) {
2952929530
errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_accessor;
2953029531
}
2953129532
else {
@@ -29534,7 +29535,11 @@ namespace ts {
2953429535
}
2953529536
}
2953629537
else if (isPrototypeProperty(base)) {
29537-
if (derived.flags & SymbolFlags.Accessor) {
29538+
if (isPrototypeProperty(derived)) {
29539+
// method is overridden with method -- correct case
29540+
continue;
29541+
}
29542+
else if (derived.flags & SymbolFlags.Accessor) {
2953829543
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor;
2953929544
}
2954029545
else {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideMethod.ts(5,9): error TS2423: Class 'A' defines instance member function 'm', but extended class 'B' defines it as instance member accessor.
2+
3+
4+
==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideMethod.ts (1 errors) ====
5+
class A {
6+
m() { }
7+
}
8+
class B extends A {
9+
get m() { return () => 1 }
10+
~
11+
!!! error TS2423: Class 'A' defines instance member function 'm', but extended class 'B' defines it as instance member accessor.
12+
}
13+

‎tests/baselines/reference/accessorsOverrideMethod.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//// [accessorsOverrideMethod.ts]
2+
class A {
3+
m() { }
4+
}
5+
class B extends A {
6+
get m() { return () => 1 }
7+
}
8+
9+
10+
//// [accessorsOverrideMethod.js]
11+
class A {
12+
m() { }
13+
}
14+
class B extends A {
15+
get m() { return () => 1; }
16+
}

‎tests/baselines/reference/accessorsOverrideMethod.symbols‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideMethod.ts ===
2+
class A {
3+
>A : Symbol(A, Decl(accessorsOverrideMethod.ts, 0, 0))
4+
5+
m() { }
6+
>m : Symbol(A.m, Decl(accessorsOverrideMethod.ts, 0, 9))
7+
}
8+
class B extends A {
9+
>B : Symbol(B, Decl(accessorsOverrideMethod.ts, 2, 1))
10+
>A : Symbol(A, Decl(accessorsOverrideMethod.ts, 0, 0))
11+
12+
get m() { return () => 1 }
13+
>m : Symbol(B.m, Decl(accessorsOverrideMethod.ts, 3, 19))
14+
}
15+

‎tests/baselines/reference/accessorsOverrideMethod.types‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideMethod.ts ===
2+
class A {
3+
>A : A
4+
5+
m() { }
6+
>m : () => void
7+
}
8+
class B extends A {
9+
>B : B
10+
>A : A
11+
12+
get m() { return () => 1 }
13+
>m : () => number
14+
>() => 1 : () => number
15+
>1 : 1
16+
}
17+

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
tests/cases/compiler/a.js(14,10): error TS2424: Class 'A' defines instance member function 'foo', but extended class 'B' defines it as instance member property.
2+
3+
4+
==== tests/cases/compiler/a.js (1 errors) ====
5+
// @ts-check
6+
class A {
7+
constructor() {
8+
9+
}
10+
foo() {
11+
return 4;
12+
}
13+
}
14+
15+
class B extends A {
16+
constructor() {
17+
super();
18+
this.foo = () => 3;
19+
~~~
20+
!!! error TS2424: Class 'A' defines instance member function 'foo', but extended class 'B' defines it as instance member property.
21+
}
22+
}
23+
24+
const i = new B();
25+
i.foo();

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
tests/cases/compiler/inheritanceMemberAccessorOverridingMethod.ts(8,9): error TS2423: Class 'a' defines instance member function 'x', but extended class 'b' defines it as instance member accessor.
2+
3+
4+
==== tests/cases/compiler/inheritanceMemberAccessorOverridingMethod.ts (1 errors) ====
5+
class a {
6+
x() {
7+
return "20";
8+
}
9+
}
10+
11+
class b extends a {
12+
get x() {
13+
~
14+
!!! error TS2423: Class 'a' defines instance member function 'x', but extended class 'b' defines it as instance member accessor.
15+
return () => "20";
16+
}
17+
set x(aValue) {
18+
19+
}
20+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
tests/cases/compiler/inheritanceMemberPropertyOverridingMethod.ts(8,5): error TS2424: Class 'a' defines instance member function 'x', but extended class 'b' defines it as instance member property.
2+
3+
4+
==== tests/cases/compiler/inheritanceMemberPropertyOverridingMethod.ts (1 errors) ====
5+
class a {
6+
x() {
7+
return "20";
8+
}
9+
}
10+
11+
class b extends a {
12+
x: () => string;
13+
~
14+
!!! error TS2424: Class 'a' defines instance member function 'x', but extended class 'b' defines it as instance member property.
15+
}

‎tests/baselines/reference/overrideInterfaceProperty.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//// [overrideInterfaceProperty.ts]
2+
interface Mup<K, V> {
3+
readonly size: number;
4+
}
5+
interface MupConstructor {
6+
new(): Mup<any, any>;
7+
new<K, V>(entries?: readonly (readonly [K, V])[] | null): Mup<K, V>;
8+
readonly prototype: Mup<any, any>;
9+
}
10+
declare var Mup: MupConstructor;
11+
12+
class Sizz extends Mup {
13+
// ok, because Mup is an interface
14+
get size() { return 0 }
15+
}
16+
class Kasizz extends Mup {
17+
size = -1
18+
}
19+
20+
21+
//// [overrideInterfaceProperty.js]
22+
class Sizz extends Mup {
23+
// ok, because Mup is an interface
24+
get size() { return 0; }
25+
}
26+
class Kasizz extends Mup {
27+
constructor() {
28+
super(...arguments);
29+
this.size = -1;
30+
}
31+
}

‎tests/baselines/reference/overrideInterfaceProperty.symbols‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
=== tests/cases/conformance/classes/propertyMemberDeclarations/overrideInterfaceProperty.ts ===
2+
interface Mup<K, V> {
3+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
4+
>K : Symbol(K, Decl(overrideInterfaceProperty.ts, 0, 14))
5+
>V : Symbol(V, Decl(overrideInterfaceProperty.ts, 0, 16))
6+
7+
readonly size: number;
8+
>size : Symbol(Mup.size, Decl(overrideInterfaceProperty.ts, 0, 21))
9+
}
10+
interface MupConstructor {
11+
>MupConstructor : Symbol(MupConstructor, Decl(overrideInterfaceProperty.ts, 2, 1))
12+
13+
new(): Mup<any, any>;
14+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
15+
16+
new<K, V>(entries?: readonly (readonly [K, V])[] | null): Mup<K, V>;
17+
>K : Symbol(K, Decl(overrideInterfaceProperty.ts, 5, 8))
18+
>V : Symbol(V, Decl(overrideInterfaceProperty.ts, 5, 10))
19+
>entries : Symbol(entries, Decl(overrideInterfaceProperty.ts, 5, 14))
20+
>K : Symbol(K, Decl(overrideInterfaceProperty.ts, 5, 8))
21+
>V : Symbol(V, Decl(overrideInterfaceProperty.ts, 5, 10))
22+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
23+
>K : Symbol(K, Decl(overrideInterfaceProperty.ts, 5, 8))
24+
>V : Symbol(V, Decl(overrideInterfaceProperty.ts, 5, 10))
25+
26+
readonly prototype: Mup<any, any>;
27+
>prototype : Symbol(MupConstructor.prototype, Decl(overrideInterfaceProperty.ts, 5, 72))
28+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
29+
}
30+
declare var Mup: MupConstructor;
31+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
32+
>MupConstructor : Symbol(MupConstructor, Decl(overrideInterfaceProperty.ts, 2, 1))
33+
34+
class Sizz extends Mup {
35+
>Sizz : Symbol(Sizz, Decl(overrideInterfaceProperty.ts, 8, 32))
36+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
37+
38+
// ok, because Mup is an interface
39+
get size() { return 0 }
40+
>size : Symbol(Sizz.size, Decl(overrideInterfaceProperty.ts, 10, 24))
41+
}
42+
class Kasizz extends Mup {
43+
>Kasizz : Symbol(Kasizz, Decl(overrideInterfaceProperty.ts, 13, 1))
44+
>Mup : Symbol(Mup, Decl(overrideInterfaceProperty.ts, 0, 0), Decl(overrideInterfaceProperty.ts, 8, 11))
45+
46+
size = -1
47+
>size : Symbol(Kasizz.size, Decl(overrideInterfaceProperty.ts, 14, 26))
48+
}
49+

0 commit comments

Comments
 (0)