Kapt3: Fix parameter names in JvmOverloads-generated methods (KT-15024)

compiler/backend/src/org/jetbrains/kotlin/codegen/DefaultParameterValueSubstitutor.kt

@@ -29,6 +29,7 @@ import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmOverloadsAnnotation
 import org.jetbrains.kotlin.resolve.jvm.diagnostics.OtherOrigin
 import org.jetbrains.org.objectweb.asm.Label
 import org.jetbrains.org.objectweb.asm.Opcodes
+import org.jetbrains.org.objectweb.asm.Type
 import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter
 
 /**
@@ -36,6 +37,12 @@ import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter
  * parameter values substituted.
  */
 class DefaultParameterValueSubstitutor(val state: GenerationState) {
+    private companion object {
+        // rename -> JvmOverloads
+        private val ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS: String =
+                Type.getObjectType("synthetic/kotlin/jvm/GeneratedByJvmOverloads").descriptor
+    }
+
     /**
      * If all of the parameters of the specified constructor declare default values,
      * generates a no-argument constructor that passes default values for all arguments.
@@ -135,6 +142,10 @@ class DefaultParameterValueSubstitutor(val state: GenerationState) {
 
         AnnotationCodegen.forMethod(mv, memberCodegen, typeMapper).genAnnotations(functionDescriptor, signature.returnType)
 
+        if (state.classBuilderMode == ClassBuilderMode.KAPT3) {
+            mv.visitAnnotation(ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS, /* visible = */ false)
+        }
+
         remainingParameters.withIndex().forEach {
             val annotationCodegen = AnnotationCodegen.forParameter(it.index, mv, memberCodegen, typeMapper)
             annotationCodegen.genAnnotations(it.value, signature.valueParameters[it.index].asmType)

plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt

@@ -63,6 +63,7 @@ class ClassFileToSourceStubConverter(
         private val BLACKLISTED_ANNOTATIONS = listOf(
                 "java.lang.Deprecated", "kotlin.Deprecated", // Deprecated annotations
                 "java.lang.Synthetic",
+                "synthetic.kotlin.jvm.GeneratedByJvmOverloads", // kapt3-related annotation for marking JvmOverloads-generated methods
                 "java.lang.annotation.", // Java annotations
                 "org.jetbrains.annotations.", // Nullable/NotNull, ReadOnly, Mutable
                 "kotlin.jvm.", "kotlin.Metadata" // Kotlin annotations from runtime

plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/parseParameters.kt

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.kapt3.stubs
 
 import org.jetbrains.kotlin.kapt3.util.isAbstract
 import org.jetbrains.kotlin.kapt3.util.isEnum
+import org.jetbrains.kotlin.kapt3.util.isJvmOverloadsGenerated
 import org.jetbrains.kotlin.kapt3.util.isStatic
 import org.jetbrains.org.objectweb.asm.Type
 import org.jetbrains.org.objectweb.asm.tree.AnnotationNode
@@ -36,6 +37,7 @@ internal fun MethodNode.getParametersInfo(containingClass: ClassNode): List<Para
     val localVariables = this.localVariables ?: emptyList()
     val parameters = this.parameters ?: emptyList()
     val isStatic = isStatic(access)
+    val isJvmOverloads = this.isJvmOverloadsGenerated()
 
     // First and second parameters in enum constructors are synthetic, we should ignore them
     val isEnumConstructor = (name == "<init>") && containingClass.isEnum()
@@ -53,7 +55,14 @@ internal fun MethodNode.getParametersInfo(containingClass: ClassNode): List<Para
         else
             null
 
-        name = name ?: localVariables.getOrNull(index + (if (isStatic) 0 else 1))?.name
+        val localVariableIndexOffset = when {
+            isStatic -> 0
+            isJvmOverloads -> 0
+            else -> 1
+        }
+
+        // @JvmOverloads constructors and ordinary methods don't have "this" local variable
+        name = name ?: localVariables.getOrNull(index + localVariableIndexOffset)?.name
                    ?: "p${index - startParameterIndex}"
 
         // Property setters has bad parameter names

plugins/kapt3/src/org/jetbrains/kotlin/kapt3/util/asmUtils.kt

@@ -17,6 +17,8 @@
 package org.jetbrains.kotlin.kapt3.util
 
 import org.jetbrains.org.objectweb.asm.Opcodes
+import org.jetbrains.org.objectweb.asm.Type
+import org.jetbrains.org.objectweb.asm.tree.AnnotationNode
 import org.jetbrains.org.objectweb.asm.tree.ClassNode
 import org.jetbrains.org.objectweb.asm.tree.MethodNode
 
@@ -30,4 +32,18 @@ internal fun ClassNode.isEnum() = (access and Opcodes.ACC_ENUM) != 0
 internal fun ClassNode.isAnnotation() = (access and Opcodes.ACC_ANNOTATION) != 0
 internal fun MethodNode.isVarargs() = (access and Opcodes.ACC_VARARGS) != 0
 
-internal fun <T> List<T>?.isNullOrEmpty() = this == null || this.isEmpty()
+internal fun <T> List<T>?.isNullOrEmpty() = this == null || this.isEmpty()
+
+internal fun MethodNode.isJvmOverloadsGenerated(): Boolean {
+    return (invisibleAnnotations?.any { it.isJvmOverloadsGenerated() } ?: false)
+           || (visibleAnnotations?.any { it.isJvmOverloadsGenerated() } ?: false)
+}
+
+// Constant from DefaultParameterValueSubstitutor can't be used in Maven build because of ProGuard
+// rename this as well
+private val ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS: String =
+        Type.getObjectType("synthetic/kotlin/jvm/GeneratedByJvmOverloads").descriptor
+
+private fun AnnotationNode.isJvmOverloadsGenerated(): Boolean {
+    return this.desc == ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS
+}

plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java

@@ -102,6 +102,12 @@ public class ClassFileToSourceStubConverterTestGenerated extends AbstractClassFi
         doTest(fileName);
     }
 
+    @TestMetadata("jvmOverloads.kt")
+    public void testJvmOverloads() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/jvmOverloads.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("jvmStatic.kt")
     public void testJvmStatic() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/jvmStatic.kt");

plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/KotlinKapt3IntegrationTests.kt

@@ -17,6 +17,8 @@
 package org.jetbrains.kotlin.kapt3.test
 
 import org.junit.Test
+import javax.lang.model.element.ElementKind
+import javax.lang.model.element.ExecutableElement
 
 class KotlinKapt3IntegrationTests : AbstractKotlinKapt3IntegrationTest() {
     @Test
@@ -36,4 +38,30 @@ class KotlinKapt3IntegrationTests : AbstractKotlinKapt3IntegrationTest() {
         assertEquals("firstValue", options["firstKey"])
         assertTrue("secondKey" in options)
     }
+
+    @Test
+    fun testOverloads() = test("Overloads", "test.MyAnnotation") { set, roundEnv, env ->
+        assertEquals(1, set.size)
+        val annotatedElements = roundEnv.getElementsAnnotatedWith(set.single())
+        assertEquals(1, annotatedElements.size)
+        val constructors = annotatedElements
+                .first()
+                .enclosedElements
+                .filter { it.kind == ElementKind.CONSTRUCTOR }
+                .map { it as ExecutableElement }
+                .sortedBy { it.parameters.size }
+        assertEquals(2, constructors.size)
+        assertEquals(2, constructors[0].parameters.size)
+        assertEquals(3, constructors[1].parameters.size)
+        assertEquals("int", constructors[0].parameters[0].asType().toString())
+        assertEquals("long", constructors[0].parameters[1].asType().toString())
+        assertEquals("int", constructors[1].parameters[0].asType().toString())
+        assertEquals("long", constructors[1].parameters[1].asType().toString())
+        assertEquals("java.lang.String", constructors[1].parameters[2].asType().toString())
+        assertEquals("someInt", constructors[0].parameters[0].simpleName.toString())
+        assertEquals("someLong", constructors[0].parameters[1].simpleName.toString())
+        assertEquals("someInt", constructors[1].parameters[0].simpleName.toString())
+        assertEquals("someLong", constructors[1].parameters[1].simpleName.toString())
+        assertEquals("someString", constructors[1].parameters[2].simpleName.toString())
+    }
 }

plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/KotlinKaptContextTestGenerated.java

@@ -36,6 +36,12 @@ public class KotlinKaptContextTestGenerated extends AbstractKotlinKaptContextTes
         KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("plugins/kapt3/testData/kotlinRunner"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
     }
 
+    @TestMetadata("Overloads.kt")
+    public void testOverloads() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/kotlinRunner/Overloads.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("Simple.kt")
     public void testSimple() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/kotlinRunner/Simple.kt");

plugins/kapt3/testData/converter/jvmOverloads.kt

@@ -0,0 +1,17 @@
+class State @JvmOverloads constructor(
+        val someInt: Int,
+        val someLong: Long,
+        val someString: String = ""
+)
+
+class State2 @JvmOverloads constructor(
+        @JvmField val someInt: Int,
+        @JvmField val someLong: Long = 2,
+        @JvmField val someString: String = ""
+) {
+    @JvmOverloads
+    fun test(someInt: Int, someLong: Long = 1, someString: String = "A"): Int = 5
+
+    fun someMethod(str: String) {}
+    fun methodWithoutArgs() {}
+}

plugins/kapt3/testData/converter/jvmOverloads.txt

@@ -0,0 +1,64 @@
+public final class State {
+    private final int someInt = 0;
+    private final long someLong = 0L;
+    private final java.lang.String someString = null;
+
+    public final int getSomeInt() {
+        return 0;
+    }
+
+    public final long getSomeLong() {
+        return 0L;
+    }
+
+    public final java.lang.String getSomeString() {
+        return null;
+    }
+
+    public State(int someInt, long someLong, java.lang.String someString) {
+        super();
+    }
+
+    public State(int someInt, long someLong) {
+        super();
+    }
+}
+
+////////////////////
+
+
+public final class State2 {
+    public final int someInt = 0;
+    public final long someLong = 0L;
+    public final java.lang.String someString = null;
+
+    public final int test(int someInt, long someLong, java.lang.String someString) {
+        return 0;
+    }
+
+    public final int test(int someInt, long someLong) {
+        return 0;
+    }
+
+    public final int test(int someInt) {
+        return 0;
+    }
+
+    public final void someMethod(java.lang.String str) {
+    }
+
+    public final void methodWithoutArgs() {
+    }
+
+    public State2(int someInt, long someLong, java.lang.String someString) {
+        super();
+    }
+
+    public State2(int someInt, long someLong) {
+        super();
+    }
+
+    public State2(int someInt) {
+        super();
+    }
+}

plugins/kapt3/testData/converter/modifiers.txt

@@ -143,7 +143,7 @@ public final class Modifiers {
         return null;
     }
 
-    public final java.lang.String overloads(java.lang.String p0) {
+    public final java.lang.String overloads(java.lang.String a) {
         return null;
     }
 

plugins/kapt3/testData/kotlinRunner/Overloads.it.txt

@@ -0,0 +1,42 @@
+package test;
+
+public abstract @interface MyAnnotation {
+}
+
+////////////////////
+
+package test;
+
+@MyAnnotation()
+public final class State {
+    private final int someInt = 0;
+    private final long someLong = 0L;
+    private final java.lang.String someString = null;
+
+    public final int getSomeInt() {
+        return 0;
+    }
+
+    public final long getSomeLong() {
+        return 0L;
+    }
+
+    public final java.lang.String getSomeString() {
+        return null;
+    }
+
+    public State(int someInt, long someLong, java.lang.String someString) {
+        super();
+    }
+
+    public State(int someInt, long someLong) {
+        super();
+    }
+}
+
+////////////////////
+
+package error;
+
+public final class NonExistentClass {
+}

plugins/kapt3/testData/kotlinRunner/Overloads.kt

@@ -0,0 +1,10 @@
+package test
+
+internal annotation class MyAnnotation
+
+@MyAnnotation
+internal class State @JvmOverloads constructor(
+        val someInt: Int,
+        val someLong: Long,
+        val someString: String = ""
+)

plugins/kapt3/testData/kotlinRunner/Overloads.txt

@@ -0,0 +1,4 @@
+StateMyAnnotation.java:
+
+package generated;
+class StateMyAnnotation {}