From 3ffe25316682ab1c95a1d9fef56d9dbcb5a2453c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 9 Apr 2020 08:27:08 -0700 Subject: [PATCH] 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. --- src/compiler/checker.ts | 5 ++-- .../accessorsOverrideProperty6.errors.txt | 24 +++++++++++++++++++ ...emberAccessorOverridingProperty.errors.txt | 5 +++- ...emberPropertyOverridingAccessor.errors.txt | 5 +++- ...it-when-useDefineForClassFields-changes.js | 8 ++++++- 5 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/accessorsOverrideProperty6.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 98c059ca30..18c03b8b30 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -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) diff --git a/tests/baselines/reference/accessorsOverrideProperty6.errors.txt b/tests/baselines/reference/accessorsOverrideProperty6.errors.txt new file mode 100644 index 0000000000..ad3c13e684 --- /dev/null +++ b/tests/baselines/reference/accessorsOverrideProperty6.errors.txt @@ -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 + } + \ No newline at end of file diff --git a/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt b/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt index 98b9d349d6..0872ab47f2 100644 --- a/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt +++ b/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt @@ -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) { diff --git a/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt b/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt index ec8ec05a8a..61e7e78942 100644 --- a/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt +++ b/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt @@ -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. } \ No newline at end of file diff --git a/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js b/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js index 721df9bc34..0e2e3b2a40 100644 --- a/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js +++ b/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js @@ -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.