Skip to content

ignore static and declared member if checking override #43569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36943,7 +36943,7 @@ namespace ts {
}
}

checkMembersForMissingOverrideModifier(node, type, typeWithThis);
checkMembersForMissingOverrideModifier(node, type, typeWithThis, staticType);

const implementedTypeNodes = getEffectiveImplementsTypeNodes(node);
if (implementedTypeNodes) {
Expand Down Expand Up @@ -36980,13 +36980,18 @@ namespace ts {
}
}

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

for (const member of node.members) {
if (hasAmbientModifier(member)) {
continue;
}

if (isConstructorDeclaration(member)) {
forEach(member.parameters, param => {
if (isParameterPropertyDeclaration(param, member)) {
Expand All @@ -36996,17 +37001,22 @@ namespace ts {
}
checkClassMember(member);
}

function checkClassMember(member: ClassElement | ParameterPropertyDeclaration, memberIsParameterProperty?: boolean) {
const hasOverride = hasOverrideModifier(member);
const hasStatic = hasStaticModifier(member);
if (baseWithThis && (hasOverride || compilerOptions.noImplicitOverride)) {
const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
if (!declaredProp) {
return;
}

const thisType = hasStatic ? staticType : typeWithThis;
const baseType = hasStatic ? baseStaticType : baseWithThis;
const prop = getPropertyOfType(thisType, declaredProp.escapedName);
const baseProp = getPropertyOfType(baseType, declaredProp.escapedName);

const baseClassName = typeToString(baseWithThis);
const prop = getPropertyOfType(typeWithThis, declaredProp.escapedName);
const baseProp = getPropertyOfType(baseWithThis, declaredProp.escapedName);
if (prop && !baseProp && hasOverride) {
error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0, baseClassName);
}
Expand Down Expand Up @@ -40443,9 +40453,6 @@ namespace ts {
else if (flags & ModifierFlags.Ambient) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "override", "declare");
}
else if (flags & ModifierFlags.Static) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
}
else if (flags & ModifierFlags.Readonly) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "override", "readonly");
}
Expand Down Expand Up @@ -40500,9 +40507,6 @@ namespace ts {
else if (flags & ModifierFlags.Readonly) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "readonly");
}
else if (flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
}
else if (flags & ModifierFlags.Async) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "async");
}
Expand Down
41 changes: 41 additions & 0 deletions tests/baselines/reference/override13.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
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'.
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'.
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'.
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'.


==== tests/cases/conformance/override/override13.ts (4 errors) ====
class Foo {
property = 1
static staticProperty = 2
}

class SubFoo extends Foo {
property = 42;
~~~~~~~~
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
staticProperty = 42;
}

class StaticSubFoo extends Foo {
static property = 42;
static staticProperty = 42;
~~~~~~~~~~~~~~
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Foo'.
}

class Intermediate extends Foo {}

class Derived extends Intermediate {
property = 42;
~~~~~~~~
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
staticProperty = 42;
}

class StaticDerived extends Intermediate {
static property = 42;
static staticProperty = 42;
~~~~~~~~~~~~~~
!!! error TS4114: This member must have an 'override' modifier because it overrides a member in the base class 'Intermediate'.
}
51 changes: 51 additions & 0 deletions tests/baselines/reference/override13.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//// [override13.ts]
class Foo {
property = 1
static staticProperty = 2
}

class SubFoo extends Foo {
property = 42;
staticProperty = 42;
}

class StaticSubFoo extends Foo {
static property = 42;
static staticProperty = 42;
}

class Intermediate extends Foo {}

class Derived extends Intermediate {
property = 42;
staticProperty = 42;
}

class StaticDerived extends Intermediate {
static property = 42;
static staticProperty = 42;
}

//// [override13.js]
class Foo {
property = 1;
static staticProperty = 2;
}
class SubFoo extends Foo {
property = 42;
staticProperty = 42;
}
class StaticSubFoo extends Foo {
static property = 42;
static staticProperty = 42;
}
class Intermediate extends Foo {
}
class Derived extends Intermediate {
property = 42;
staticProperty = 42;
}
class StaticDerived extends Intermediate {
static property = 42;
static staticProperty = 42;
}
58 changes: 58 additions & 0 deletions tests/baselines/reference/override13.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
=== tests/cases/conformance/override/override13.ts ===
class Foo {
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))

property = 1
>property : Symbol(Foo.property, Decl(override13.ts, 0, 11))

static staticProperty = 2
>staticProperty : Symbol(Foo.staticProperty, Decl(override13.ts, 1, 16))
}

class SubFoo extends Foo {
>SubFoo : Symbol(SubFoo, Decl(override13.ts, 3, 1))
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))

property = 42;
>property : Symbol(SubFoo.property, Decl(override13.ts, 5, 26))

