Generalize parameter index calculation in ASM 7 support

Second part for d2a205c72d commit #KT-27774 Fixed
2018-11-12 09:41:18 +01:00
parent e044ec78a1
commit 7ee13ca353
23 changed files with 376 additions and 11 deletions
@@ -29,10 +29,14 @@ internal class AnnotationsAndParameterCollectorMethodVisitor(
        private val member: BinaryJavaMethodBase,
        private val context: ClassifierResolutionContext,
        private val signatureParser: BinaryClassSignatureParser,
-        private val parametersToSkipNumber: Int
+        private val parametersToSkipNumber: Int,
+        private val parametersCountInMethodDesc: Int
 ) : MethodVisitor(ASM_API_VERSION_FOR_CLASS_READING) {
    private var parameterIndex = 0

+    private var visibleAnnotableParameterCount = parametersCountInMethodDesc
+    private var invisibleAnnotableParameterCount = parametersCountInMethodDesc
+
    override fun visitAnnotationDefault(): AnnotationVisitor? {
        member.safeAs<BinaryJavaMethod>()?.hasAnnotationParameterDefaultValue = true
        // We don't store default value in Java model
@@ -59,8 +63,19 @@ internal class AnnotationsAndParameterCollectorMethodVisitor(
                    desc, context, signatureParser
            )

+    @Suppress("NOTHING_TO_OVERRIDE")
+    override fun visitAnnotableParameterCount(parameterCount: Int, visible: Boolean) {
+        if (visible) {
+            visibleAnnotableParameterCount = parameterCount
+        } else {
+            invisibleAnnotableParameterCount = parameterCount
+        }
+    }
+
    override fun visitParameterAnnotation(parameter: Int, desc: String, visible: Boolean): AnnotationVisitor? {
-        val index = if (Opcodes.API_VERSION <= Opcodes.ASM6) parameter - parametersToSkipNumber else parameter
+        val absoluteParameterIndex =
+            parameter + parametersCountInMethodDesc - if (visible) visibleAnnotableParameterCount else invisibleAnnotableParameterCount
+        val index = absoluteParameterIndex - parametersToSkipNumber
        if (index < 0) return null

        val annotations =
@@ -106,7 +106,14 @@ abstract class BinaryJavaMethodBase(
                else -> 0
            }

-            return member to AnnotationsAndParameterCollectorMethodVisitor(member, parentContext, signatureParser, paramIgnoreCount)
+            return member to
+                    AnnotationsAndParameterCollectorMethodVisitor(
+                        member,
+                        parentContext,
+                        signatureParser,
+                        paramIgnoreCount,
+                        Type.getArgumentTypes(desc).size
+                    )
        }

        private fun parseMethodDescription(
@@ -111,6 +111,28 @@ class SimpleKotlinGradleIT : BaseGradleIT() {
    @Test
    fun testGroovyInterop() {
        Project("groovyInterop").build("build") {
+            assertTasksExecuted(":test")
+            assertContains("GroovyInteropTest PASSED")
+            assertSuccessful()
+        }
+    }
+
+    //Proguard corrupts RuntimeInvisibleParameterAnnotations/RuntimeVisibleParameterAnnotations tables:
+    // https://sourceforge.net/p/proguard/bugs/735/
+    @Test
+    fun testInteropWithProguarded() {
+        Project("interopWithProguarded").build("build") {
+            assertTasksExecuted(":test")
+            assertContains("InteropWithProguardedTest PASSED")
+            assertSuccessful()
+        }
+    }
+
+    @Test
+    fun testScalaInterop() {
+        Project("scalaInterop").build("build") {
+            assertTasksExecuted(":test")
+            assertContains("ScalaInteropTest PASSED")
            assertSuccessful()
        }
    }
@@ -18,6 +18,8 @@ repositories {

 dependencies {
    compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
+    compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
+    testCompile 'junit:junit:4.12'
    compile project(":lib")
 }

@@ -1,7 +1,11 @@
 public class ClassWithReferenceToInner {
-    public void f1(Thread.State state) {}
+    public String f1(Thread.State state) {
+        return "OK"
+    }

-    public void f2(Outer.Nested nested) {}
+    public String f2(Outer.Nested nested) {
+        return "OK"
+    }
 }

 class Outer {
@@ -0,0 +1,17 @@
+package genum
+
+import java.lang.annotation.Retention
+import java.lang.annotation.RetentionPolicy
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface FooEnum {}
+
+enum GEnum {
+    FOO("123");
+
+    String value
+
+    GEnum(@FooEnum String value) {
+        this.value = value
+    }
+}
@@ -0,0 +1,17 @@
+package inner
+
+import java.lang.annotation.Retention
+import java.lang.annotation.RetentionPolicy
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface FooInner {}
+
+class Outer {
+    class Inner {
+        String name
+
+        Inner(@FooInner String name) {
+            this.name = name
+        }
+    }
+}
@@ -1,6 +0,0 @@
-fun main(args: Array<String>) {
-    System.out.println(MyTraitAccessor().myField)
-
-    System.out.println(ClassWithReferenceToInner().f1(null))
-    System.out.println(ClassWithReferenceToInner().f2(null))
-}
@@ -0,0 +1,41 @@
+import org.junit.Assert
+import kotlin.reflect.full.valueParameters
+
+class GroovyInteropTest {
+
+    @org.junit.Test
+    fun classWithReferenceToInner() {
+        Assert.assertEquals("OK", ClassWithReferenceToInner().f1(null))
+        Assert.assertEquals("OK", ClassWithReferenceToInner().f2(null))
+    }
+
+    @org.junit.Test
+    fun groovyTraitAccessor() {
+        Assert.assertEquals(1, MyTraitAccessor().myField)
+    }
+
+    @org.junit.Test
+    fun parametersInInnerClassConstructor() {
+        val inner = inner.Outer().Inner("123")
+        Assert.assertEquals("123", inner.name)
+
+        val valueParameters = inner::class.constructors.single().valueParameters
+        Assert.assertEquals(1, valueParameters.size)
+        val annotations = valueParameters[0].annotations
+        Assert.assertEquals(1, annotations.size)
+        Assert.assertEquals("FooInner", annotations[0].annotationClass.simpleName)
+    }
+
+    @org.junit.Test
+    fun parametersInEnumConstructor() {
+        val enumValue = genum.GEnum.FOO
+        Assert.assertEquals("123", enumValue.value)
+
+        val valueParameters = enumValue::class.constructors.single().valueParameters
+        Assert.assertTrue(valueParameters.isNotEmpty())
+        //valueParameters.last() cause Groovy doesn't mark name and ordinal as synthetic and doesn't generate signature
+        val annotations = valueParameters.last().annotations
+        Assert.assertEquals(1, annotations.size)
+        Assert.assertEquals("FooEnum", annotations[0].annotationClass.simpleName)
+    }
+}
@@ -0,0 +1,28 @@
+buildscript {
+    repositories {
+        mavenLocal()
+        mavenCentral()
+    }
+    dependencies {
+        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
+    }
+}
+
+apply plugin: "kotlin"
+apply plugin: "java"
+
+repositories {
+    mavenLocal()
+    mavenCentral()
+}
+
+dependencies {
+    compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
+    compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
+    testCompile 'junit:junit:4.12'
+    compile project(path: ":lib", configuration: "proguarded")
+}
+
+compileKotlin {
+    kotlinOptions.jvmTarget = "1.8"
+}
@@ -0,0 +1,52 @@
+apply plugin: 'java'
+apply plugin: 'kotlin'
+
+buildscript {
+    repositories {
+        mavenLocal()
+        mavenCentral()
+    }
+    dependencies {
+        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
+        classpath 'net.sf.proguard:proguard-gradle:6.0.3'
+    }
+}
+
+repositories {
+    mavenLocal()
+    mavenCentral()
+}
+
+dependencies {
+    compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
+
+}
+
+task obfuscate(type: proguard.gradle.ProGuardTask, dependsOn: jar) {
+    injars "${jar.archivePath}"
+    outjars "$buildDir/proguard/out.jar"
+    libraryjars "$System.env.JAVA_HOME/jre/lib/rt.jar"
+    libraryjars configurations["compile"]
+
+    dontobfuscate
+    dontoptimize
+    keep 'class *'
+    keepclassmembers 'class * { \
+            public <init>(...); \
+            }'
+
+}
+
+configurations {
+    proguarded
+}
+
+artifacts {
+    proguarded(file(obfuscate.outJarFiles[0])) {
+        name 'out'
+        type 'jar'
+        builtBy obfuscate
+    }
+}
+
+
@@ -0,0 +1,13 @@
+package jenum;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+public enum JEnum {
+    OK("123");
+
+    @Retention(RetentionPolicy.RUNTIME)
+    public @interface Foo {}
+
+    JEnum(@Foo String foo) {}
+}
@@ -0,0 +1,17 @@
+package iclass;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+public class JInnerClass {
+    @Retention(RetentionPolicy.RUNTIME)
+    public @interface Foo {}
+
+    public class Inner {
+        public String foo;
+
+        public Inner(@Foo String foo) {
+            this.foo = foo;
+        }
+    }
+}
@@ -0,0 +1,11 @@
+package kenum
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+enum class KEnum(@Foo val foo: String) {
+    OK("123");
+
+    @Retention(RetentionPolicy.RUNTIME)
+    annotation class Foo {}
+}
@@ -0,0 +1,11 @@
+package kclass
+
+import java.lang.annotation.Retention
+import java.lang.annotation.RetentionPolicy
+
+public class KInnerClass {
+    @Retention(RetentionPolicy.RUNTIME)
+    annotation class Foo
+
+    inner class Inner(@Foo val foo: String){}
+}
@@ -0,0 +1 @@
+include ":lib"
@@ -0,0 +1,40 @@
+import org.junit.Assert
+import kotlin.reflect.full.valueParameters
+import kotlin.reflect.KClass
+
+class InteropWithProguardedTest {
+
+    @org.junit.Test
+    fun parametersInInnerJavaClassConstructor() {
+        val inner = iclass.JInnerClass().Inner("123")
+        testAnnotationsInConstructor(inner::class)
+    }
+
+    @org.junit.Test
+    fun parametersInInnerKotlinClassConstructor() {
+        val inner = kclass.KInnerClass().Inner("123")
+        testAnnotationsInConstructor(inner::class)
+    }
+
+    @org.junit.Test
+    fun parametersInJavaEnumConstructor() {
+        val enumValue = jenum.JEnum.OK
+        Assert.assertEquals("OK", enumValue.name)
+        testAnnotationsInConstructor(enumValue::class)
+    }
+
+    @org.junit.Test
+    fun parametersInKotlinEnumConstructor() {
+        val enumValue = kenum.KEnum.OK
+        Assert.assertEquals("OK", enumValue.name)
+        testAnnotationsInConstructor(enumValue::class)
+    }
+
+    private fun testAnnotationsInConstructor(clazz: KClass<*>) {
+        val valueParameters = clazz.constructors.single().valueParameters
+        Assert.assertTrue(valueParameters.isNotEmpty())
+        val annotations = valueParameters[0].annotations
+        Assert.assertEquals(1, annotations.size)
+        Assert.assertEquals("Foo", annotations[0].annotationClass.simpleName)
+    }
+}
@@ -0,0 +1,28 @@
+buildscript {
+    repositories {
+        mavenLocal()
+        mavenCentral()
+    }
+    dependencies {
+        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
+    }
+}
+
+apply plugin: "kotlin"
+apply plugin: "java"
+
+repositories {
+    mavenLocal()
+    mavenCentral()
+}
+
+dependencies {
+    compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
+    compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
+    testCompile 'junit:junit:4.12'
+    compile project(":lib")
+}
+
+compileKotlin {
+    kotlinOptions.jvmTarget = "1.8"
+}
@@ -0,0 +1,10 @@
+apply plugin: "java"
+apply plugin: "scala"
+
+repositories {
+    mavenCentral()
+}
+
+dependencies {
+    implementation 'org.scala-lang:scala-library:2.12.7'
+}
@@ -0,0 +1,7 @@
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Foo {
+
+}
@@ -0,0 +1,5 @@
+class Outer {
+    class Inner(@Foo val name: String) {
+        var x = 1
+    }
+}
@@ -0,0 +1 @@
+include ":lib"
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+import org.junit.Assert
+import kotlin.reflect.full.valueParameters
+
+class ScalaInteropTest {
+
+    @org.junit.Test
+    fun parametersInInnerClassConstructor() {
+        val inner = Outer().Inner("123")
+        Assert.assertEquals("123", inner.name())
+
+        val valueParameters = inner::class.constructors.single().valueParameters
+        Assert.assertEquals(1, valueParameters.size)
+        val annotations = valueParameters[0].annotations
+        Assert.assertEquals(1, annotations.size)
+        Assert.assertEquals("Foo", annotations[0].annotationClass.simpleName)
+    }
+}