Skip to content

Commit b0c2860

Browse files
authored
ignore static and declared member if checking override (#43569)
* ignore static member if checking override * Ignore declared member when check override * Check static override too * Add more tests
1 parent a354a77 commit b0c2860

File tree

12 files changed

+327
-20
lines changed

12 files changed

+327
-20
lines changed

src/compiler/checker.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36943,7 +36943,7 @@ namespace ts {
3694336943
}
3694436944
}
3694536945

36946-
checkMembersForMissingOverrideModifier(node, type, typeWithThis);
36946+
checkMembersForMissingOverrideModifier(node, type, typeWithThis, staticType);
3694736947

3694836948
const implementedTypeNodes = getEffectiveImplementsTypeNodes(node);
3694936949
if (implementedTypeNodes) {
@@ -36980,13 +36980,18 @@ namespace ts {
3698036980
}
3698136981
}
3698236982

36983-
function checkMembersForMissingOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type) {
36983+
function checkMembersForMissingOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type, staticType: ObjectType) {
3698436984
const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
3698536985
const baseTypeNode = getEffectiveBaseTypeNode(node);
3698636986
const baseTypes = baseTypeNode && getBaseTypes(type);
3698736987
const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;
36988+
const baseStaticType = getBaseConstructorTypeOfClass(type);
3698836989

3698936990
for (const member of node.members) {
36991+
if (hasAmbientModifier(member)) {
36992+
continue;
36993+
}
36994+
3699036995
if (isConstructorDeclaration(member)) {
3699136996
forEach(member.parameters, param => {
3699236997
if (isParameterPropertyDeclaration(param, member)) {
@@ -36996,17 +37001,22 @@ namespace ts {
3699637001
}
3699737002
checkClassMember(member);
3699837003
}
37004+
3699937005
function checkClassMember(member: ClassElement | ParameterPropertyDeclaration, memberIsParameterProperty?: boolean) {
3700037006
const hasOverride = hasOverrideModifier(member);
37007+
const hasStatic = hasStaticModifier(member);
3700137008
if (baseWithThis && (hasOverride || compilerOptions.noImplicitOverride)) {
3700237009
const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
3700337010
if (!declaredProp) {
3700437011
return;
3700537012
}
3700637013

37014+
const thisType = hasStatic ? staticType : typeWithThis;
37015+
const baseType = hasStatic ? baseStaticType : baseWithThis;
37016+
const prop = getPropertyOfType(thisType, declaredProp.escapedName);
37017+
const baseProp = getPropertyOfType(baseType, declaredProp.escapedName);
37018+
3700737019
const baseClassName = typeToString(baseWithThis);
37008-
const prop = getPropertyOfType(typeWithThis, declaredProp.escapedName);
37009-
const baseProp = getPropertyOfType(baseWithThis, declaredProp.escapedName);
3701037020
if (prop && !baseProp && hasOverride) {
3701137021
error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0, baseClassName);
3701237022
}
@@ -40443,9 +40453,6 @@ namespace ts {
4044340453
else if (flags & ModifierFlags.Ambient) {
4044440454
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "override", "declare");
4044540455
}
40446-
else if (flags & ModifierFlags.Static) {
40447-
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
40448-
}
4044940456
else if (flags & ModifierFlags.Readonly) {
4045040457
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "override", "readonly");
4045140458
}
@@ -40500,9 +40507,6 @@ namespace ts {
4050040507
else if (flags & ModifierFlags.Readonly) {
4050140508
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "readonly");
4050240509
}
40503-
else if (flags & ModifierFlags.Override) {
40504-
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
40505-
}
4050640510
else if (flags & ModifierFlags.Async) {
4050740511
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "async");
4050840512
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
tests/cases/conformance/override/override13.ts(7,5): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
2+
tests/cases/conformance/override/override13.ts(13,12): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
3+
tests/cases/conformance/override/override13.ts(19,5): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
4+
tests/cases/conformance/override/override13.ts(25,12): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
5+
6+
7+
==== tests/cases/conformance/override/override13.ts (4 errors) ====
8+
class Foo {
9+
property = 1
10+
static staticProperty = 2
11+
}
12+
13+
class SubFoo extends Foo {
14+
property = 42;
15+
~~~~~~~~
16+
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
17+
staticProperty = 42;
18+
}
19+
20+
class StaticSubFoo extends Foo {
21+
static property = 42;
22+
static staticProperty = 42;
23+
~~~~~~~~~~~~~~
24+
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
25+
}
26+
27+
class Intermediate extends Foo {}
28+
29+
class Derived extends Intermediate {
30+
property = 42;
31+
~~~~~~~~
32+
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
33+
staticProperty = 42;
34+
}
35+
36+
class StaticDerived extends Intermediate {
37+
static property = 42;
38+
static staticProperty = 42;
39+
~~~~~~~~~~~~~~
40+
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
41+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
//// [override13.ts]
2+
class Foo {
3+
property = 1
4+
static staticProperty = 2
5+
}
6+
7+
class SubFoo extends Foo {
8+
property = 42;
9+
staticProperty = 42;
10+
}
11+
12+
class StaticSubFoo extends Foo {
13+
static property = 42;
14+
static staticProperty = 42;
15+
}
16+
17+
class Intermediate extends Foo {}
18+
19+
class Derived extends Intermediate {
20+
property = 42;
21+
staticProperty = 42;
22+
}
23+
24+
class StaticDerived extends Intermediate {
25+
static property = 42;
26+
static staticProperty = 42;
27+
}
28+
29+
//// [override13.js]
30+
class Foo {
31+
property = 1;
32+
static staticProperty = 2;
33+
}
34+
class SubFoo extends Foo {
35+
property = 42;
36+
staticProperty = 42;
37+
}
38+
class StaticSubFoo extends Foo {
39+
static property = 42;
40+
static staticProperty = 42;
41+
}
42+
class Intermediate extends Foo {
43+
}
44+
class Derived extends Intermediate {
45+
property = 42;
46+
staticProperty = 42;
47+
}
48+
class StaticDerived extends Intermediate {
49+
static property = 42;
50+
static staticProperty = 42;
51+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
=== tests/cases/conformance/override/override13.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))
4+
5+
property = 1
6+
>property : Symbol(Foo.property, Decl(override13.ts, 0, 11))
7+
8+
static staticProperty = 2
9+
>staticProperty : Symbol(Foo.staticProperty, Decl(override13.ts, 1, 16))
10+
}
11+
12+
class SubFoo extends Foo {
13+
>SubFoo : Symbol(SubFoo, Decl(override13.ts, 3, 1))
14+
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))
15+
16+
property = 42;
17+
>property : Symbol(SubFoo.property, Decl(override13.ts, 5, 26))
18+
19+
staticProperty = 42;
20+
>staticProperty : Symbol(SubFoo.staticProperty, Decl(override13.ts, 6, 18))
21+
}
22+
23+
class StaticSubFoo extends Foo {
24+
>StaticSubFoo : Symbol(StaticSubFoo, Decl(override13.ts, 8, 1))
25+
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))
26+
27+
static property = 42;
28+
>property : Symbol(StaticSubFoo.property, Decl(override13.ts, 10, 32))
29+
30+
static staticProperty = 42;
31+
>staticProperty : Symbol(StaticSubFoo.staticProperty, Decl(override13.ts, 11, 25))
32+
}
33+
34+
class Intermediate extends Foo {}
35+
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))
36+
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))
37+
38+
class Derived extends Intermediate {
39+
>Derived : Symbol(Derived, Decl(override13.ts, 15, 33))
40+
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))
41+
42+
property = 42;
43+
>property : Symbol(Derived.property, Decl(override13.ts, 17, 36))
44+
45+
staticProperty = 42;
46+
>staticProperty : Symbol(Derived.staticProperty, Decl(override13.ts, 18, 18))
47+
}
48+
49+
class StaticDerived extends Intermediate {
50+
>StaticDerived : Symbol(StaticDerived, Decl(override13.ts, 20, 1))
51+
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))
52+
53+
static property = 42;
54+
>property : Symbol(StaticDerived.property, Decl(override13.ts, 22, 42))
55+
56+
static staticProperty = 42;
57+
>staticProperty : Symbol(StaticDerived.staticProperty, Decl(override13.ts, 23, 25))
58+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
=== tests/cases/conformance/override/override13.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
property = 1
6+
>property : number
7+
>1 : 1
8+
9+
static staticProperty = 2
10+
>staticProperty : number
11+
>2 : 2
12+
}
13+
14+
class SubFoo extends Foo {
15+
>SubFoo : SubFoo
16+
>Foo : Foo
17+
18+
property = 42;
19+
>property : number
20+
>42 : 42
21+
22+
staticProperty = 42;
23+
>staticProperty : number
24+
>42 : 42
25+
}
26+
27+
class StaticSubFoo extends Foo {
28+
>StaticSubFoo : StaticSubFoo
29+
>Foo : Foo
30+
31+
static property = 42;
32+
>property : number
33+
>42 : 42
34+
35+
static staticProperty = 42;
36+
>staticProperty : number
37+
>42 : 42
38+
}
39+
40+
class Intermediate extends Foo {}
41+
>Intermediate : Intermediate
42+
>Foo : Foo
43+
44+
class Derived extends Intermediate {
45+
>Derived : Derived
46+
>Intermediate : Intermediate
47+
48+
property = 42;
49+
>property : number
50+
>42 : 42
51+
52+
staticProperty = 42;
53+
>staticProperty : number
54+
>42 : 42
55+
}
56+
57+
class StaticDerived extends Intermediate {
58+
>StaticDerived : StaticDerived
59+
>Intermediate : Intermediate
60+
61+
static property = 42;
62+
>property : number
63+
>42 : 42
64+
65+
static staticProperty = 42;
66+
>staticProperty : number
67+
>42 : 42
68+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//// [override14.ts]
2+
class Foo {
3+
property = 1
4+
}
5+
6+
class SubFoo extends Foo {
7+
declare property: number
8+
}
9+
10+
11+
//// [override14.js]
12+
class Foo {
13+
property = 1;
14+
}
15+
class SubFoo extends Foo {
16+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
=== tests/cases/conformance/override/override14.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(override14.ts, 0, 0))
4+
5+
property = 1
6+
>property : Symbol(Foo.property, Decl(override14.ts, 0, 11))
7+
}
8+
9+
class SubFoo extends Foo {
10+
>SubFoo : Symbol(SubFoo, Decl(override14.ts, 2, 1))
11+
>Foo : Symbol(Foo, Decl(override14.ts, 0, 0))
12+
13+
declare property: number
14+
>property : Symbol(SubFoo.property, Decl(override14.ts, 4, 26))
15+
}
16+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
=== tests/cases/conformance/override/override14.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
property = 1
6+
>property : number
7+
>1 : 1
8+
}
9+
10+
class SubFoo extends Foo {
11+
>SubFoo : SubFoo
12+
>Foo : Foo
13+
14+
declare property: number
15+
>property : number
16+
}
17+