staticProperty = 42;
>staticProperty : Symbol(SubFoo.staticProperty, Decl(override13.ts, 6, 18))
}

class StaticSubFoo extends Foo {
>StaticSubFoo : Symbol(StaticSubFoo, Decl(override13.ts, 8, 1))
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))

static property = 42;
>property : Symbol(StaticSubFoo.property, Decl(override13.ts, 10, 32))

static staticProperty = 42;
>staticProperty : Symbol(StaticSubFoo.staticProperty, Decl(override13.ts, 11, 25))
}

class Intermediate extends Foo {}
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))
>Foo : Symbol(Foo, Decl(override13.ts, 0, 0))

class Derived extends Intermediate {
>Derived : Symbol(Derived, Decl(override13.ts, 15, 33))
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))

property = 42;
>property : Symbol(Derived.property, Decl(override13.ts, 17, 36))

staticProperty = 42;
>staticProperty : Symbol(Derived.staticProperty, Decl(override13.ts, 18, 18))
}

class StaticDerived extends Intermediate {
>StaticDerived : Symbol(StaticDerived, Decl(override13.ts, 20, 1))
>Intermediate : Symbol(Intermediate, Decl(override13.ts, 13, 1))

static property = 42;
>property : Symbol(StaticDerived.property, Decl(override13.ts, 22, 42))

static staticProperty = 42;
>staticProperty : Symbol(StaticDerived.staticProperty, Decl(override13.ts, 23, 25))
}
68 changes: 68 additions & 0 deletions tests/baselines/reference/override13.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
=== tests/cases/conformance/override/override13.ts ===
class Foo {
>Foo : Foo

property = 1
>property : number
>1 : 1

static staticProperty = 2
>staticProperty : number
>2 : 2
}

class SubFoo extends Foo {
>SubFoo : SubFoo
>Foo : Foo

property = 42;
>property : number
>42 : 42

staticProperty = 42;
>staticProperty : number
>42 : 42
}

class StaticSubFoo extends Foo {
>StaticSubFoo : StaticSubFoo
>Foo : Foo

static property = 42;
>property : number
>42 : 42

static staticProperty = 42;
>staticProperty : number
>42 : 42
}

class Intermediate extends Foo {}
>Intermediate : Intermediate
>Foo : Foo

class Derived extends Intermediate {
>Derived : Derived
>Intermediate : Intermediate

property = 42;
>property : number
>42 : 42

staticProperty = 42;
>staticProperty : number
>42 : 42
}

class StaticDerived extends Intermediate {
>StaticDerived : StaticDerived
>Intermediate : Intermediate

static property = 42;
>property : number
>42 : 42

static staticProperty = 42;
>staticProperty : number
>42 : 42
}
16 changes: 16 additions & 0 deletions tests/baselines/reference/override14.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//// [override14.ts]
class Foo {
property = 1
}

class SubFoo extends Foo {
declare property: number
}


//// [override14.js]
class Foo {
property = 1;
}
class SubFoo extends Foo {
}
16 changes: 16 additions & 0 deletions tests/baselines/reference/override14.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
=== tests/cases/conformance/override/override14.ts ===
class Foo {
>Foo : Symbol(Foo, Decl(override14.ts, 0, 0))

property = 1
>property : Symbol(Foo.property, Decl(override14.ts, 0, 11))
}

class SubFoo extends Foo {
>SubFoo : Symbol(SubFoo, Decl(override14.ts, 2, 1))
>Foo : Symbol(Foo, Decl(override14.ts, 0, 0))

declare property: number
>property : Symbol(SubFoo.property, Decl(override14.ts, 4, 26))
}

17 changes: 17 additions & 0 deletions tests/baselines/reference/override14.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
=== tests/cases/conformance/override/override14.ts ===
class Foo {
>Foo : Foo

property = 1
>property : number
>1 : 1
}

class SubFoo extends Foo {
>SubFoo : SubFoo
>Foo : Foo

declare property: number
>property : number
}

11 changes: 4 additions & 7 deletions tests/baselines/reference/override5.errors.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
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'.
tests/cases/conformance/override/override5.ts(14,14): error TS1040: 'override' modifier cannot be used in an ambient context.
tests/cases/conformance/override/override5.ts(16,14): error TS1029: 'override' modifier must precede 'readonly' modifier.
tests/cases/conformance/override/override5.ts(20,14): error TS1243: 'static' modifier cannot be used with 'override' modifier.
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'.
tests/cases/conformance/override/override5.ts(22,14): error TS1030: 'override' modifier already seen.
tests/cases/conformance/override/override5.ts(25,14): error TS1029: 'public' modifier must precede 'override' modifier.
tests/cases/conformance/override/override5.ts(27,5): error TS1089: 'override' modifier cannot appear on a constructor declaration.
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.
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.


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

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

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

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

override override oop: number;
~~~~~~~~
Expand Down
Loading