From b8aa2f81a21f3706137b3ac1f2930aaaf540a929 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 4 Sep 2017 14:24:07 +0300 Subject: [PATCH] Fix delegate property name clash in JS BE See KT-19542 --- .../js/test/semantics/BoxJsTestGenerated.java | 6 +++ .../kotlin/js/translate/context/Namer.java | 10 ----- .../js/translate/context/StaticContext.java | 4 +- .../translate/context/TranslationContext.java | 2 +- .../translate/declaration/ClassTranslator.kt | 2 +- .../declaration/DeclarationBodyVisitor.kt | 5 +-- .../declaration/PropertyTranslator.kt | 5 +-- .../initializer/InitializerUtils.java | 6 +-- .../initializer/InitializerVisitor.java | 2 +- .../kotlin/js/translate/utils/JsAstUtils.java | 2 +- .../clashingNameInSubclass.kt | 44 +++++++++++++++++++ 11 files changed, 63 insertions(+), 25 deletions(-) create mode 100644 js/js.translator/testData/box/delegateProperty/clashingNameInSubclass.kt diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index 0d70962a716..7668a307a75 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java @@ -1106,6 +1106,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("clashingNameInSubclass.kt") + public void testClashingNameInSubclass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/clashingNameInSubclass.kt"); + doTest(fileName); + } + @TestMetadata("delegateByExtensionProperty.kt") public void testDelegateByExtensionProperty() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/delegateByExtensionProperty.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java index 8c935830ff5..455b7efc7de 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java @@ -159,16 +159,6 @@ public final class Namer { return DELEGATE; } - @NotNull - public static String getDelegateName(@NotNull String propertyName) { - return propertyName + DELEGATE; - } - - @NotNull - public static JsNameRef getDelegateNameRef(String propertyName) { - return new JsNameRef(getDelegateName(propertyName), new JsThisRef()); - } - @NotNull public static JsNameRef getFunctionCallRef(@NotNull JsExpression functionExpression) { return pureFqn(CALL_FUNCTION, functionExpression); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java index 00e12d73892..6d480e0f6c8 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java @@ -116,7 +116,7 @@ public final class StaticContext { private final Map nameCache = new HashMap<>(); @NotNull - private final Map backingFieldNameCache = new HashMap<>(); + private final Map backingFieldNameCache = new HashMap<>(); @NotNull private final Map fqnCache = new HashMap<>(); @@ -353,7 +353,7 @@ public final class StaticContext { } @NotNull - public JsName getNameForBackingField(@NotNull PropertyDescriptor property) { + public JsName getNameForBackingField(@NotNull VariableDescriptorWithAccessors property) { JsName name = backingFieldNameCache.get(property); if (name == null) { diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index 4a244f0f82f..0c9fc85298e 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -407,7 +407,7 @@ public class TranslationContext { } @NotNull - public JsName getNameForBackingField(@NotNull PropertyDescriptor property) { + public JsName getNameForBackingField(@NotNull VariableDescriptorWithAccessors property) { return staticContext.getNameForBackingField(property); } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt index 986d12c05bd..e84dfaaea63 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt @@ -412,7 +412,7 @@ class ClassTranslator private constructor( function.parameters.add(i, JsParameter(name)) if (fieldName != null && constructor == primaryConstructor) { val source = (constructor.descriptor as? DeclarationDescriptorWithSource)?.source - additionalStatements += JsAstUtils.defineSimpleProperty(fieldName.ident, name.makeRef(), source) + additionalStatements += JsAstUtils.defineSimpleProperty(fieldName, name.makeRef(), source) } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.kt index b8bf07a00ce..abc52b3fb3c 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.kt @@ -170,7 +170,6 @@ class DeclarationBodyVisitor( } } - override fun getBackingFieldReference(descriptor: PropertyDescriptor): JsExpression { - return Namer.getDelegateNameRef(descriptor.name.asString()) - } + override fun getBackingFieldReference(descriptor: PropertyDescriptor): JsExpression = + JsNameRef(context.getNameForBackingField(descriptor), JsThisRef()) } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt index 546d2c5de7b..50d0c8815ea 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt @@ -22,7 +22,6 @@ import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.js.backend.ast.* import org.jetbrains.kotlin.js.translate.callTranslator.CallTranslator import org.jetbrains.kotlin.js.translate.context.Namer -import org.jetbrains.kotlin.js.translate.context.Namer.getDelegateNameRef import org.jetbrains.kotlin.js.translate.context.Namer.getReceiverParameterName import org.jetbrains.kotlin.js.translate.context.TranslationContext import org.jetbrains.kotlin.js.translate.expression.translateAndAliasParameters @@ -225,7 +224,7 @@ private class PropertyTranslator( private fun generateDefaultGetter(): JsPropertyInitializer { val getterDescriptor = descriptor.getter ?: throw IllegalStateException("Getter descriptor should not be null") val defaultFunction = createFunction(getterDescriptor).apply { - val delegateRef = getDelegateNameRef(descriptor.name.asString()) + val delegateRef = JsNameRef(context().getNameForBackingField(descriptor), JsThisRef()) DefaultPropertyTranslator(descriptor, context(), delegateRef).generateDefaultGetterFunction(getterDescriptor, this) } return generateDefaultAccessor(getterDescriptor, defaultFunction) @@ -234,7 +233,7 @@ private class PropertyTranslator( private fun generateDefaultSetter(): JsPropertyInitializer { val setterDescriptor = descriptor.setter ?: throw IllegalStateException("Setter descriptor should not be null") val defaultFunction = createFunction(setterDescriptor).apply { - val delegateRef = getDelegateNameRef(descriptor.name.asString()) + val delegateRef = JsNameRef(context().getNameForBackingField(descriptor), JsThisRef()) DefaultPropertyTranslator(descriptor, context(), delegateRef).generateDefaultSetterFunction(setterDescriptor, this) } return generateDefaultAccessor(setterDescriptor, defaultFunction) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerUtils.java index 98321034bf7..543ca6f9e13 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerUtils.java @@ -43,12 +43,12 @@ public final class InitializerUtils { return assignment.makeStmt(); } - @Nullable + @NotNull public static JsStatement generateInitializerForDelegate( + @NotNull TranslationContext context, @NotNull PropertyDescriptor descriptor, @NotNull JsExpression value ) { - String name = descriptor.getName().asString(); - return JsAstUtils.defineSimpleProperty(Namer.getDelegateName(name), value, descriptor.getSource()); + return JsAstUtils.defineSimpleProperty(context.getNameForBackingField(descriptor), value, descriptor.getSource()); } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerVisitor.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerVisitor.java index aa6eaccc8b8..898fd42a937 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerVisitor.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/initializer/InitializerVisitor.java @@ -55,7 +55,7 @@ public final class InitializerVisitor extends TranslatorVisitor { } else if (delegate != null) { assert value != null; - statement = generateInitializerForDelegate(descriptor, value); + statement = generateInitializerForDelegate(context, descriptor, value); } else if (Boolean.TRUE.equals(context.bindingContext().get(BindingContext.BACKING_FIELD_REQUIRED, descriptor))) { JsNameRef backingFieldReference = TranslationUtils.backingFieldReference(context, descriptor); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java index 3fe60f3ed13..7a0eac82c8b 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsAstUtils.java @@ -444,7 +444,7 @@ public final class JsAstUtils { } @NotNull - public static JsStatement defineSimpleProperty(@NotNull String name, @NotNull JsExpression value, @Nullable SourceElement source) { + public static JsStatement defineSimpleProperty(@NotNull JsName name, @NotNull JsExpression value, @Nullable SourceElement source) { JsExpression assignment = assignment(new JsNameRef(name, new JsThisRef()), value); if (source != null) { assignment.setSource(KotlinSourceElementKt.getPsi(source)); diff --git a/js/js.translator/testData/box/delegateProperty/clashingNameInSubclass.kt b/js/js.translator/testData/box/delegateProperty/clashingNameInSubclass.kt new file mode 100644 index 00000000000..2512c20b0f2 --- /dev/null +++ b/js/js.translator/testData/box/delegateProperty/clashingNameInSubclass.kt @@ -0,0 +1,44 @@ +// EXPECTED_REACHABLE_NODES: 1043 +fun box(): String { + val b = B() + + var r = b.getFooA() + if (r != "A.foo") return "fail1: $r" + + r = b.getFooB() + if (r != "B.foo") return "fail2: $r" + + r = b.getBarA() + if (r != "A.bar") return "fail3: $r" + + r = b.getBarB() + if (r != "B.bar") return "fail4: $r" + + return "OK" +} + +open class A { + open val foo by lazy { + "A.foo" + } + + private val bar by lazy { + "A.bar" + } + + fun getBarA() = bar +} + +class B : A() { + override val foo by lazy { + "B.foo" + } + + private val bar by lazy { + "B.bar" + } + + fun getFooB() = foo + fun getFooA() = super.foo + fun getBarB() = bar +} \ No newline at end of file