From f6ba2a958ae52c53ad4792a7d4e2dcbf1c01a514 Mon Sep 17 00:00:00 2001 From: Ilya Goncharov Date: Thu, 21 Oct 2021 10:52:48 +0000 Subject: [PATCH] [JS IR] No generate exportness for overridden properties [JS IR] Add check on existence of overridden property in tests ^KT-49326 fixed [JS IR] Add details to comment with exporting properties ^KT-49326 fixed [JS IR] No generate exportness for overridden properties, if only no override val -> var ^KT-49326 fixed Merge-request: KT-MR-4810 --- .../transformers/irToJs/JsClassGenerator.kt | 13 +- .../box/propertyOverride/exportedBaseClass.kt | 125 ++++++++++++++++-- 2 files changed, 125 insertions(+), 13 deletions(-) diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt index aa4a3591e20..63027c054e4 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt @@ -133,8 +133,19 @@ class JsClassGenerator(private val irClass: IrClass, val context: JsGenerationCo // Don't generate `defineProperty` if the property overrides a property from an exported class, // because we've already generated `defineProperty` for the base class property. // In other words, we only want to generate `defineProperty` once for each property. + // The exception is case when we override val with var, + // so we need regenerate `defineProperty` with setter. + val noOverriddenGetter = property.getter?.overriddenSymbols?.isEmpty() == true + + val overriddenExportedGetter = property.getter?.overriddenSymbols?.isNotEmpty() == true && + property.getter?.isOverriddenExported(context.staticContext.backendContext) == true + + val noOverriddenExportedSetter = property.setter?.isOverriddenExported(context.staticContext.backendContext) == false + + val needOverrideBySetter = overriddenExportedGetter && noOverriddenExportedSetter + if (irClass.isExported(context.staticContext.backendContext) && - property.getter?.isOverriddenExported(context.staticContext.backendContext) == false || + (noOverriddenGetter || needOverrideBySetter) || property.getter?.overridesExternal() == true || property.getJsName() != null ) { diff --git a/js/js.translator/testData/box/propertyOverride/exportedBaseClass.kt b/js/js.translator/testData/box/propertyOverride/exportedBaseClass.kt index a8d400ec358..18bae014f9a 100644 --- a/js/js.translator/testData/box/propertyOverride/exportedBaseClass.kt +++ b/js/js.translator/testData/box/propertyOverride/exportedBaseClass.kt @@ -10,13 +10,14 @@ abstract class ExportedBase { abstract val foo: String abstract var bar: String + abstract val baz: String val fooFinal: String get() = foo } @JsExport -class ExportedDerived1 : ExportedBase() { +open class ExportedDerived1 : ExportedBase() { override val foo: String get() = "ExportedDerived1.foo" @@ -25,15 +26,16 @@ class ExportedDerived1 : ExportedBase() { override var bar: String get() = _bar set(value) { _bar = value } -} -abstract class NonExportedBase { - abstract val foo: String - abstract var bar: String + private var _baz = "ExportedDerived1.baz" + + override var baz: String + get() = _baz + set(value) { _baz = value } } @JsExport -class ExportedDerived2 : NonExportedBase() { +class ExportedDerived2 : ExportedDerived1() { override val foo: String get() = "ExportedDerived2.foo" @@ -42,6 +44,36 @@ class ExportedDerived2 : NonExportedBase() { override var bar: String get() = _bar set(value) { _bar = value } + + private var _baz = "ExportedDerived2.baz" + + override var baz: String + get() = _baz + set(value) { _baz = value } +} + +abstract class NonExportedBase { + abstract val foo: String + abstract var bar: String + abstract val baz: String +} + +@JsExport +class ExportedDerived3 : NonExportedBase() { + override val foo: String + get() = "ExportedDerived3.foo" + + private var _bar = "ExportedDerived3.bar" + + override var bar: String + get() = _bar + set(value) { _bar = value } + + private var _baz = "ExportedDerived3.baz" + + override var baz: String + get() = _baz + set(value) { _baz = value } } // Non-exported @@ -54,6 +86,12 @@ open class Derived1 : ExportedBase() { override var bar: String get() = _bar set(value) { _bar = value } + + private var _baz = "11" + + override var baz: String + get() = _baz + set(value) { _baz = value } } class Derived2 : Derived1() { @@ -65,6 +103,12 @@ class Derived2 : Derived1() { override var bar: String get() = _bar set(value) { _bar = value } + + private var _baz = "22" + + override var baz: String + get() = _baz + set(value) { _baz = value } } @JsExport @@ -98,6 +142,11 @@ function box() { d.ExportedBase.prototype.hasOwnProperty('bar'), 'Property bar should be defined for ExportedBase.prototype' ); + assertEquals( + true, + d.ExportedBase.prototype.hasOwnProperty('baz'), + 'Property baz should be defined for ExportedBase.prototype' + ); } assertEquals( @@ -112,6 +161,7 @@ function box() { assertEquals('1', derived1.bar, "derived1.bar initial value"); derived1.bar = '11'; assertEquals('11', derived1.bar, "derived1.bar after write"); + assertEquals('11', derived1.baz, "derived1.baz initial value"); if (!d.isLegacyBackend()) { assertEquals( false, @@ -123,6 +173,11 @@ function box() { Object.getPrototypeOf(derived1).hasOwnProperty('bar'), 'Property bar should of Derived1 be inherited from ExportedBase.prototype' ); + assertEquals( + false, + Object.getPrototypeOf(derived1).hasOwnProperty('baz'), + 'Property baz should of Derived1 be inherited from ExportedBase.prototype' + ); } var derived2 = d.getDerived2(); @@ -131,6 +186,7 @@ function box() { assertEquals('2', derived2.bar, "derived2.bar initial value"); derived2.bar = '22'; assertEquals('22', derived2.bar, "derived2.bar after write"); + assertEquals('22', derived2.baz, "derived2.baz initial value"); if (!d.isLegacyBackend()) { assertEquals( false, @@ -142,6 +198,11 @@ function box() { Object.getPrototypeOf(derived2).hasOwnProperty('bar'), 'Property bar of Derived2 should be inherited from ExportedBase.prototype' ); + assertEquals( + false, + Object.getPrototypeOf(derived2).hasOwnProperty('baz'), + 'Property baz of Derived2 should be inherited from ExportedBase.prototype' + ); } var exportedDerived1 = new d.ExportedDerived1(); @@ -149,6 +210,9 @@ function box() { assertEquals('ExportedDerived1.bar', exportedDerived1.bar, "exportedDerived1.bar initial value"); exportedDerived1.bar = 'ExportedDerived1.bar (updated)'; assertEquals('ExportedDerived1.bar (updated)', exportedDerived1.bar, "exportedDerived1.bar after write"); + assertEquals('ExportedDerived1.baz', exportedDerived1.baz, "exportedDerived1.baz initial value"); + exportedDerived1.baz = 'ExportedDerived1.baz (updated)'; + assertEquals('ExportedDerived1.baz (updated)', exportedDerived1.baz, "exportedDerived1.baz after write"); if (!d.isLegacyBackend()) { assertEquals( false, @@ -160,15 +224,52 @@ function box() { d.ExportedDerived1.prototype.hasOwnProperty('bar'), 'Property bar of ExportedDerived1 should be inherited from ExportedBase.prototype' ); + + // true because it is val -> var override, to we need to redefine property with setter + assertEquals( + true, + d.ExportedDerived1.prototype.hasOwnProperty('baz'), + 'Property baz of ExportedDerived1 should be defined for ExportedDerived1.prototype' + ); } var exportedDerived2 = new d.ExportedDerived2(); - assertEquals('ExportedDerived2.foo', exportedDerived2.foo, "exportedDerived2.foo"); - assertEquals('ExportedDerived2.bar', exportedDerived2.bar, "exportedDerived2.bar initial value"); - exportedDerived2.bar = 'ExportedDerived2.bar (updated)'; - assertEquals('ExportedDerived2.bar (updated)', exportedDerived2.bar, "exportedDerived2.bar after write"); - assertEquals(true, d.ExportedDerived2.prototype.hasOwnProperty('foo'), 'Property foo should be defined for ExportedDerived2.prototype'); - assertEquals(true, d.ExportedDerived2.prototype.hasOwnProperty('bar'), 'Property bar should be defined for ExportedDerived2.prototype'); + if (!d.isLegacyBackend()) { + assertEquals( + false, + d.ExportedDerived2.prototype.hasOwnProperty('foo'), + 'Property foo should not be defined for ExportedDerived2.prototype' + ); + assertEquals( + false, + d.ExportedDerived2.prototype.hasOwnProperty('bar'), + 'Property bar should not be defined for ExportedDerived2.prototype' + ); + assertEquals( + false, + d.ExportedDerived2.prototype.hasOwnProperty('baz'), + 'Property baz should not be defined for ExportedDerived2.prototype' + ); + } + + var exportedDerived3 = new d.ExportedDerived3 (); + if (!d.isLegacyBackend()) { + assertEquals( + false, + d.ExportedDerived3.prototype.hasOwnProperty('foo'), + 'Property foo should not be defined for ExportedDerived3.prototype' + ); + assertEquals( + false, + d.ExportedDerived3.prototype.hasOwnProperty('bar'), + 'Property bar should not be defined for ExportedDerived3.prototype' + ); + assertEquals( + false, + d.ExportedDerived3.prototype.hasOwnProperty('baz'), + 'Property baz should not be defined for ExportedDerived3.prototype' + ); + } return 'OK'; } \ No newline at end of file