From 6388c1885c67d6e655824d2fce1b498f8f1b4f08 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 20 Jun 2017 17:48:09 +0300 Subject: [PATCH] Fix KCallable.call for protected members from base class Class.getMethod does not return protected methods from super class, so we invoke getDeclaredMethod on each super class manually instead #KT-18480 Fixed --- .../box/reflection/call/protectedMembers.kt | 35 +++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 6 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 +++ .../LightAnalysisModeTestGenerated.java | 6 +++ .../jvm/internal/KDeclarationContainerImpl.kt | 45 +++++++++++++------ .../reflect/jvm/internal/KFunctionImpl.kt | 17 +++---- .../reflect/jvm/internal/KPropertyImpl.kt | 7 ++- .../src/kotlin/reflect/jvm/internal/util.kt | 7 +++ .../semantics/JsCodegenBoxTestGenerated.java | 12 +++++ 9 files changed, 113 insertions(+), 28 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/call/protectedMembers.kt diff --git a/compiler/testData/codegen/box/reflection/call/protectedMembers.kt b/compiler/testData/codegen/box/reflection/call/protectedMembers.kt new file mode 100644 index 00000000000..ab438543565 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/call/protectedMembers.kt @@ -0,0 +1,35 @@ +// IGNORE_BACKEND: JS, NATIVE +// WITH_REFLECT + +import kotlin.reflect.* +import kotlin.reflect.jvm.* +import kotlin.test.* + +abstract class Base { + protected val protectedVal: String + get() = "1" + + var publicVarProtectedSet: String = "" + protected set + + protected fun protectedFun(): String = "3" +} + +class Derived : Base() + +fun member(name: String): KCallable<*> = Derived::class.members.single { it.name == name }.apply { isAccessible = true } + +fun box(): String { + val a = Derived() + + assertEquals("1", member("protectedVal").call(a)) + + val publicVarProtectedSet = member("publicVarProtectedSet") as KMutableProperty1 + publicVarProtectedSet.setter.call(a, "2") + assertEquals("2", publicVarProtectedSet.getter.call(a)) + assertEquals("2", publicVarProtectedSet.call(a)) + + assertEquals("3", member("protectedFun").call(a)) + + return "OK" +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 5402f760d3a..28c26aa9392 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -14008,6 +14008,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("protectedMembers.kt") + public void testProtectedMembers() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/protectedMembers.kt"); + doTest(fileName); + } + @TestMetadata("returnUnit.kt") public void testReturnUnit() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/returnUnit.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 04f6b2e6851..2d790151026 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -14008,6 +14008,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("protectedMembers.kt") + public void testProtectedMembers() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/protectedMembers.kt"); + doTest(fileName); + } + @TestMetadata("returnUnit.kt") public void testReturnUnit() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/returnUnit.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 2a0f9f9c5e7..ea33ea6f66c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -14008,6 +14008,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("protectedMembers.kt") + public void testProtectedMembers() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/protectedMembers.kt"); + doTest(fileName); + } + @TestMetadata("returnUnit.kt") public void testReturnUnit() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/returnUnit.kt"); diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt index 33f93595d37..fde05d29532 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt @@ -26,7 +26,6 @@ import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.scopes.MemberScope import java.lang.reflect.Constructor import java.lang.reflect.Method -import java.util.* import kotlin.jvm.internal.ClassBasedDeclarationContainer internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContainer { @@ -151,10 +150,28 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain return functions.single() } - private fun Class<*>.tryGetMethod(name: String, parameterTypes: List>, returnType: Class<*>, declared: Boolean): Method? = + private fun Class<*>.lookupMethod(name: String, parameterTypes: List>, returnType: Class<*>, isPublic: Boolean): Method? { + val parametersArray = parameterTypes.toTypedArray() + + // If we're looking for a public method, just use Java reflection's getMethod/getMethods + if (isPublic) { + return tryGetMethod(name, parametersArray, returnType, declared = false) + } + + // If we're looking for a non-public method, it might be located not only in this class, but also in any of its superclasses + var klass: Class<*>? = this + while (klass != null) { + val method = klass.tryGetMethod(name, parametersArray, returnType, declared = true) + if (method != null) return method + klass = klass.superclass + } + + return null + } + + private fun Class<*>.tryGetMethod(name: String, parameterTypes: Array>, returnType: Class<*>, declared: Boolean): Method? = try { - val parametersArray = parameterTypes.toTypedArray() - val result = if (declared) getDeclaredMethod(name, *parametersArray) else getMethod(name, *parametersArray) + val result = if (declared) getDeclaredMethod(name, *parameterTypes) else getMethod(name, *parameterTypes) if (result.returnType == returnType) result else { @@ -166,7 +183,7 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain allMethods.firstOrNull { method -> method.name == name && method.returnType == returnType && - Arrays.equals(method.parameterTypes, parametersArray) + method.parameterTypes.contentEquals(parameterTypes) } } } @@ -174,7 +191,7 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain null } - private fun Class<*>.tryGetConstructor(parameterTypes: List>, declared: Boolean) = + private fun Class<*>.tryGetConstructor(parameterTypes: List>, declared: Boolean): Constructor<*>? = try { if (declared) getDeclaredConstructor(*parameterTypes.toTypedArray()) else getConstructor(*parameterTypes.toTypedArray()) @@ -183,13 +200,13 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain null } - fun findMethodBySignature(name: String, desc: String, declared: Boolean): Method? { + fun findMethodBySignature(name: String, desc: String, isPublic: Boolean): Method? { if (name == "") return null - return methodOwner.tryGetMethod(name, loadParameterTypes(desc), loadReturnType(desc), declared) + return methodOwner.lookupMethod(name, loadParameterTypes(desc), loadReturnType(desc), isPublic) } - fun findDefaultMethod(name: String, desc: String, isMember: Boolean, declared: Boolean): Method? { + fun findDefaultMethod(name: String, desc: String, isMember: Boolean, isPublic: Boolean): Method? { if (name == "") return null val parameterTypes = arrayListOf>() @@ -198,18 +215,18 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain } addParametersAndMasks(parameterTypes, desc, false) - return methodOwner.tryGetMethod(name + JvmAbi.DEFAULT_PARAMS_IMPL_SUFFIX, parameterTypes, loadReturnType(desc), declared) + return methodOwner.lookupMethod(name + JvmAbi.DEFAULT_PARAMS_IMPL_SUFFIX, parameterTypes, loadReturnType(desc), isPublic) } - fun findConstructorBySignature(desc: String, declared: Boolean): Constructor<*>? { - return jClass.tryGetConstructor(loadParameterTypes(desc), declared) + fun findConstructorBySignature(desc: String, isPublic: Boolean): Constructor<*>? { + return jClass.tryGetConstructor(loadParameterTypes(desc), declared = !isPublic) } - fun findDefaultConstructor(desc: String, declared: Boolean): Constructor<*>? { + fun findDefaultConstructor(desc: String, isPublic: Boolean): Constructor<*>? { val parameterTypes = arrayListOf>() addParametersAndMasks(parameterTypes, desc, true) - return jClass.tryGetConstructor(parameterTypes, declared) + return jClass.tryGetConstructor(parameterTypes, declared = !isPublic) } private fun addParametersAndMasks(result: MutableList>, desc: String, isConstructor: Boolean) { diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KFunctionImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KFunctionImpl.kt index fb104310d0d..308becedf63 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KFunctionImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KFunctionImpl.kt @@ -18,8 +18,6 @@ package kotlin.reflect.jvm.internal import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.descriptors.Visibilities -import org.jetbrains.kotlin.descriptors.annotations.isEffectivelyInlineOnly import java.lang.reflect.Constructor import java.lang.reflect.Member import java.lang.reflect.Method @@ -58,21 +56,16 @@ internal class KFunctionImpl private constructor( override val name: String get() = descriptor.name.asString() - private fun isPrivateInBytecode(): Boolean = - Visibilities.isPrivate(descriptor.visibility) || - descriptor.isEffectivelyInlineOnly() - - private fun isDeclared(): Boolean = isPrivateInBytecode() - override val caller: FunctionCaller<*> by ReflectProperties.lazySoft caller@ { val jvmSignature = RuntimeTypeMapper.mapSignature(descriptor) val member: Member? = when (jvmSignature) { is KotlinConstructor -> { if (isAnnotationConstructor) return@caller AnnotationConstructorCaller(container.jClass, parameters.map { it.name!! }, POSITIONAL_CALL, KOTLIN) - container.findConstructorBySignature(jvmSignature.constructorDesc, isDeclared()) + container.findConstructorBySignature(jvmSignature.constructorDesc, descriptor.isPublicInBytecode) } - is KotlinFunction -> container.findMethodBySignature(jvmSignature.methodName, jvmSignature.methodDesc, isDeclared()) + is KotlinFunction -> + container.findMethodBySignature(jvmSignature.methodName, jvmSignature.methodDesc, descriptor.isPublicInBytecode) is JavaMethod -> jvmSignature.method is JavaConstructor -> jvmSignature.constructor is FakeJavaAnnotationConstructor -> { @@ -102,12 +95,12 @@ internal class KFunctionImpl private constructor( val member: Member? = when (jvmSignature) { is KotlinFunction -> { container.findDefaultMethod(jvmSignature.methodName, jvmSignature.methodDesc, - !Modifier.isStatic(caller.member!!.modifiers), isDeclared()) + !Modifier.isStatic(caller.member!!.modifiers), descriptor.isPublicInBytecode) } is KotlinConstructor -> { if (isAnnotationConstructor) return@defaultCaller AnnotationConstructorCaller(container.jClass, parameters.map { it.name!! }, CALL_BY_NAME, KOTLIN) - container.findDefaultConstructor(jvmSignature.constructorDesc, isDeclared()) + container.findDefaultConstructor(jvmSignature.constructorDesc, descriptor.isPublicInBytecode) } is FakeJavaAnnotationConstructor -> { val methods = jvmSignature.methods diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt index 6c8ef253178..d4e4a71e61d 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt @@ -235,12 +235,15 @@ private fun KPropertyImpl.Accessor<*, *>.computeCallerForAccessor(isGetter: Bool property.container.findMethodBySignature( jvmSignature.nameResolver.getString(signature.name), jvmSignature.nameResolver.getString(signature.desc), - Visibilities.isPrivate(descriptor.visibility) + descriptor.isPublicInBytecode ) } when { - accessor == null -> computeFieldCaller(property.javaField!!) + accessor == null -> computeFieldCaller( + property.javaField + ?: throw KotlinReflectionInternalError("No accessors or field is found for property $property") + ) !Modifier.isStatic(accessor.modifiers) -> if (isBound) FunctionCaller.BoundInstanceMethod(accessor, property.boundReceiver) else FunctionCaller.InstanceMethod(accessor) diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt index d66b21ce79b..04ac8c56422 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt @@ -16,10 +16,12 @@ package kotlin.reflect.jvm.internal +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.descriptors.annotations.Annotated +import org.jetbrains.kotlin.descriptors.annotations.isEffectivelyInlineOnly import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.load.java.components.RuntimeSourceElementFactory import org.jetbrains.kotlin.load.java.reflect.tryLoadClass @@ -145,3 +147,8 @@ internal val ReflectKotlinClass.packageModuleName: String? } } +internal val CallableMemberDescriptor.isPublicInBytecode: Boolean + get() { + val visibility = visibility + return (visibility == Visibilities.PUBLIC || visibility == Visibilities.INTERNAL) && !isEffectivelyInlineOnly() + } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index d2b3b149800..b75339e6253 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -15988,6 +15988,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } + @TestMetadata("protectedMembers.kt") + public void testProtectedMembers() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/protectedMembers.kt"); + try { + doTest(fileName); + } + catch (Throwable ignore) { + return; + } + throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); + } + @TestMetadata("returnUnit.kt") public void testReturnUnit() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/call/returnUnit.kt");