Java to Kotlin converter: constructor overloads to default parameter values conversion

j2k/src/org/jetbrains/jet/j2k/ConstructorConverter.kt

@@ -32,45 +32,121 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
 
     private val className = psiClass.getName()!!
     private val constructors = psiClass.getConstructors()
-    private val primaryConstructor: PsiMethod? = run {
+    private val constructorsToDrop = HashSet<PsiMethod>()
-        when (constructors.size) {
+    private val lastParamDefaults = ArrayList<Expression>() // defaults for a few last parameters of primary constructor in reverse order
-            0 -> null
+    private val primaryConstructor: PsiMethod? = when (constructors.size) {
+        0 -> null
+        1 -> constructors.single()
+        else -> choosePrimaryConstructor()
+    }
 
-            1 -> constructors.single()
+    private class TargetConstructorInfo(
+            /**
+             * Target constructor (one which is finally invoked by this one)
+             */
+            val constructor: PsiMethod,
+            /**
+             * Is not null if this constructor is equivalent to the target constructor with a few last parameters having default values
+             */
+            val parameterDefaults: List<Expression>?)
 
-            else -> {
+    private fun choosePrimaryConstructor(): PsiMethod? {
-                val toTargetConstructorMap = HashMap<PsiMethod, PsiMethod>()
+        val toTargetConstructorMap = HashMap<PsiMethod, TargetConstructorInfo>()
-                for (constructor in constructors) {
+        for (constructor in constructors) {
-                    val firstStatement = constructor.getBody()?.getStatements()?.firstOrNull()
+            val firstStatement = constructor.getBody()?.getStatements()?.firstOrNull()
-                    val refExpr = ((firstStatement as? PsiExpressionStatement)
+            val methodCall = (firstStatement as? PsiExpressionStatement)?.getExpression() as? PsiMethodCallExpression
-                            ?.getExpression() as? PsiMethodCallExpression)
+            if (methodCall != null) {
-                            ?.getMethodExpression()
+                val refExpr = methodCall.getMethodExpression()
-                    if (refExpr != null && refExpr.getCanonicalText() == "this") {
+                if (refExpr.getCanonicalText() == "this") {
-                        val target = refExpr.resolve() as? PsiMethod
+                    val target = refExpr.resolve() as? PsiMethod
-                        if (target != null && target.isConstructor()) {
+                    if (target != null && target.isConstructor()) {
-                            val finalTarget = toTargetConstructorMap[target] ?: target!!/*TODO: see KT-5335*/
+                        var parameterDefaults = calcTargetParameterDefaults(constructor, target, methodCall)
-                            toTargetConstructorMap[constructor] = finalTarget
+
-                            for (entry in toTargetConstructorMap.entrySet()) {
+                        val finalTargetInfo = toTargetConstructorMap[target]
-                                if (entry.getValue() == constructor) {
+                        if (finalTargetInfo != null && parameterDefaults != null) {
-                                    entry.setValue(finalTarget)
+                            parameterDefaults = if (finalTargetInfo.parameterDefaults != null)
-                                }
+                                parameterDefaults!! + finalTargetInfo.parameterDefaults
+                            else
+                                null
+                        }
+                        val finalTarget = finalTargetInfo?.constructor ?: target!! //TODO: see KT-5335
+
+                        toTargetConstructorMap[constructor] = TargetConstructorInfo(finalTarget, parameterDefaults)
+                        for (entry in toTargetConstructorMap.entrySet()) {
+                            if (entry.value.constructor == constructor) {
+                                val newParameterDefaults = if (parameterDefaults != null)
+                                    entry.value.parameterDefaults?.plus(parameterDefaults!!)
+                                else
+                                    null
+                                entry.setValue(TargetConstructorInfo(finalTarget, newParameterDefaults))
                             }
                         }
                     }
                 }
+            }
+        }
 
-                val candidates = constructors.filter { it !in toTargetConstructorMap }
+        val candidates = constructors.filter { it !in toTargetConstructorMap }
-                if (candidates.size == 1) { // there should be only one constructor which does not call other constructor
+        if (candidates.size != 1) return null // there should be only one constructor which does not call other constructor
-                    val candidate = candidates.single()
+        val primary = candidates.single()
-                    if (toTargetConstructorMap.values().all { it == candidate } /* all other constructors call our candidate (directly or indirectly)*/)
+        if (toTargetConstructorMap.values().any() { it.constructor != primary }) return null // all other constructors call our candidate (directly or indirectly)
-                        candidate
+
-                    else
+        dropConstructorsForDefaultValues(primary, toTargetConstructorMap)
-                        null
+
+        return primary
+    }
+
+    private fun calcTargetParameterDefaults(constructor: PsiMethod, target: PsiMethod, targetCall: PsiMethodCallExpression): List<Expression>? {
+        if (constructor.getBody()!!.getStatements().size != 1) return null // constructor's body should consist of only "this(...)"
+        val parameters = constructor.getParameterList().getParameters()
+        val targetParameters = target.getParameterList().getParameters()
+        if (parameters.size >= targetParameters.size) return null
+        val args = targetCall.getArgumentList().getExpressions()
+        if (args.size != targetParameters.size) return null // incorrect code
+
+        for (i in parameters.indices) {
+            val parameter = parameters[i]
+            val targetParameter = targetParameters[i]
+            if (parameter.getName() != targetParameter.getName() || parameter.getType() != targetParameter.getType()) return null
+            val arg = args[i]
+            if (arg !is PsiReferenceExpression || arg.getQualifier() != null) return null
+            if (arg.resolve() != parameter) return null
+        }
+
+        val result = ArrayList<Expression>(args.size - parameters.size)
+        for (i in (parameters.size..args.size-1)) {
+            result.add(converter.convertExpression(args[i]))
+        }
+        return result
+    }
+
+    private fun dropConstructorsForDefaultValues(primary: PsiMethod, toTargetConstructorMap: Map<PsiMethod, TargetConstructorInfo>) {
+        //TODO: should we drop when annotations exist?
+
+        val dropCandidates = toTargetConstructorMap
+                .filter { it.value.parameterDefaults != null }
+                .map { it.key }
+                .filter { it.accessModifier() == primary.accessModifier() }
+                .sortBy { -it.getParameterList().getParametersCount() } // we will try to drop them starting from ones with more parameters
+        val primaryParamCount = primary.getParameterList().getParametersCount()
+        @DropCandidatesLoop
+        for (constructor in dropCandidates) {
+            val paramCount = constructor.getParameterList().getParametersCount()
+            assert(paramCount < primaryParamCount)
+            val defaults = toTargetConstructorMap[constructor]!!.parameterDefaults!!
+            assert(defaults.size == primaryParamCount - paramCount)
+
+            for (i in (0..defaults.size-1)) {
+                val default = defaults[defaults.size - i - 1]
+                if (i < lastParamDefaults.size) { // default for this parameter has already been assigned
+                    if (lastParamDefaults[i].canonicalCode() != default.canonicalCode()) continue@DropCandidatesLoop
                 }
                 else {
-                    null
+                    lastParamDefaults.add(default)
                 }
             }
+
+            constructorsToDrop.add(constructor)
         }
     }
 
@@ -90,11 +166,13 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
                                   annotations: Annotations,
                                   modifiers: Modifiers,
                                   membersToRemove: MutableSet<PsiMember>,
-                                  postProcessBody: (Block) -> Block): Member {
+                                  postProcessBody: (Block) -> Block): Member? {
         if (constructor == primaryConstructor) {
             return convertPrimaryConstructor(constructor, annotations, modifiers, membersToRemove, postProcessBody)
         }
         else {
+            if (constructor in constructorsToDrop) return null
+
             val params = converter.convertParameterList(constructor.getParameterList())
             val bodyConverter = converter.withExpressionVisitor { object : ExpressionVisitor(it, mapOf()/*TODO: see KT-5327*/) {
                 override fun visitReferenceExpression(expression: PsiReferenceExpression) {
@@ -175,9 +253,12 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
             Block.Empty
         }
 
-        val parameterList = ParameterList(params.map { parameter ->
+        val parameterList = ParameterList(params.indices.map { i ->
+            val parameter = params[i]
+            val indexFromEnd = params.size - i - 1
+            val defaultValue = if (indexFromEnd < lastParamDefaults.size) lastParamDefaults[indexFromEnd] else null
             if (!parameterToField.containsKey(parameter)) {
-                converter.convertParameter(parameter)
+                converter.convertParameter(parameter, defaultValue = defaultValue)
             }
             else {
                 val (field, `type`) = parameterToField[parameter]!!
@@ -185,7 +266,8 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
                           `type`,
                           if (isVal(field)) Parameter.VarValModifier.Val else Parameter.VarValModifier.Var,
                           converter.convertAnnotations(parameter) + converter.convertAnnotations(field),
-                          converter.convertModifiers(field).filter { it in ACCESS_MODIFIERS }).assignPrototypes(listOf(parameter, field), CommentsAndSpacesInheritance(blankLinesBefore = false))
+                          converter.convertModifiers(field).filter { it in ACCESS_MODIFIERS },
+                          defaultValue).assignPrototypes(listOf(parameter, field), CommentsAndSpacesInheritance(blankLinesBefore = false))
             }
         }).assignPrototype(constructor.getParameterList())
         return PrimaryConstructor(annotations, modifiers, parameterList, block).assignPrototype(constructor)

j2k/src/org/jetbrains/jet/j2k/Converter.kt

@@ -110,7 +110,7 @@ public class Converter private(val project: Project, val settings: ConverterSett
         for (element in psiClass.getChildren()) {
             if (element is PsiMember) {
                 val converted = convertMember(element, membersToRemove, constructorConverter)
-                if (!converted.isEmpty) {
+                if (converted != null && !converted.isEmpty) {
                     convertedMembers.put(element, converted)
                 }
             }
@@ -172,7 +172,7 @@ public class Converter private(val project: Project, val settings: ConverterSett
         }
     }
 
-    private fun convertMember(member: PsiMember, membersToRemove: MutableSet<PsiMember>, constructorConverter: ConstructorConverter?): Member = when (member) {
+    private fun convertMember(member: PsiMember, membersToRemove: MutableSet<PsiMember>, constructorConverter: ConstructorConverter?): Member? = when (member) {
         is PsiMethod -> convertMethod(member, membersToRemove, constructorConverter)
         is PsiField -> convertField(member)
         is PsiClass -> convertClass(member)
@@ -262,11 +262,11 @@ public class Converter private(val project: Project, val settings: ConverterSett
         return if (convertedType == initializerType) null else convertedType
     }
 
-    private fun convertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member {
+    private fun convertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member? {
-        return withMethodReturnType(method.getReturnType()).doConvertMethod(method, membersToRemove, constructorConverter).assignPrototype(method)
+        return withMethodReturnType(method.getReturnType()).doConvertMethod(method, membersToRemove, constructorConverter)?.assignPrototype(method)
     }
 
-    private fun doConvertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member {
+    private fun doConvertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member? {
         val returnType = typeConverter.convertMethodReturnType(method)
 
         val annotations = (convertAnnotations(method) + convertThrows(method)).assignNoPrototype()
@@ -401,13 +401,14 @@ public class Converter private(val project: Project, val settings: ConverterSett
     fun convertParameter(parameter: PsiParameter,
                          nullability: Nullability = Nullability.Default,
                          varValModifier: Parameter.VarValModifier = Parameter.VarValModifier.None,
-                         modifiers: Modifiers = Modifiers.Empty): Parameter {
+                         modifiers: Modifiers = Modifiers.Empty,
+                         defaultValue: Expression? = null): Parameter {
         var `type` = typeConverter.convertVariableType(parameter)
         when (nullability) {
             Nullability.NotNull -> `type` = `type`.toNotNullType()
             Nullability.Nullable -> `type` = `type`.toNullableType()
         }
-        return Parameter(parameter.declarationIdentifier(), `type`, varValModifier, convertAnnotations(parameter), modifiers).assignPrototype(parameter)
+        return Parameter(parameter.declarationIdentifier(), `type`, varValModifier, convertAnnotations(parameter), modifiers, defaultValue).assignPrototype(parameter)
     }
 
     fun convertExpression(expression: PsiExpression?, expectedType: PsiType?): Expression {

j2k/src/org/jetbrains/jet/j2k/Utils.kt

@@ -126,3 +126,10 @@ fun PsiElement.getContainingConstructor(): PsiMethod? {
 }
 
 fun PsiElement.isConstructor(): Boolean = this is PsiMethod && this.isConstructor()
+
+fun PsiModifierListOwner.accessModifier(): String = when {
+    hasModifierProperty(PsiModifier.PUBLIC) -> PsiModifier.PUBLIC
+    hasModifierProperty(PsiModifier.PRIVATE) -> PsiModifier.PRIVATE
+    hasModifierProperty(PsiModifier.PROTECTED) -> PsiModifier.PROTECTED
+    else -> PsiModifier.PACKAGE_LOCAL
+}

j2k/src/org/jetbrains/jet/j2k/ast/Parameter.kt

@@ -22,7 +22,8 @@ class Parameter(val identifier: Identifier,
                 val `type`: Type,
                 val varVal: Parameter.VarValModifier,
                 val annotations: Annotations,
-                val modifiers: Modifiers) : Element() {
+                val modifiers: Modifiers,
+                val defaultValue: Expression? = null) : Element() {
     public enum class VarValModifier {
         None
         Val
@@ -42,6 +43,10 @@ class Parameter(val identifier: Identifier,
             VarValModifier.Val -> builder.append("val ")
         }
 
-        builder.append(identifier).append(":").append(`type`)
+        builder append identifier append ":" append `type`
+
+        if (defaultValue != null) {
+            builder append " = " append defaultValue
+        }
     }
 }

j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java

@@ -16,14 +16,17 @@
 
 package org.jetbrains.jet.j2k.test;
 
+import junit.framework.Assert;
 import junit.framework.Test;
 import junit.framework.TestSuite;
+
+import java.io.File;
+import java.util.regex.Pattern;
 import org.jetbrains.jet.JetTestUtils;
 import org.jetbrains.jet.test.InnerTestClasses;
 import org.jetbrains.jet.test.TestMetadata;
 
-import java.io.File;
+import org.jetbrains.jet.j2k.test.AbstractJavaToKotlinConverterTest;
-import java.util.regex.Pattern;
 
 /** This class is generated by {@link org.jetbrains.jet.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
 @SuppressWarnings("all")
@@ -873,6 +876,31 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             doTest("j2k/tests/testData/ast/constructors/noPrimary.java");
         }
         
+        @TestMetadata("parameterDefaults1.java")
+        public void testParameterDefaults1() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/parameterDefaults1.java");
+        }
+        
+        @TestMetadata("parameterDefaults2.java")
+        public void testParameterDefaults2() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/parameterDefaults2.java");
+        }
+        
+        @TestMetadata("parameterDefaults3.java")
+        public void testParameterDefaults3() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/parameterDefaults3.java");
+        }
+        
+        @TestMetadata("parameterDefaults4.java")
+        public void testParameterDefaults4() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/parameterDefaults4.java");
+        }
+        
+        @TestMetadata("parameterDefaults5.java")
+        public void testParameterDefaults5() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/parameterDefaults5.java");
+        }
+        
         @TestMetadata("parameterModification.java")
         public void testParameterModification() throws Exception {
             doTest("j2k/tests/testData/ast/constructors/parameterModification.java");

j2k/tests/testData/ast/comments/commentsForConstructors.java

@@ -7,10 +7,15 @@ class A {
         v = 1;
     } // end of primary constructor body
 
-    // this is a secondary constructor
+    // this is a secondary constructor 1
     A() {
         this(1);
-    } // end of secondary constructor body
+    } // end of secondary constructor 1 body
+
+    // this is a secondary constructor 2
+    A(String s) {
+        this(s.length());
+    } // end of secondary constructor 2 body
 }
 
 class B {

j2k/tests/testData/ast/comments/commentsForConstructors.kt

@@ -1,16 +1,17 @@
-// this is a secondary constructor
+// this is a secondary constructor 2
-fun A(): A {
+fun A(s: String): A {
-    return A(1)
+    return A(s.length())
-} // end of secondary constructor body
+} // end of secondary constructor 2 body
 
 class A// this is a primary constructor
-(p: Int) {
+(p: Int = 1) {
     private val v: Int
 
     {
         v = 1
     } // end of primary constructor body
-}
+}// this is a secondary constructor 1
+// end of secondary constructor 1 body
 
 class B// this constructor will disappear
 (private val x: Int) // end of constructor body

j2k/tests/testData/ast/constructors/allCallsPrimary.kt

@@ -1,15 +1,6 @@
 package pack
 
-
+class C(arg1: Int, arg2: Int = 0, arg3: Int = 0)
-fun C(arg1: Int, arg2: Int): C {
-    return C(arg1, arg2, 0)
-}
-
-fun C(arg1: Int): C {
-    return C(arg1, 0, 0)
-}
-
-class C(arg1: Int, arg2: Int, arg3: Int)
 
 public class User {
     class object {

j2k/tests/testData/ast/constructors/parameterDefaults1.java

@@ -0,0 +1,19 @@
+//file
+package pack
+
+class C {
+  C(int a, int b, int c, int d, int e) {
+  }
+
+  C(int a, int b, int c) {
+    this(a, b, c, 0, 0);
+  }
+
+  C(int a) {
+    this(a, 0, 0, 0, 1);
+  }
+
+  C() {
+    this(0, 0, 0, 0, 0);
+  }
+}

j2k/tests/testData/ast/constructors/parameterDefaults1.kt

@@ -0,0 +1,8 @@
+package pack
+
+
+fun C(a: Int): C {
+    return C(a, 0, 0, 0, 1)
+}
+
+class C(a: Int = 0, b: Int = 0, c: Int = 0, d: Int = 0, e: Int = 0)

j2k/tests/testData/ast/constructors/parameterDefaults2.java

@@ -0,0 +1,19 @@
+//file
+package pack
+
+class C {
+  C(int a, int b, int c, int d, int e) {
+  }
+
+  C(int a1, int b1, int c1) {
+    this(a1, b1, c1, 0, 0);
+  }
+
+  C(byte b) {
+    this(b, 0, 0, 0, 0);
+  }
+
+  C() {
+    this(0, 0, 0, 0, 0);
+  }
+}

j2k/tests/testData/ast/constructors/parameterDefaults2.kt

@@ -0,0 +1,12 @@
+package pack
+
+
+fun C(a1: Int, b1: Int, c1: Int): C {
+    return C(a1, b1, c1, 0, 0)
+}
+
+fun C(b: Byte): C {
+    return C(b.toInt(), 0, 0, 0, 0)
+}
+
+class C(a: Int = 0, b: Int = 0, c: Int = 0, d: Int = 0, e: Int = 0)

j2k/tests/testData/ast/constructors/parameterDefaults3.java

@@ -0,0 +1,15 @@
+//file
+package pack
+
+class C {
+  C(int a, int b, int c, int d, int e) {
+  }
+
+  C(int a, int b, int c) {
+    this(b, a, c, 0, 0);
+  }
+
+  C() {
+    this(0, 0, 0, 0, 0);
+  }
+}

j2k/tests/testData/ast/constructors/parameterDefaults3.kt

@@ -0,0 +1,8 @@
+package pack
+
+
+fun C(a: Int, b: Int, c: Int): C {
+    return C(b, a, c, 0, 0)
+}
+
+class C(a: Int = 0, b: Int = 0, c: Int = 0, d: Int = 0, e: Int = 0)

j2k/tests/testData/ast/constructors/parameterDefaults4.java

@@ -0,0 +1,23 @@
+//file
+package pack
+
+class C {
+  C(int a, int b, int c, int d, int e) {
+  }
+
+  C(int a, int b, int c) {
+    this(a, b, c, 4, 5);
+  }
+
+  C(int a) {
+    this(a, 2, 3);
+  }
+
+  C(int a, int b) {
+    this(a, b, 3, 4, 5);
+  }
+
+  C() {
+    this(1);
+  }
+}

j2k/tests/testData/ast/constructors/parameterDefaults4.kt

@@ -0,0 +1,3 @@
+package pack
+
+class C(a: Int = 1, b: Int = 2, c: Int = 3, d: Int = 4, e: Int = 5)

j2k/tests/testData/ast/constructors/parameterDefaults5.java

@@ -0,0 +1,23 @@
+//file
+package pack
+
+class C {
+  C(int a, int b, int c, int d, int e) {
+  }
+
+  C() {
+    this(1);
+  }
+
+  C(int a, int b) {
+    this(a, b, 3, 4, 5);
+  }
+
+  C(int a) {
+    this(a, 2, 3);
+  }
+
+  C(int a, int b, int c) {
+    this(a, b, c, 4, 5);
+  }
+}

j2k/tests/testData/ast/constructors/parameterDefaults5.kt

@@ -0,0 +1,3 @@
+package pack
+
+class C(a: Int = 1, b: Int = 2, c: Int = 3, d: Int = 4, e: Int = 5)

j2k/tests/testData/ast/constructors/parameterModification.kt

@@ -5,11 +5,7 @@ fun C(arg1: Int, arg2: Int): C {
     return __
 }
 
-fun C(arg1: Int): C {
+class C(arg1: Int, arg2: Int = 0, arg3: Int = 0) {
-    return C(arg1, 0, 0)
-}
-
-class C(arg1: Int, arg2: Int, arg3: Int) {
     private val field: Int
 
     {

j2k/tests/testData/ast/constructors/privateConstructors.kt

@@ -1,9 +1,5 @@
-private fun C(arg1: Int, arg2: Int): C {
-    return C(arg1, arg2, 0)
-}
-
 fun C(arg1: Int): C {
     return C(arg1, 0, 0)
 }
 
-class C private(arg1: Int, arg2: Int, arg3: Int)
+class C private(arg1: Int, arg2: Int, arg3: Int = 0)