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

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<Derived, String>
+    publicVarProtectedSet.setter.call(a, "2")
+    assertEquals("2", publicVarProtectedSet.getter.call(a))
+    assertEquals("2", publicVarProtectedSet.call(a))
+
+    assertEquals("3", member("protectedFun").call(a))
+
+    return "OK"
+}

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");

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");

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");

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<Class<*>>, returnType: Class<*>, declared: Boolean): Method? =
+    private fun Class<*>.lookupMethod(name: String, parameterTypes: List<Class<*>>, 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<Class<*>>, 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<Class<*>>, declared: Boolean) =
+    private fun Class<*>.tryGetConstructor(parameterTypes: List<Class<*>>, 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 == "<init>") 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 == "<init>") return null
 
         val parameterTypes = arrayListOf<Class<*>>()
@@ -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<Class<*>>()
         addParametersAndMasks(parameterTypes, desc, true)
 
-        return jClass.tryGetConstructor(parameterTypes, declared)
+        return jClass.tryGetConstructor(parameterTypes, declared = !isPublic)
     }
 
     private fun addParametersAndMasks(result: MutableList<Class<*>>, desc: String, isConstructor: Boolean) {

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

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)

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()
+    }

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");