diff --git a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt index 8803fe7472d..bfa5aa3de17 100644 --- a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt +++ b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt @@ -1120,10 +1120,6 @@ fun main(args: Array) { model("codegen/box/secondaryConstructors", targetBackend = TargetBackend.JS) } - testClass() { - model("codegen/box/classes/inner", targetBackend = TargetBackend.JS) - } - testClass() { model("codegen/box/classes/", targetBackend = TargetBackend.JS) } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ClosureTest.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ClosureTest.java index c57de186e69..1a9687502a1 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ClosureTest.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ClosureTest.java @@ -172,15 +172,13 @@ public final class ClosureTest extends SingleFileTranslationTest { checkFooBoxIsOk(); } - // TODO: tests UsageTracker but encounters issue somewhere else. Uncomment when issue with `this` generation gets fixed - // Presumably, KT-11996 - /*public void testDeepInnerClassInLocalClass() throws Exception { + public void testDeepInnerClassInLocalClass() throws Exception { checkFooBoxIsOk(); } public void testDeepInnerClassInLocalClassFromExtension() throws Exception { checkFooBoxIsOk(); - }*/ + } public void testImplicitGenericReceiverInExtensionInLocalClass() throws Exception { checkFooBoxIsOk(); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTest.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTest.java index c1f5c94bf3c..dc64e100719 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTest.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTest.java @@ -86,6 +86,14 @@ public class NestedTypesTest extends SingleFileTranslationTest { checkFooBoxIsOk(); } + public void testPrivateFieldNotOverridenInNestedSubclass() throws Exception { + checkFooBoxIsOk(); + } + + public void testOuterClassReferenceFromSecondaryConstructor() throws Exception { + checkFooBoxIsOk(); + } + @NotNull @Override protected List additionalJsFiles(@NotNull EcmaVersion ecmaVersion) { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTestGenerated.java deleted file mode 100644 index d1d22ed91f4..00000000000 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/NestedTypesTestGenerated.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright 2010-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.js.test.semantics; - -import com.intellij.testFramework.TestDataPath; -import org.jetbrains.kotlin.test.JUnit3RunnerWithInners; -import org.jetbrains.kotlin.test.KotlinTestUtils; -import org.jetbrains.kotlin.test.TestMetadata; -import org.junit.runner.RunWith; - -import java.io.File; -import java.util.regex.Pattern; - -/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */ -@SuppressWarnings("all") -@TestMetadata("compiler/testData/codegen/box/classes/inner") -@TestDataPath("$PROJECT_ROOT") -@RunWith(JUnit3RunnerWithInners.class) -public class NestedTypesTestGenerated extends AbstractNestedTypesTest { - public void testAllFilesPresentInInner() throws Exception { - KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/classes/inner"), Pattern.compile("^(.+)\\.kt$"), true); - } - - @TestMetadata("instantiateInDerived.kt") - public void testInstantiateInDerived() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/instantiateInDerived.kt"); - doTest(fileName); - } - - @TestMetadata("instantiateInDerivedLabeled.kt") - public void testInstantiateInDerivedLabeled() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/instantiateInDerivedLabeled.kt"); - doTest(fileName); - } - - @TestMetadata("instantiateInSameClass.kt") - public void testInstantiateInSameClass() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/instantiateInSameClass.kt"); - doTest(fileName); - } - - @TestMetadata("kt6708.kt") - public void testKt6708() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/kt6708.kt"); - doTest(fileName); - } - - @TestMetadata("properOuter.kt") - public void testProperOuter() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/properOuter.kt"); - doTest(fileName); - } - - @TestMetadata("properSuperLinking.kt") - public void testProperSuperLinking() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/inner/properSuperLinking.kt"); - doTest(fileName); - } -} diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/abstractClassesForGeneratedTests.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/abstractClassesForGeneratedTests.kt index b68b1296fda..092ba3ab6d4 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/abstractClassesForGeneratedTests.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/abstractClassesForGeneratedTests.kt @@ -50,8 +50,6 @@ abstract class AbstractReservedWordTest : SingleFileTranslationTest("reservedWor abstract class AbstractSecondaryConstructorTest : AbstractBlackBoxTest("secondaryConstructors/") -abstract class AbstractNestedTypesTest : AbstractBlackBoxTest("classes/inner/") - abstract class AbstractInnerNestedTest : AbstractBlackBoxTest("innerNested/") abstract class AbstractClassesTest : AbstractBlackBoxTest("classes/") diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfo.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfo.kt index f08dbd91f13..a22ba45d10f 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfo.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfo.kt @@ -21,6 +21,7 @@ import com.google.dart.compiler.backend.js.ast.JsConditional import com.google.dart.compiler.backend.js.ast.JsExpression import com.google.dart.compiler.backend.js.ast.JsLiteral import org.jetbrains.kotlin.descriptors.CallableDescriptor +import org.jetbrains.kotlin.descriptors.ConstructorDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.diagnostics.DiagnosticUtils import org.jetbrains.kotlin.js.translate.context.TranslationContext @@ -31,6 +32,7 @@ import org.jetbrains.kotlin.js.translate.utils.TranslationUtils import org.jetbrains.kotlin.resolve.calls.callUtil.isSafeCall import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.kotlin.resolve.calls.tasks.ExplicitReceiverKind.* +import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitClassReceiver import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue interface CallInfo { @@ -99,18 +101,27 @@ fun TranslationContext.getCallInfo(resolvedCall: ResolvedCall, explicitReceivers: ExplicitReceivers): CallInfo { val receiverKind = resolvedCall.explicitReceiverKind + // I'm not sure if it's a proper code, and why it should work. Just copied similar logic from ExpressionCodegen.generateConstructorCall. + // See box/classes/inner/instantiateInDerived.kt + // TODO: revisit this code later, write more tests (or borrow them from JVM backend) + fun isConstructorCall(): Boolean { + val descriptor = resolvedCall.resultingDescriptor.original + return descriptor is ConstructorDescriptor && descriptor.containingDeclaration.isInner && + resolvedCall.dispatchReceiver is ImplicitClassReceiver + } + fun getDispatchReceiver(): JsExpression? { val receiverValue = resolvedCall.dispatchReceiver ?: return null return when (receiverKind) { DISPATCH_RECEIVER, BOTH_RECEIVERS -> explicitReceivers.extensionOrDispatchReceiver - else -> this.getDispatchReceiver(receiverValue) + else -> this.getDispatchReceiver(receiverValue, isConstructorCall()) } } @@ -119,7 +130,7 @@ private fun TranslationContext.createCallInfo(resolvedCall: ResolvedCall explicitReceivers.extensionOrDispatchReceiver BOTH_RECEIVERS -> explicitReceivers.extensionReceiver - else -> this.getDispatchReceiver(receiverValue) + else -> this.getDispatchReceiver(receiverValue, isConstructorCall()) } } 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 eae779bc1a1..300450981fb 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 @@ -339,7 +339,17 @@ public class TranslationContext { } @NotNull - public JsExpression getDispatchReceiver(@NotNull ReceiverParameterDescriptor descriptor) { + public JsExpression getDispatchReceiver(@NotNull ReceiverParameterDescriptor descriptor, boolean allowSuperCall) { + // I don't see any reason for descriptor being treated inconsistently for different cases of call. + // descriptor should always point on the exact class. I.e., in the code + // + // class A { inner class B : A { foo() } } + // + // implicit receiver for `foo` must always point to either `A` or `B` depending on where `foo()` is picked by resolver. + // However, it's not always true. According to ExpressionCodegen, this rule is violated in the case of constructor call. + // It's reasonable since there won't be any ambiguity in this case, but it's simply inconsistent. + // TODO: avoid `allowSuperCall` by convincing people to alter behaviour of frontend + JsExpression alias = getAliasForDescriptor(descriptor); if (alias != null) { return alias; @@ -362,16 +372,13 @@ public class TranslationContext { ClassDescriptor cls = (ClassDescriptor) classifier; - JsExpression receiver = getAliasForDescriptor(cls.getThisAsReceiverParameter()); - - // I could reproduce situation when receiver == null, although there's no test to reproduce it. Have no time for investigation - // Hint: try generating field with getter/setter, see deepInnerClassInLocalClass.kt - // TODO: revisit this code later + assert classDescriptor != null : "Can't get ReceiverParameterDescriptor in top level"; + JsExpression receiver = getAliasForDescriptor(classDescriptor.getThisAsReceiverParameter()); if (receiver == null) { receiver = JsLiteral.THIS; } - return getDispatchReceiverPath(cls, receiver); + return getDispatchReceiverPath(cls, receiver, allowSuperCall); } private boolean isConstructorOrDirectScope(DeclarationDescriptor descriptor) { @@ -384,7 +391,7 @@ public class TranslationContext { } @NotNull - private JsExpression getDispatchReceiverPath(@Nullable ClassDescriptor cls, JsExpression thisExpression) { + private JsExpression getDispatchReceiverPath(@Nullable ClassDescriptor cls, JsExpression thisExpression, boolean allowSuperCall) { if (cls != null) { JsExpression alias = getAliasForDescriptor(cls); if (alias != null) { @@ -392,17 +399,16 @@ public class TranslationContext { } } if (classDescriptor == cls || - (classDescriptor != null && - cls != null && DescriptorUtils.isSubclass(classDescriptor, cls)) || + (allowSuperCall && classDescriptor != null && cls != null && DescriptorUtils.isSubclass(classDescriptor, cls)) || parent == null) { return thisExpression; } ClassDescriptor parentDescriptor = parent.classDescriptor; if (classDescriptor != parentDescriptor) { - return new JsNameRef(Namer.OUTER_FIELD_NAME, parent.getDispatchReceiverPath(cls, thisExpression)); + return new JsNameRef(Namer.OUTER_FIELD_NAME, parent.getDispatchReceiverPath(cls, thisExpression, allowSuperCall)); } else { - return parent.getDispatchReceiverPath(cls, thisExpression); + return parent.getDispatchReceiverPath(cls, thisExpression, allowSuperCall); } } @@ -496,7 +502,7 @@ public class TranslationContext { JsExpression alias = getAliasForDescriptor(descriptor); if (alias != null) return alias; if (descriptor instanceof ReceiverParameterDescriptor) { - return getDispatchReceiver((ReceiverParameterDescriptor) descriptor); + return getDispatchReceiver((ReceiverParameterDescriptor) descriptor, false); } return getNameForDescriptor(descriptor).makeRef(); } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java index 170f3b8e048..d8788545bb6 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java @@ -468,7 +468,8 @@ public final class ExpressionVisitor extends TranslatorVisitor { getDescriptorForReferenceExpression(context.bindingContext(), expression.getInstanceReference()); assert thisExpression != null : "This expression must reference a descriptor: " + expression.getText(); - return context.getDispatchReceiver(getReceiverParameterForDeclaration(thisExpression)).source(expression); + // TODO: not sure if `false` is a proper argument here, revisit this code later + return context.getDispatchReceiver(getReceiverParameterForDeclaration(thisExpression), false).source(expression); } @Override @@ -483,7 +484,9 @@ public final class ExpressionVisitor extends TranslatorVisitor { public JsNode visitSuperExpression(@NotNull KtSuperExpression expression, @NotNull TranslationContext context) { DeclarationDescriptor superTarget = getSuperTarget(context, expression); ReceiverParameterDescriptor receiver = getReceiverParameterForDeclaration(superTarget); - return context.getDispatchReceiver(receiver); + + // TODO: not sure if `true` is a proper argument here, revisit this code later + return context.getDispatchReceiver(receiver, true); } @Override diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/TranslationUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/TranslationUtils.java index b0a7fe17058..9ccde9782e2 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/TranslationUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/TranslationUtils.java @@ -156,7 +156,7 @@ public final class TranslationUtils { receiver = JsLiteral.THIS; } else { - receiver = context.getDispatchReceiver(JsDescriptorUtils.getReceiverParameterForDeclaration(containingDescriptor)); + receiver = context.getDispatchReceiver(JsDescriptorUtils.getReceiverParameterForDeclaration(containingDescriptor), false); } return new JsNameRef(backingFieldName, receiver); } diff --git a/js/js.translator/testData/closure/cases/deepInnerClassInLocalClass.kt b/js/js.translator/testData/closure/cases/deepInnerClassInLocalClass.kt index 967870deac9..6980413fb78 100644 --- a/js/js.translator/testData/closure/cases/deepInnerClassInLocalClass.kt +++ b/js/js.translator/testData/closure/cases/deepInnerClassInLocalClass.kt @@ -3,13 +3,13 @@ package foo class A() { fun test(): Int { open class B(open val x: Int) { - inner class C(override val x: Int) : B(x * 10) { + inner class C(x: Int) : B(x * 10) { inner class D() { var baz: () -> Int = { 0 } constructor(b: Boolean) : this() { baz = { x + this@B.x } } - fun bar() = { x + this@B.x } + fun bar() = { 100 * (x + this@B.x) } } } } diff --git a/js/js.translator/testData/closure/cases/deepInnerClassInLocalClassFromExtension.kt b/js/js.translator/testData/closure/cases/deepInnerClassInLocalClassFromExtension.kt index bd168d1fdce..fd263f98dc2 100644 --- a/js/js.translator/testData/closure/cases/deepInnerClassInLocalClassFromExtension.kt +++ b/js/js.translator/testData/closure/cases/deepInnerClassInLocalClassFromExtension.kt @@ -3,7 +3,7 @@ package foo class A() { fun test(): Int { open class B(open val x: Int) { - inner class C(override val x: Int) : B(x * 10) { + inner class C(x: Int) : B(x * 10) { inner class D() { fun baz() = bar() } diff --git a/js/js.translator/testData/nestedTypes/cases/outerClassReferenceFromSecondaryConstructor.kt b/js/js.translator/testData/nestedTypes/cases/outerClassReferenceFromSecondaryConstructor.kt new file mode 100644 index 00000000000..29b394fe985 --- /dev/null +++ b/js/js.translator/testData/nestedTypes/cases/outerClassReferenceFromSecondaryConstructor.kt @@ -0,0 +1,19 @@ +package foo + +class A(val x: Int) { + inner class B() { + inner class C() { + var result = 0 + + constructor(y: Boolean) : this() { + result = x + } + } + } +} + +fun box(): String { + assertEquals(23, A(23).B().C(true).result) + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/nestedTypes/cases/privateFieldNotOverridenInNestedSubclass.kt b/js/js.translator/testData/nestedTypes/cases/privateFieldNotOverridenInNestedSubclass.kt new file mode 100644 index 00000000000..f73f5fbd55a --- /dev/null +++ b/js/js.translator/testData/nestedTypes/cases/privateFieldNotOverridenInNestedSubclass.kt @@ -0,0 +1,18 @@ +package foo + +open class A(private val bar: String = "1") { + inner class B : A("2") { + fun foo(a: A): String { + return bar + a.bar + } + } +} + +fun box(): String { + var r = "" + + r = A().B().foo(A("3")) + if (r != "13") return "r = $r" + + return "OK" +} \ No newline at end of file