Java to Kotlin converter: refactored code to perform primary constructor detection only once

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

@@ -24,18 +24,62 @@ import java.util.HashMap
 import java.util.ArrayList
 import java.util.HashSet
 
-class ConstructorConverter(private val converter: Converter) {
+class ConstructorConverter(private val psiClass: PsiClass, private val converter: Converter) {
     private val typeConverter = converter.typeConverter
 
     private val tempValName: String = "__"
     private fun tempValIdentifier(): Identifier = Identifier(tempValName, false).assignNoPrototype()
 
+    private val className = psiClass.getName()!!
+    private val constructors = psiClass.getConstructors()
+    private val primaryConstructor: PsiMethod? = run {
+        when (constructors.size) {
+            0 -> null
+
+            1 -> constructors.single()
+
+            else -> {
+                val toTargetConstructorMap = HashMap<PsiMethod, PsiMethod>()
+                for (constructor in constructors) {
+                    val firstStatement = constructor.getBody()?.getStatements()?.firstOrNull()
+                    val refExpr = ((firstStatement as? PsiExpressionStatement)
+                            ?.getExpression() as? PsiMethodCallExpression)
+                            ?.getMethodExpression()
+                    if (refExpr != null && refExpr.getCanonicalText() == "this") {
+                        val target = refExpr.resolve() as? PsiMethod
+                        if (target != null && target.isConstructor()) {
+                            val finalTarget = toTargetConstructorMap[target] ?: target!!/*TODO: see KT-5335*/
+                            toTargetConstructorMap[constructor] = finalTarget
+                            for (entry in toTargetConstructorMap.entrySet()) {
+                                if (entry.getValue() == constructor) {
+                                    entry.setValue(finalTarget)
+                                }
+                            }
+                        }
+                    }
+                }
+
+                val candidates = constructors.filter { it !in toTargetConstructorMap }
+                if (candidates.size == 1) { // there should be only one constructor which does not call other constructor
+                    val candidate = candidates.single()
+                    if (toTargetConstructorMap.values().all { it == candidate } /* all other constructors call our candidate (directly or indirectly)*/)
+                        candidate
+                    else
+                        null
+                }
+                else {
+                    null
+                }
+            }
+        }
+    }
+
     public fun convertConstructor(constructor: PsiMethod,
                                   annotations: Annotations,
                                   modifiers: Modifiers,
                                   membersToRemove: MutableSet<PsiMember>,
                                   postProcessBody: (Block) -> Block): Member {
-        if (constructor.isPrimaryConstructor()) {
+        if (constructor == primaryConstructor) {
             return convertPrimaryConstructor(constructor, annotations, modifiers, membersToRemove, postProcessBody)
         }
         else {
@@ -165,17 +209,17 @@ class ConstructorConverter(private val converter: Converter) {
         return null
     }
 
-    public fun postProcessConstructors(classBody: ClassBody, psiClass: PsiClass): ClassBody {
+    public fun postProcessConstructors(classBody: ClassBody): ClassBody {
-        if (psiClass.getPrimaryConstructor() == null && psiClass.getConstructors().size > 1) {
+        if (primaryConstructor == null && constructors.size > 1) {
-            return generateArtificialPrimaryConstructor(psiClass.getName()!!, classBody)
+            return generateArtificialPrimaryConstructor(classBody)
         }
         else {
-            val updatedFunctions = replaceConstructorCallsInFactoryFunctions(classBody.factoryFunctions, psiClass.getName()!!)
+            val updatedFunctions = replaceConstructorCallsInFactoryFunctions(classBody.factoryFunctions)
             return ClassBody(classBody.primaryConstructorSignature, classBody.members, classBody.classObjectMembers, updatedFunctions, classBody.lBrace, classBody.rBrace)
         }
     }
 
-    private fun generateArtificialPrimaryConstructor(className: String, classBody: ClassBody): ClassBody {
+    private fun generateArtificialPrimaryConstructor(classBody: ClassBody): ClassBody {
         assert(classBody.primaryConstructorSignature == null)
 
         val fieldsToInitialize = classBody.members.filterIsInstance(javaClass<Field>()).filter { it.isVal }
@@ -183,8 +227,8 @@ class ConstructorConverter(private val converter: Converter) {
         for (function in classBody.factoryFunctions) {
             val body = function.body!!
             // 2 cases: secondary constructor either calls another constructor or does not call any
-            val newStatements = replaceConstructorCallInFactoryFunction(body, className) ?:
+            val newStatements = replaceConstructorCallInFactoryFunction(body) ?:
-                    insertCallToArtificialPrimary(body, className, fieldsToInitialize)
+                    insertCallToArtificialPrimary(body, fieldsToInitialize)
             val newBody = Block(newStatements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype()
             updatedFactoryFunctions.add(function.withBody(newBody))
         }
@@ -203,10 +247,10 @@ class ConstructorConverter(private val converter: Converter) {
         return ClassBody(constructorSignature, updatedMembers, classBody.classObjectMembers, updatedFactoryFunctions, classBody.lBrace, classBody.rBrace)
     }
 
-    private fun replaceConstructorCallsInFactoryFunctions(functions: List<FactoryFunction>, className: String): List<FactoryFunction> {
+    private fun replaceConstructorCallsInFactoryFunctions(functions: List<FactoryFunction>): List<FactoryFunction> {
         return functions.map { function ->
             val body = function.body!!
-            val statements = replaceConstructorCallInFactoryFunction(body, className)
+            val statements = replaceConstructorCallInFactoryFunction(body)
             if (statements != null) {
                 function.withBody(Block(statements, body.lBrace, body.rBrace).assignPrototypesFrom(body))
             }
@@ -216,7 +260,7 @@ class ConstructorConverter(private val converter: Converter) {
         }
     }
 
-    private fun replaceConstructorCallInFactoryFunction(body: Block, className: String): List<Statement>? {
+    private fun replaceConstructorCallInFactoryFunction(body: Block): List<Statement>? {
         val statements = ArrayList(body.statements)
 
         // searching for other constructor call in form "this(...)"
@@ -241,7 +285,7 @@ class ConstructorConverter(private val converter: Converter) {
         return null
     }
 
-    private fun insertCallToArtificialPrimary(body: Block, className: String, fieldsToInitialize: Collection<Field>): List<Statement> {
+    private fun insertCallToArtificialPrimary(body: Block, fieldsToInitialize: Collection<Field>): List<Statement> {
         val initializers = HashMap<Field, Expression?>()
         for (field in fieldsToInitialize) {
             initializers.put(field, getDefaultInitializer(field))
@@ -287,50 +331,4 @@ class ConstructorConverter(private val converter: Converter) {
         }
         return statements
     }
-
-    private fun PsiMethod.isPrimaryConstructor(): Boolean {
-        if (!isConstructor()) return false
-        val parent = getParent()
-        if (parent !is PsiClass) return false
-        return parent.getPrimaryConstructor() == this
-    }
-
-    private fun PsiClass.getPrimaryConstructor(): PsiMethod? {
-        val constructors = getConstructors()
-        when (constructors.size) {
-            0 -> return null
-
-            1 -> return constructors.single()
-
-            else -> {
-                val toTargetConstructorMap = HashMap<PsiMethod, PsiMethod>()
-                for (constructor in constructors) {
-                    val firstStatement = constructor.getBody()?.getStatements()?.firstOrNull()
-                    val refExpr = ((firstStatement as? PsiExpressionStatement)
-                            ?.getExpression() as? PsiMethodCallExpression)
-                            ?.getMethodExpression()
-                    if (refExpr != null && refExpr.getCanonicalText() == "this") {
-                        val target = refExpr.resolve() as? PsiMethod
-                        if (target != null && target.isConstructor()) {
-                            val finalTarget = toTargetConstructorMap[target] ?: target!!/*TODO: see KT-5335*/
-                            toTargetConstructorMap[constructor] = finalTarget
-                            for (entry in toTargetConstructorMap.entrySet()) {
-                                if (entry.getValue() == constructor) {
-                                    entry.setValue(finalTarget)
-                                }
-                            }
-                        }
-                    }
-                }
-
-                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 candidate = candidates.single()
-                return if (toTargetConstructorMap.values().all { it == candidate } /* all other constructors call our candidate (directly or indirectly)*/)
-                    candidate
-                else
-                    null
-            }
-        }
-    }
 }

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

@@ -40,7 +40,6 @@ public class Converter private(val project: Project, val settings: ConverterSett
     val typeConverter: TypeConverter = state.typeConverter
     val methodReturnType: PsiType? = state.methodReturnType
 
-    private val constructorConverter = ConstructorConverter(this)
     private val expressionVisitor = state.expressionVisitorFactory(this)
     private val statementVisitor = state.statementVisitorFactory(this)
 
@@ -68,7 +67,7 @@ public class Converter private(val project: Project, val settings: ConverterSett
     private fun convertTopElement(element: PsiElement?): Element? = when (element) {
         is PsiJavaFile -> convertFile(element)
         is PsiClass -> convertClass(element)
-        is PsiMethod -> convertMethod(element, HashSet())
+        is PsiMethod -> convertMethod(element, null, null)
         is PsiField -> convertField(element)
         is PsiStatement -> convertStatement(element)
         is PsiExpression -> convertExpression(element)
@@ -102,15 +101,15 @@ public class Converter private(val project: Project, val settings: ConverterSett
     }
 
     fun convertAnonymousClassBody(anonymousClass: PsiAnonymousClass): AnonymousClassBody {
-        return AnonymousClassBody(convertBody(anonymousClass), anonymousClass.getBaseClassType().resolve()?.isInterface() ?: false).assignPrototype(anonymousClass)
+        return AnonymousClassBody(convertBody(anonymousClass, null), anonymousClass.getBaseClassType().resolve()?.isInterface() ?: false).assignPrototype(anonymousClass)
     }
 
-    private fun convertBody(psiClass: PsiClass): ClassBody {
+    private fun convertBody(psiClass: PsiClass, constructorConverter: ConstructorConverter?): ClassBody {
         val membersToRemove = HashSet<PsiMember>()
         val convertedMembers = LinkedHashMap<PsiMember, Member>()
         for (element in psiClass.getChildren()) {
             if (element is PsiMember) {
-                val converted = convertMember(element, membersToRemove)
+                val converted = convertMember(element, membersToRemove, constructorConverter)
                 if (!converted.isEmpty) {
                     convertedMembers.put(element, converted)
                 }
@@ -166,8 +165,8 @@ public class Converter private(val project: Project, val settings: ConverterSett
         }
     }
 
-    private fun convertMember(member: PsiMember, membersToRemove: MutableSet<PsiMember>): Member = when (member) {
+    private fun convertMember(member: PsiMember, membersToRemove: MutableSet<PsiMember>, constructorConverter: ConstructorConverter?): Member = when (member) {
-        is PsiMethod -> convertMethod(member, membersToRemove)
+        is PsiMethod -> convertMethod(member, membersToRemove, constructorConverter)
         is PsiField -> convertField(member)
         is PsiClass -> convertClass(member)
         is PsiClassInitializer -> convertInitializer(member)
@@ -181,7 +180,10 @@ public class Converter private(val project: Project, val settings: ConverterSett
         val implementsTypes = convertToNotNullableTypes(psiClass.getImplementsListTypes())
         val extendsTypes = convertToNotNullableTypes(psiClass.getExtendsListTypes())
         val name = psiClass.declarationIdentifier()
-        var classBody = convertBody(psiClass)
+
+        val constructorConverter = ConstructorConverter(psiClass, this)
+        var classBody = convertBody(psiClass, constructorConverter)
+        classBody = constructorConverter.postProcessConstructors(classBody)
 
         return when {
             psiClass.isInterface() -> Trait(name, annotations, modifiers, typeParameters, extendsTypes, listOf(), implementsTypes, classBody)
@@ -189,8 +191,6 @@ public class Converter private(val project: Project, val settings: ConverterSett
             psiClass.isEnum() -> Enum(name, annotations, modifiers, typeParameters, listOf(), listOf(), implementsTypes, classBody)
 
             else -> {
-                classBody = constructorConverter.postProcessConstructors(classBody, psiClass)
-
                 val baseClassParams: List<Expression> = run {
                     val superVisitor = SuperVisitor()
                     psiClass.accept(superVisitor)
@@ -266,11 +266,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>): Member {
+    private fun convertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member {
-        return withMethodReturnType(method.getReturnType()).doConvertMethod(method, membersToRemove).assignPrototype(method)
+        return withMethodReturnType(method.getReturnType()).doConvertMethod(method, membersToRemove, constructorConverter).assignPrototype(method)
     }
 
-    private fun doConvertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>): Member {
+    private fun doConvertMethod(method: PsiMethod, membersToRemove: MutableSet<PsiMember>?, constructorConverter: ConstructorConverter?): Member {
         val returnType = typeConverter.convertMethodReturnType(method)
 
         val annotations = (convertAnnotations(method) + convertThrows(method)).assignNoPrototype()
@@ -297,8 +297,8 @@ public class Converter private(val project: Project, val settings: ConverterSett
             }
         }
 
-        if (method.isConstructor()) {
+        if (method.isConstructor() && constructorConverter != null) {
-            return constructorConverter.convertConstructor(method, annotations, modifiers, membersToRemove, postProcessBody)
+            return constructorConverter.convertConstructor(method, annotations, modifiers, membersToRemove!!, postProcessBody)
         }
         else {
             val isOverride = isOverride(method)