J2K: default parameter values generated for secondary constructors too

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

@@ -30,14 +30,23 @@ class ConstructorConverter(
         private val fieldCorrections: Map<PsiField, FieldCorrectionInfo>
 ) {
     private val constructors = psiClass.getConstructors()
-    private val constructorsToDrop = HashSet<PsiMethod>()
+
-    private val lastParamDefaults = ArrayList<PsiExpression>() // defaults for a few last parameters of primary constructor in reverse order
+    private val toTargetConstructorMap = buildToTargetConstructorMap()
+
     private val primaryConstructor: PsiMethod? = when (constructors.size()) {
         0 -> null
         1 -> constructors.single()
         else -> choosePrimaryConstructor()
     }
 
+    private fun choosePrimaryConstructor(): PsiMethod? {
+        val candidates = constructors.filter { it !in toTargetConstructorMap }
+        if (candidates.size() != 1) return null // there should be only one constructor which does not call other constructor
+        val primary = candidates.single()
+        if (toTargetConstructorMap.values().any() { it.constructor != primary }) return null // all other constructors call our candidate (directly or indirectly)
+        return primary
+    }
+
     private class TargetConstructorInfo(
             /**
              * Target constructor (one which is finally invoked by this one)
@@ -48,17 +57,11 @@ class ConstructorConverter(
              */
             val parameterDefaults: List<PsiExpression>?)
 
-    private fun choosePrimaryConstructor(): PsiMethod? {
+    private val constructorsToDrop = HashSet<PsiMethod>()
-        val toTargetConstructorMap = buildToTargetConstructorMap()
+    private val lastParamDefaults = HashMap<PsiMethod, ArrayList<PsiExpression>>() // defaults for a few last parameters in reverse order
 
-        val candidates = constructors.filter { it !in toTargetConstructorMap }
+    init {
-        if (candidates.size() != 1) return null // there should be only one constructor which does not call other constructor
+        dropConstructorsForDefaultValues()
-        val primary = candidates.single()
-        if (toTargetConstructorMap.values().any() { it.constructor != primary }) return null // all other constructors call our candidate (directly or indirectly)
-
-        dropConstructorsForDefaultValues(primary, toTargetConstructorMap)
-
-        return primary
     }
 
     private fun buildToTargetConstructorMap(): Map<PsiMethod, TargetConstructorInfo> {
@@ -118,27 +121,35 @@ class ConstructorConverter(
         return args.drop(parameters.size())
     }
 
-    private fun dropConstructorsForDefaultValues(primary: PsiMethod, toTargetConstructorMap: Map<PsiMethod, TargetConstructorInfo>) {
+    private fun dropConstructorsForDefaultValues() {
         val dropCandidates = toTargetConstructorMap
-                .filter { it.value.parameterDefaults != null }
+                .filter {
+                    it.value.parameterDefaults != null
+                        && it.key.accessModifier() == it.value.constructor.accessModifier()
+                        && it.key.getModifierList().getAnnotations().isEmpty() /* do not drop constructors with annotations */
+                }
                 .map { it.key }
-                .filter { it.accessModifier() == primary.accessModifier() && it.getModifierList().getAnnotations().isEmpty() /* do not drop constructors with annotations */ }
                 .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 targetInfo = toTargetConstructorMap[constructor]!!
-            val defaults = toTargetConstructorMap[constructor]!!.parameterDefaults!!
+            val targetConstructor = targetInfo.constructor
-            assert(defaults.size() == primaryParamCount - paramCount)
+            val targetParamCount = targetConstructor.getParameterList().getParametersCount()
+            assert(paramCount < targetParamCount)
+            val defaults = targetInfo.parameterDefaults!!
+            assert(defaults.size() == targetParamCount - paramCount)
+
+            val targetDefaults = lastParamDefaults.getOrPut(targetConstructor, { ArrayList() })
 
             for (i in defaults.indices) {
                 val default = defaults[defaults.size() - i - 1]
-                if (i < lastParamDefaults.size()) { // default for this parameter has already been assigned
+                if (i < targetDefaults.size()) { // default for this parameter has already been assigned
-                    if (lastParamDefaults[i].getText() != default.getText()) continue@DropCandidatesLoop
+                    if (targetDefaults[i].getText() != default.getText()) continue@DropCandidatesLoop
                 }
                 else {
-                    lastParamDefaults.add(default)
+                    targetDefaults.add(default)
                 }
             }
 
@@ -160,7 +171,8 @@ class ConstructorConverter(
         else {
             if (constructor in constructorsToDrop) return null
 
-            val params = converter.convertParameterList(constructor.getParameterList())
+            val params = convertParameterList(constructor, converter,
+                                              { parameter, default -> converter.convertParameter(parameter, defaultValue = default) })
 
             val thisOrSuper = findThisOrSuperCall(constructor)
             val thisOrSuperDeferred = if (thisOrSuper != null)
@@ -276,33 +288,51 @@ class ConstructorConverter(
             baseClassParams = emptyList()
         }
 
-        val parameterList = ParameterList(params.indices.map { i ->
+        val parameterList = convertParameterList(primaryConstructor,
+                                                 correctedConverter,
+                                                 { parameter, default ->
+                                                     if (!parameterToField.containsKey(parameter)) {
+                                                         correctedConverter.convertParameter(parameter, defaultValue = default)
+                                                     }
+                                                     else {
+                                                         val (field, type) = parameterToField[parameter]!!
+                                                         val fieldCorrection = fieldCorrections[field]
+                                                         val name = fieldCorrection?.identifier ?: field.declarationIdentifier()
+                                                         val accessModifiers = if (fieldCorrection != null)
+                                                             Modifiers(listOf()).with(fieldCorrection.access).assignNoPrototype()
+                                                         else
+                                                             converter.convertModifiers(field).filter { it in ACCESS_MODIFIERS }
+                                                         Parameter(name,
+                                                                   type,
+                                                                   if (isVal(converter.referenceSearcher, field)) Parameter.VarValModifier.Val else Parameter.VarValModifier.Var,
+                                                                   converter.convertAnnotations(parameter) + converter.convertAnnotations(field),
+                                                                   accessModifiers,
+                                                                   default).assignPrototypes(listOf(parameter, field), CommentsAndSpacesInheritance(blankLinesBefore = false))
+                                                     }
+                                                 },
+                                                 correctCodeConverter = { correct() })
+
+        return PrimaryConstructor(annotations, modifiers, parameterList, converter.deferredElement(bodyGenerator)).assignPrototype(primaryConstructor)
+    }
+
+    private fun convertParameterList(
+            constructor: PsiMethod,
+            converter: Converter,
+            convertParameter: (parameter: PsiParameter, default: DeferredElement<Expression>?) -> Parameter,
+            correctCodeConverter: CodeConverter.() -> CodeConverter = { this }
+    ): ParameterList {
+        val parameterList = constructor.getParameterList()
+        val params = parameterList.getParameters()
+        val defaults = lastParamDefaults[constructor] ?: emptyList<PsiExpression>()
+        return ParameterList(params.indices.map { i ->
             val parameter = params[i]
             val indexFromEnd = params.size() - i - 1
-            val defaultValue = if (indexFromEnd < lastParamDefaults.size())
+            val defaultValue = if (indexFromEnd < defaults.size())
-                correctedConverter.deferredElement { codeConverter -> codeConverter.correct().convertExpression(lastParamDefaults[indexFromEnd], parameter.getType()) }
+                converter.deferredElement { codeConverter -> codeConverter.correctCodeConverter().convertExpression(defaults[indexFromEnd], parameter.getType()) }
             else
                 null
-            if (!parameterToField.containsKey(parameter)) {
+            convertParameter(parameter, defaultValue)
-                correctedConverter.convertParameter(parameter, defaultValue = defaultValue)
+        }).assignPrototype(parameterList)
-            }
-            else {
-                val (field, type) = parameterToField[parameter]!!
-                val fieldCorrection = fieldCorrections[field]
-                val name = fieldCorrection?.identifier ?: field.declarationIdentifier()
-                val accessModifiers = if (fieldCorrection != null)
-                    Modifiers(listOf()).with(fieldCorrection.access).assignNoPrototype()
-                else
-                    converter.convertModifiers(field).filter { it in ACCESS_MODIFIERS }
-                Parameter(name,
-                          type,
-                          if (isVal(converter.referenceSearcher, field)) Parameter.VarValModifier.Val else Parameter.VarValModifier.Var,
-                          converter.convertAnnotations(parameter) + converter.convertAnnotations(field),
-                          accessModifiers,
-                          defaultValue).assignPrototypes(listOf(parameter, field), CommentsAndSpacesInheritance(blankLinesBefore = false))
-            }
-        }).assignPrototype(primaryConstructor.getParameterList())
-        return PrimaryConstructor(annotations, modifiers, parameterList, converter.deferredElement(bodyGenerator)).assignPrototype(primaryConstructor)
     }
 
     private fun findBackingFieldForConstructorParameter(parameter: PsiParameter, constructor: PsiMethod): Pair<PsiField, PsiStatement>? {

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

@@ -507,7 +507,7 @@ class Converter private(
     private fun convertToNotNullableTypes(types: Array<out PsiType?>): List<Type>
             = types.map { typeConverter.convertType(it, Nullability.NotNull) }
 
-    public fun convertParameterList(parameterList: PsiParameterList): ParameterList
+    private fun convertParameterList(parameterList: PsiParameterList): ParameterList
             = ParameterList(parameterList.getParameters().map { convertParameter(it) }).assignPrototype(parameterList)
 
     public fun convertParameter(parameter: PsiParameter,

j2k/testData/fileOrElement/constructors/secondaryConstructorWithDefaultValues.java

@@ -0,0 +1,16 @@
+class A {
+    private String s = "";
+    private int x = 0;
+
+    public A(){
+    }
+
+    public A(int p, String s){
+        this(p, s, 1);
+    }
+
+    public A(int p, String s, int x){
+        this.s = s;
+        this.x = x;
+    }
+}

j2k/testData/fileOrElement/constructors/secondaryConstructorWithDefaultValues.kt

@@ -0,0 +1,14 @@
+// ERROR: Val cannot be reassigned
+// ERROR: Val cannot be reassigned
+class A {
+    private val s = ""
+    private val x = 0
+
+    public constructor() {
+    }
+
+    overloads public constructor(p: Int, s: String, x: Int = 1) {
+        this.s = s
+        this.x = x
+    }
+}

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java

@@ -1307,6 +1307,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("secondaryConstructorWithDefaultValues.java")
+        public void testSecondaryConstructorWithDefaultValues() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/constructors/secondaryConstructorWithDefaultValues.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("secondaryConstructorsInInnerClass.java")
         public void testSecondaryConstructorsInInnerClass() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/constructors/secondaryConstructorsInInnerClass.java");

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java

@@ -1307,6 +1307,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("secondaryConstructorWithDefaultValues.java")
+        public void testSecondaryConstructorWithDefaultValues() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/constructors/secondaryConstructorWithDefaultValues.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("secondaryConstructorsInInnerClass.java")
         public void testSecondaryConstructorsInInnerClass() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/constructors/secondaryConstructorsInInnerClass.java");