From c4a504b3ce1fe24940760f3ec561bacce6d8691b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 4 Apr 2018 11:03:31 -0700 Subject: [PATCH] Prototype assignments count as method-like (#23137) * Prototype assignments count as method-like For the purposes of reporting prototype/instance property conflicts * Fix lint --- src/compiler/checker.ts | 27 +++++++++---- .../typeFromPropertyAssignment23.symbols | 35 ++++++++++++++++ .../typeFromPropertyAssignment23.types | 40 +++++++++++++++++++ .../salsa/typeFromPropertyAssignment23.ts | 15 +++++++ 4 files changed, 109 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 88de413b86..1315c12cdc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6503,7 +6503,7 @@ namespace ts { (modifiers & ModifierFlags.Protected ? CheckFlags.ContainsProtected : 0) | (modifiers & ModifierFlags.Private ? CheckFlags.ContainsPrivate : 0) | (modifiers & ModifierFlags.Static ? CheckFlags.ContainsStatic : 0); - if (!isMethodLike(prop)) { + if (!isPrototypeProperty(prop)) { syntheticFlag = CheckFlags.SyntheticProperty; } } @@ -16236,8 +16236,19 @@ namespace ts { return s.valueDeclaration ? getCombinedNodeFlags(s.valueDeclaration) : 0; } - function isMethodLike(symbol: Symbol) { - return !!(symbol.flags & SymbolFlags.Method || getCheckFlags(symbol) & CheckFlags.SyntheticMethod); + /** + * Return whether this symbol is a member of a prototype somewhere + * Note that this is not tracked well within the compiler, so the answer may be incorrect. + */ + function isPrototypeProperty(symbol: Symbol) { + if (symbol.flags & SymbolFlags.Method || getCheckFlags(symbol) & CheckFlags.SyntheticMethod) { + return true; + } + if (isInJavaScriptFile(symbol.valueDeclaration)) { + const parent = symbol.valueDeclaration.parent; + return parent && isBinaryExpression(parent) && + getSpecialPropertyAssignmentKind(parent) === SpecialPropertyAssignmentKind.PrototypeProperty; + } } /** @@ -23677,13 +23688,13 @@ namespace ts { continue; } - if (isMethodLike(base) && isMethodLike(derived) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) { + if (isPrototypeProperty(base) && isPrototypeProperty(derived) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) { // method is overridden with method or property/accessor is overridden with property/accessor - correct case continue; } let errorMessage: DiagnosticMessage; - if (isMethodLike(base)) { + if (isPrototypeProperty(base)) { if (derived.flags & SymbolFlags.Accessor) { errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor; } @@ -23691,11 +23702,11 @@ namespace ts { errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_property; } } - else if (base.flags & SymbolFlags.Property) { - errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function; + else if (base.flags & SymbolFlags.Accessor) { + errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function; } else { - errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_function; + errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_function; } error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, typeToString(baseType), symbolToString(base), typeToString(type)); diff --git a/tests/baselines/reference/typeFromPropertyAssignment23.symbols b/tests/baselines/reference/typeFromPropertyAssignment23.symbols index 936a9665cd..2ba8651ea5 100644 --- a/tests/baselines/reference/typeFromPropertyAssignment23.symbols +++ b/tests/baselines/reference/typeFromPropertyAssignment23.symbols @@ -40,3 +40,38 @@ D.prototype.foo = () => { this.n = 'not checked, so no error' } +// post-class prototype assignments are trying to show that these properties are abstract +class Module { +>Module : Symbol(Module, Decl(a.js, 17, 1)) +} +Module.prototype.identifier = undefined +>Module.prototype.identifier : Symbol(Module.identifier, Decl(a.js, 21, 1)) +>Module.prototype : Symbol(Module.identifier, Decl(a.js, 21, 1)) +>Module : Symbol(Module, Decl(a.js, 17, 1)) +>prototype : Symbol(Module.prototype) +>identifier : Symbol(Module.identifier, Decl(a.js, 21, 1)) +>undefined : Symbol(undefined) + +Module.prototype.size = null +>Module.prototype.size : Symbol(Module.size, Decl(a.js, 22, 39)) +>Module.prototype : Symbol(Module.size, Decl(a.js, 22, 39)) +>Module : Symbol(Module, Decl(a.js, 17, 1)) +>prototype : Symbol(Module.prototype) +>size : Symbol(Module.size, Decl(a.js, 22, 39)) + +class NormalModule extends Module { +>NormalModule : Symbol(NormalModule, Decl(a.js, 23, 28)) +>Module : Symbol(Module, Decl(a.js, 17, 1)) + + identifier() { +>identifier : Symbol(NormalModule.identifier, Decl(a.js, 25, 35)) + + return 'normal' + } + size() { +>size : Symbol(NormalModule.size, Decl(a.js, 28, 5)) + + return 0 + } +} + diff --git a/tests/baselines/reference/typeFromPropertyAssignment23.types b/tests/baselines/reference/typeFromPropertyAssignment23.types index 5dc5c3fc97..4e540e3962 100644 --- a/tests/baselines/reference/typeFromPropertyAssignment23.types +++ b/tests/baselines/reference/typeFromPropertyAssignment23.types @@ -51,3 +51,43 @@ D.prototype.foo = () => { >'not checked, so no error' : "not checked, so no error" } +// post-class prototype assignments are trying to show that these properties are abstract +class Module { +>Module : Module +} +Module.prototype.identifier = undefined +>Module.prototype.identifier = undefined : undefined +>Module.prototype.identifier : any +>Module.prototype : Module +>Module : typeof Module +>prototype : Module +>identifier : any +>undefined : undefined + +Module.prototype.size = null +>Module.prototype.size = null : null +>Module.prototype.size : any +>Module.prototype : Module +>Module : typeof Module +>prototype : Module +>size : any +>null : null + +class NormalModule extends Module { +>NormalModule : NormalModule +>Module : Module + + identifier() { +>identifier : () => string + + return 'normal' +>'normal' : "normal" + } + size() { +>size : () => number + + return 0 +>0 : 0 + } +} + diff --git a/tests/cases/conformance/salsa/typeFromPropertyAssignment23.ts b/tests/cases/conformance/salsa/typeFromPropertyAssignment23.ts index a548057094..18923e0a5a 100644 --- a/tests/cases/conformance/salsa/typeFromPropertyAssignment23.ts +++ b/tests/cases/conformance/salsa/typeFromPropertyAssignment23.ts @@ -20,3 +20,18 @@ class D extends B { } D.prototype.foo = () => { this.n = 'not checked, so no error' } + +// post-class prototype assignments are trying to show that these properties are abstract +class Module { +} +Module.prototype.identifier = undefined +Module.prototype.size = null + +class NormalModule extends Module { + identifier() { + return 'normal' + } + size() { + return 0 + } +}