Skip to content
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

Always error on property override accessor #37894

Merged
merged 10 commits into from
May 26, 2020
Next Next commit
Always error when property overrides accessor or vice versa
Previously this was only an error when useDefineForClassFields: true,
but now it's an error for all code. This is a rare mistake to make, and
usually only occurs in code written before `readonly` was available.

Codefix to come in subsequent commits.
  • Loading branch information
sandersn committed Apr 9, 2020
commit 3ffe25316682ab1c95a1d9fef56d9dbcb5a2453c
5 changes: 2 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
@@ -33373,8 +33373,7 @@ namespace ts {
const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor;
if (basePropertyFlags && derivedPropertyFlags) {
// property/accessor is overridden with property/accessor
if (!compilerOptions.useDefineForClassFields
|| baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer)
if (baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer)
|| base.valueDeclaration && base.valueDeclaration.parent.kind === SyntaxKind.InterfaceDeclaration
|| derived.valueDeclaration && isBinaryExpression(derived.valueDeclaration)) {
// when the base property is abstract or from an interface, base/derived flags don't need to match
@@ -33390,7 +33389,7 @@ namespace ts {
Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor;
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType), typeToString(type));
}
else {
else if (compilerOptions.useDefineForClassFields) {
const uninitialized = find(derived.declarations, d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer);
if (uninitialized
&& !(derived.flags & SymbolFlags.Transient)
24 changes: 24 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts(5,9): error TS2611: 'p' is defined as a property in class 'A', but is overridden here in 'B' as an accessor.
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts(12,9): error TS2611: 'p' is defined as a property in class 'C', but is overridden here in 'D' as an accessor.


==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts (2 errors) ====
class A {
p = 'yep'
}
class B extends A {
get p() { return 'oh no' } // error
~
!!! error TS2611: 'p' is defined as a property in class 'A', but is overridden here in 'B' as an accessor.
}
class C {
p = 101
}
class D extends C {
_secret = 11
get p() { return this._secret } // error
~
!!! error TS2611: 'p' is defined as a property in class 'C', but is overridden here in 'D' as an accessor.
set p(value) { this._secret = value } // error
}

Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(6,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(6,9): error TS2611: 'x' is defined as a property in class 'a', but is overridden here in 'b' as an accessor.
tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(9,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.


==== tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts (2 errors) ====
==== tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts (3 errors) ====
class a {
x: string;
}
@@ -11,6 +12,8 @@ tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(9,9): error
get x() {
~
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
~
!!! error TS2611: 'x' is defined as a property in class 'a', but is overridden here in 'b' as an accessor.
return "20";
}
set x(aValue: string) {
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(3,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(6,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(12,5): error TS2610: 'x' is defined as an accessor in class 'a', but is overridden here in 'b' as an instance property.


==== tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts (2 errors) ====
==== tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts (3 errors) ====
class a {
private __x: () => string;
get x() {
@@ -19,4 +20,6 @@ tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(6,9): error

class b extends a {
x: () => string;
~
!!! error TS2610: 'x' is defined as an accessor in class 'a', but is overridden here in 'b' as an instance property.
}
Original file line number Diff line number Diff line change
@@ -37,7 +37,13 @@ Output::
[12:00:13 AM] Starting compilation in watch mode...


[12:00:16 AM] Found 0 errors. Watching for file changes.
a.ts:2:21 - error TS2610: 'prop' is defined as an accessor in class 'C', but is overridden here in 'D' as an instance property.

2 class D extends C { prop = 1; }
   ~~~~


[12:00:16 AM] Found 1 error. Watching for file changes.