From ae14d185eb4c1f3e86ddd2696f860acd2925d4d5 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 3 Mar 2016 12:33:35 +0300 Subject: [PATCH] Support setters for Java-method-based properties in reflection Also heuristically support the corner case of multiple properties with the same name and signature in a Java class, see the comment in KDeclarationContainerImpl #KT-11258 Fixed --- .../overrideKotlinPropertyByJavaMethod.kt | 39 +++++++++++++++++++ .../BlackBoxWithJavaCodegenTestGenerated.java | 6 +++ .../jvm/internal/KDeclarationContainerImpl.kt | 20 ++++++++++ .../reflect/jvm/internal/KPropertyImpl.kt | 8 +++- .../reflect/jvm/internal/RuntimeTypeMapper.kt | 9 +++-- 5 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/boxWithJava/reflection/overrideKotlinPropertyByJavaMethod.kt diff --git a/compiler/testData/codegen/boxWithJava/reflection/overrideKotlinPropertyByJavaMethod.kt b/compiler/testData/codegen/boxWithJava/reflection/overrideKotlinPropertyByJavaMethod.kt new file mode 100644 index 00000000000..ac847b2d468 --- /dev/null +++ b/compiler/testData/codegen/boxWithJava/reflection/overrideKotlinPropertyByJavaMethod.kt @@ -0,0 +1,39 @@ +// WITH_REFLECT +// FILE: J.java + +public class J implements K { + private String foo; + + @Override + public String getFoo() { + return foo; + } + + @Override + public void setFoo(String s) { + foo = s; + } +} + +// FILE: K.kt + +import kotlin.test.assertEquals +import kotlin.reflect.KParameter + +interface K { + var foo: String +} + +fun box(): String { + val p = J::foo + assertEquals("foo", p.name) + + if (p.parameters.size != 1) return "Should have only 1 parameter" + if (p.parameters.single().kind != KParameter.Kind.INSTANCE) return "Should have an instance parameter" + + if (J::class.members.none { it == p }) return "No foo in members" + + val j = J() + p.setter.call(j, "OK") + return p.getter.call(j) +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxWithJavaCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxWithJavaCodegenTestGenerated.java index 69c3d9f3ec9..caf303962cc 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxWithJavaCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxWithJavaCodegenTestGenerated.java @@ -667,6 +667,12 @@ public class BlackBoxWithJavaCodegenTestGenerated extends AbstractBlackBoxCodege doTest(fileName); } + @TestMetadata("overrideKotlinPropertyByJavaMethod.kt") + public void testOverrideKotlinPropertyByJavaMethod() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxWithJava/reflection/overrideKotlinPropertyByJavaMethod.kt"); + doTest(fileName); + } + @TestMetadata("parametersHaveNoNames.kt") public void testParametersHaveNoNames() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxWithJava/reflection/parametersHaveNoNames.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 7c940bc7374..84a4be1d113 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt @@ -26,6 +26,7 @@ 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 import kotlin.reflect.KotlinReflectionInternalError @@ -106,6 +107,25 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain } if (properties.size != 1) { + // Try working around the case of a Java class with a field 'foo' and a method 'getFoo' which overrides Kotlin property 'foo'. + // Such class has two property descriptors with the name 'foo' in its scope and they may be indistinguishable from each other. + // However, it's not possible to write 'A::foo' if they're indistinguishable; overload resolution would not be able to choose + // between the two. So we assume that one of the properties must have a greater visibility than the other, and try loading + // that one first. + // Note that this heuristic may result in _incorrect behavior_ if a KProperty object for a less visible property is obtained + // by other means (through reflection API) and then the soft-referenced descriptor instance for that property is invalidated + // because there's no more memory left. In that case the KProperty object will now point to another (more visible) property. + // TODO: consider writing additional info (besides signature) to property reference objects to distinguish them in this case + + val mostVisibleProperties = properties + .groupBy { it.visibility } + .toSortedMap(Comparator { first, second -> + Visibilities.compare(first, second) ?: 0 + }).values.last() + if (mostVisibleProperties.size == 1) { + return mostVisibleProperties.first() + } + val debugText = "'$name' (JVM signature: $signature)" throw KotlinReflectionInternalError( if (properties.isEmpty()) "Property $debugText not resolved in $this" 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 370b1cdd2c4..f15b53a6194 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt @@ -152,8 +152,12 @@ private fun KPropertyImpl.Accessor<*>.computeCallerForAccessor(isGetter: Boolean computeFieldCaller(jvmSignature.field) } is JvmPropertySignature.JavaMethodProperty -> { - if (!isGetter) throw KotlinReflectionInternalError("Setter requested for special built-in $this") - FunctionCaller.InstanceMethod(jvmSignature.method) + val method = + if (isGetter) jvmSignature.getterMethod + else jvmSignature.setterMethod ?: throw KotlinReflectionInternalError( + "No source found for setter of Java method property: ${jvmSignature.getterMethod}" + ) + FunctionCaller.InstanceMethod(method) } } } diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt index 3fa3e279e59..3eeee629d53 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt @@ -129,8 +129,8 @@ internal sealed class JvmPropertySignature { override fun asString(): String = string } - class JavaMethodProperty(val method: Method) : JvmPropertySignature() { - override fun asString(): String = method.signature + class JavaMethodProperty(val getterMethod: Method, val setterMethod: Method?) : JvmPropertySignature() { + override fun asString(): String = getterMethod.signature } class JavaField(val field: Field) : JvmPropertySignature() { @@ -206,7 +206,10 @@ internal object RuntimeTypeMapper { val element = (property.source as? JavaSourceElement)?.javaElement when (element) { is ReflectJavaField -> return JvmPropertySignature.JavaField(element.member) - is ReflectJavaMethod -> return JvmPropertySignature.JavaMethodProperty(element.member) + is ReflectJavaMethod -> return JvmPropertySignature.JavaMethodProperty( + element.member, + ((property.setter?.source as? JavaSourceElement)?.javaElement as? ReflectJavaMethod)?.member + ) else -> throw KotlinReflectionInternalError("Incorrect resolution sequence for Java field $property (source = $element)") } }