tests/baselines/reference/override5.errors.txt

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
1-
tests/cases/conformance/override/override5.ts(12,13): error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'B'.
21
tests/cases/conformance/override/override5.ts(14,14): error TS1040: 'override' modifier cannot be used in an ambient context.
32
tests/cases/conformance/override/override5.ts(16,14): error TS1029: 'override' modifier must precede 'readonly' modifier.
4-
tests/cases/conformance/override/override5.ts(20,14): error TS1243: 'static' modifier cannot be used with 'override' modifier.
3+
tests/cases/conformance/override/override5.ts(20,21): error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'B'.
54
tests/cases/conformance/override/override5.ts(22,14): error TS1030: 'override' modifier already seen.
65
tests/cases/conformance/override/override5.ts(25,14): error TS1029: 'public' modifier must precede 'override' modifier.
76
tests/cases/conformance/override/override5.ts(27,5): error TS1089: 'override' modifier cannot appear on a constructor declaration.
87
tests/cases/conformance/override/override5.ts(44,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
98
tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member cannot have an 'override' modifier because its containing class 'AND' does not extend another class.
109

1110

12-
==== tests/cases/conformance/override/override5.ts (9 errors) ====
11+
==== tests/cases/conformance/override/override5.ts (8 errors) ====
1312
class B {
1413
p1: number = 1;
1514
p2: number = 2;
@@ -22,8 +21,6 @@ tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member
2221

2322
class D extends B{
2423
declare p1: number
25-
~~
26-
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'B'.
2724

2825
override declare p2: number;
2926
~~~~~~~
@@ -36,8 +33,8 @@ tests/cases/conformance/override/override5.ts(45,23): error TS4112: This member
3633
override readonly p4: number;
3734

3835
override static sp: number;
39-
~~~~~~
40-
!!! error TS1243: 'static' modifier cannot be used with 'override' modifier.
36+
~~
37+
!!! error TS4113: This member cannot have an 'override' modifier because it is not declared in the base class 'B'.
4138

4239
override override oop: number;
4340
~~~~~~~~

0 commit comments

Comments
 (0)