New J2K: consider literal expressions in data class conversion & fix names conflict

#KT-32253 fixed

nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/ConvertToDataClassProcessing.kt

@@ -11,19 +11,15 @@ import com.intellij.psi.search.searches.ReferencesSearch
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget
 import org.jetbrains.kotlin.idea.core.setVisibility
 import org.jetbrains.kotlin.idea.intentions.addUseSiteTarget
-import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
 import org.jetbrains.kotlin.idea.util.CommentSaver
 import org.jetbrains.kotlin.nj2k.NewJ2kConverterContext
 import org.jetbrains.kotlin.nj2k.escaped
-import org.jetbrains.kotlin.nj2k.postProcessing.ElementsBasedPostProcessing
+import org.jetbrains.kotlin.nj2k.postProcessing.*
-import org.jetbrains.kotlin.nj2k.postProcessing.descendantsOfType
-import org.jetbrains.kotlin.nj2k.postProcessing.type
-import org.jetbrains.kotlin.nj2k.postProcessing.unpackedReferenceToProperty
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.asAssignment
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
 import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierTypeOrDefault
-import org.jetbrains.kotlin.types.typeUtil.makeNotNullable
+import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 
 class ConvertToDataClassProcessing : ElementsBasedPostProcessing() {
     private fun KtCallableDeclaration.rename(newName: String) {
@@ -35,43 +31,52 @@ class ConvertToDataClassProcessing : ElementsBasedPostProcessing() {
         setName(escapedName)
     }
 
-    private data class DataClassInfo(
-        val constructorParameter: KtParameter,
-        val property: KtProperty,
-        val initBlockStatement: KtBinaryExpression
-    )
 
-    private fun collectPropertiesData(klass: KtClass): List<DataClassInfo> =
+    private fun collectInitializations(klass: KtClass): List<Initialization<*>> {
-        klass.getAnonymousInitializers()
+        val parametersUsed = mutableSetOf<KtParameter>()
-            .flatMap { (it.body as KtBlockExpression).statements }
+        val propertyUsed = mutableSetOf<KtProperty>()
-            .asSequence()
+        @Suppress("UNCHECKED_CAST") return klass.getAnonymousInitializers()
-            .mapNotNull { it.asAssignment() }
+            .singleOrNull()
-            .mapNotNull { statement ->
+            ?.body?.safeAs<KtBlockExpression>()
-                val property =
+            ?.statements
-                    statement.left
+            ?.asSequence()
-                        ?.unpackedReferenceToProperty()
+            ?.map { statement ->
-                        ?.takeIf { it.containingClass() == klass } ?: return@mapNotNull null
+                val assignment = statement.asAssignment() ?: return@map null
-                if (property.getter != null || property.setter != null) return@mapNotNull null
+                val property = assignment.left
-                if (property.initializer != null) return@mapNotNull null
+                    ?.unpackedReferenceToProperty()
-                val constructorParameter =
+                    ?.takeIf { property ->
-                    ((statement.right as? KtReferenceExpression)
+                        property.containingClass() == klass
-                        ?.references
+                                && property.initializer == null
-                        ?.firstOrNull { it is KtSimpleNameReference }
+                                && property !in propertyUsed
-                        ?.resolve() as? KtParameter)
+                    }
-                        ?.takeIf {
+                    ?: return@map null
-                            it.containingClass() == klass && !it.hasValOrVar()
+                propertyUsed += property
-                        } ?: return@mapNotNull null
-                val constructorParameterType = constructorParameter.type() ?: return@mapNotNull null
-                val propertyType = property.type() ?: return@mapNotNull null
 
-                if (constructorParameterType.makeNotNullable() != propertyType.makeNotNullable()) return@mapNotNull null
+                when (val rightSide = assignment.right) {
-
+                    is KtReferenceExpression -> {
-                DataClassInfo(
+                        val parameter = rightSide
-                    constructorParameter,
+                            .resolve()
-                    property,
+                            ?.safeAs<KtParameter>()
-                    statement
+                            ?.takeIf { parameter ->
-                )
+                                parameter.containingClass() == klass
-            }.toList()
+                                        && !parameter.hasValOrVar()
+                                        && parameter !in parametersUsed
+                            } ?: return@map null
+                        val propertyType = property.type() ?: return@map null
+                        val parameterType = parameter.type() ?: return@map null
+                        if (propertyType != parameterType) return@map null
+                        parametersUsed += parameter
+                        ConstructorParameterInitialization(property, parameter, assignment)
+                    }
+                    is KtConstantExpression, is KtStringTemplateExpression -> {
+                        LiteralInitialization(property, rightSide, assignment)
+                    }
+                    else -> null
+                }
+            }?.takeWhile { it != null }
+            .orEmpty()
+            .toList() as List<Initialization<*>>
+    }
 
     override fun runProcessing(elements: List<PsiElement>, converterContext: NewJ2kConverterContext) {
         for (klass in elements.descendantsOfType<KtClass>()) {
@@ -79,39 +84,78 @@ class ConvertToDataClassProcessing : ElementsBasedPostProcessing() {
         }
     }
 
-    private fun convertClass(klass: KtClass) {
+    private fun ConstructorParameterInitialization.mergePropertyAndConstructorParameter() {
-        val factory = KtPsiFactory(klass)
+        val (property, constructorParameter, _) = this
-        for ((constructorParameter, property, statement) in collectPropertiesData(klass)) {
+        val factory = KtPsiFactory(property)
-            constructorParameter.addBefore(property.valOrVarKeyword, constructorParameter.nameIdentifier!!)
+        constructorParameter.addBefore(property.valOrVarKeyword, constructorParameter.nameIdentifier!!)
-            constructorParameter.addAfter(factory.createWhiteSpace(), constructorParameter.valOrVarKeyword!!)
+        constructorParameter.addAfter(factory.createWhiteSpace(), constructorParameter.valOrVarKeyword!!)
-            constructorParameter.rename(property.name!!)
+        constructorParameter.rename(property.name!!)
-            val propertyCommentSaver = CommentSaver(property, saveLineBreaks = true)
+        val propertyCommentSaver = CommentSaver(property, saveLineBreaks = true)
 
-            constructorParameter.setVisibility(property.visibilityModifierTypeOrDefault())
+        constructorParameter.setVisibility(property.visibilityModifierTypeOrDefault())
-            for (annotationEntry in constructorParameter.annotationEntries) {
+        for (annotationEntry in constructorParameter.annotationEntries) {
-                if (annotationEntry.useSiteTarget == null) {
+            if (annotationEntry.useSiteTarget == null) {
-                    annotationEntry.addUseSiteTarget(AnnotationUseSiteTarget.CONSTRUCTOR_PARAMETER, klass.project)
+                annotationEntry.addUseSiteTarget(AnnotationUseSiteTarget.CONSTRUCTOR_PARAMETER, property.project)
-                }
             }
-
-            for (annotationEntry in property.annotationEntries) {
-                constructorParameter.addAnnotationEntry(annotationEntry).also { entry ->
-                    if (entry.useSiteTarget == null) {
-                        entry.addUseSiteTarget(AnnotationUseSiteTarget.FIELD, klass.project)
-                    }
-                }
-            }
-            property.delete()
-            statement.delete()
-            propertyCommentSaver.restore(constructorParameter, forceAdjustIndent = false)
         }
 
-        for (initBlock in klass.getAnonymousInitializers()) {
+        for (annotationEntry in property.annotationEntries) {
+            constructorParameter.addAnnotationEntry(annotationEntry).also { entry ->
+                if (entry.useSiteTarget == null) {
+                    entry.addUseSiteTarget(AnnotationUseSiteTarget.FIELD, property.project)
+                }
+            }
+        }
+        property.delete()
+        propertyCommentSaver.restore(constructorParameter, forceAdjustIndent = false)
+    }
+
+    private fun KtClass.removeEmptyInitBlocks() {
+        for (initBlock in getAnonymousInitializers()) {
             if ((initBlock.body as KtBlockExpression).statements.isEmpty()) {
                 val commentSaver = CommentSaver(initBlock)
                 initBlock.delete()
-                klass.primaryConstructor?.let { commentSaver.restore(it) }
+                primaryConstructor?.let { commentSaver.restore(it) }
             }
         }
     }
-}
+
+    private fun convertClass(klass: KtClass) {
+        for (initialization in collectInitializations(klass)) {
+            val statementCommentSaver = CommentSaver(initialization.statement, saveLineBreaks = true)
+            val restoreStatementCommentsTarget: KtExpression
+            when (initialization) {
+                is ConstructorParameterInitialization -> {
+                    initialization.mergePropertyAndConstructorParameter()
+                    restoreStatementCommentsTarget = initialization.initializer
+                }
+                is LiteralInitialization -> {
+                    val (property, initializer, _) = initialization
+                    property.initializer = initializer
+                    restoreStatementCommentsTarget = property
+                }
+            }
+            initialization.statement.delete()
+            statementCommentSaver.restore(restoreStatementCommentsTarget, forceAdjustIndent = false)
+        }
+        klass.removeEmptyInitBlocks()
+    }
+}
+
+private sealed class Initialization<I : KtElement> {
+    abstract val property: KtProperty
+    abstract val initializer: I
+    abstract val statement: KtBinaryExpression
+}
+
+private data class ConstructorParameterInitialization(
+    override val property: KtProperty,
+    override val initializer: KtParameter,
+    override val statement: KtBinaryExpression
+) : Initialization<KtParameter>()
+
+private data class LiteralInitialization(
+    override val property: KtProperty,
+    override val initializer: KtExpression,
+    override val statement: KtBinaryExpression
+) : Initialization<KtExpression>()

nj2k/testData/newJ2k/comments/commentsForConstructors.kt

@@ -1,5 +1,6 @@
-internal class A @JvmOverloads constructor(p: Int = 1) {
+internal class A // end of primary constructor body
-    private val v: Int
+@JvmOverloads constructor(p: Int = 1) {
+    private val v = 1
 
     // this is a secondary constructor 2
     constructor(s: String) : this(s.length) {} // end of secondary constructor 2 body
@@ -8,9 +9,6 @@ internal class A @JvmOverloads constructor(p: Int = 1) {
 
 
     // this is a primary constructor
-    init {
-        v = 1
-    } // end of primary constructor body
 
 
     // end of secondary constructor 1 body
@@ -24,18 +22,15 @@ internal class B // end of constructor body
 }
 
 internal class CtorComment {
-    var myA: String
+    var myA = "a"
-
+/*
-    /*
      * The magic of comments
      */
     // single line magic comments
-    init {
+
-        myA = "a"
-    }
 }
 
 internal class CtorComment2 /*
      * The magic of comments
      */
-// single line magic comments
+// single line magic comments

nj2k/testData/newJ2k/constructors/allCallsPrimary2.kt

@@ -1,6 +1,6 @@
 internal class C(val myArg1: Int) {
-    var myArg2: Int
+    var myArg2 = 0
-    var myArg3: Int
+    var myArg3 = 0
 
     constructor(arg1: Int, arg2: Int, arg3: Int) : this(arg1) {
         myArg2 = arg2
@@ -12,10 +12,6 @@ internal class C(val myArg1: Int) {
         myArg3 = 0
     }
 
-    init {
-        myArg2 = 0
-        myArg3 = 0
-    }
 }
 
 object User {

nj2k/testData/newJ2k/constructors/customerBuilder.kt

@@ -1,12 +1,16 @@
 package org.test.customer
 
-internal class Customer(val firstName: String, val lastName: String) {
+internal class Customer(first: String, last: String) {
+    val firstName: String
+    val lastName: String
 
     private fun doSmthBefore() {}
     private fun doSmthAfter() {}
 
     init {
         doSmthBefore()
+        firstName = first
+        lastName = last
         doSmthAfter()
     }
 }
@@ -31,7 +35,10 @@ internal class CustomerBuilder {
 
 object User {
     fun main() {
-        val customer = CustomerBuilder().WithFirstName("Homer").WithLastName("Simpson").Build()
+        val customer = CustomerBuilder()
+                .WithFirstName("Homer")
+                .WithLastName("Simpson")
+                .Build()
         println(customer.firstName)
         println(customer.lastName)
     }

nj2k/testData/newJ2k/constructors/fieldsInitializedFromParams5.kt

@@ -1,8 +1,7 @@
 internal class C(p: Int) {
-    var p: Int
+    var p = 0
 
     init {
-        this.p = 0
         if (p > 0) {
             this.p = p
         }

nj2k/testData/newJ2k/detectProperties/DataClassWithMutableField.kt

@@ -1,12 +1,7 @@
-class Test(count: Int) {
+class Test(var count: Int) {
-    var count: Int
-        private set
 
     fun inc() {
         this.count++
     }
 
-    init {
-        this.count = count
-    }
 }

nj2k/testData/newJ2k/detectProperties/DifferentFieldNameAndSuperClass.kt

@@ -6,4 +6,4 @@ open class Base internal constructor(x: Int) {
     }
 }
 
-internal class Derived(b: Base) : Base(b.x)
+internal class Derived(b: Base) : Base(b.x)

nj2k/testData/newJ2k/detectProperties/FieldUsagesInFactoryMethods.kt

@@ -1,6 +1,6 @@
 internal class C(val arg1: Int) {
-    var arg2: Int
+    var arg2 = 0
-    var arg3: Int
+    var arg3 = 0
 
     constructor(arg1: Int, arg2: Int, arg3: Int) : this(arg1) {
         this.arg2 = arg2
@@ -12,9 +12,4 @@ internal class C(val arg1: Int) {
         arg3 = 0
     }
 
-    init {
-        arg2 = 0
-        arg3 = 0
-    }
-
 }

nj2k/testData/newJ2k/detectProperties/kt-32253.java

@@ -0,0 +1,13 @@
+public class TestInitInCtor {
+    private int i;
+    private int j;
+
+    public TestInitInCtor(int i) {
+        this.i = i;
+        j = i;
+    }
+
+    public int foo() {
+        return i + j;
+    }
+}

nj2k/testData/newJ2k/detectProperties/kt-32253.kt

@@ -0,0 +1,10 @@
+class TestInitInCtor(private val i: Int) {
+    private val j: Int
+    fun foo(): Int {
+        return i + j
+    }
+
+    init {
+        j = i
+    }
+}

nj2k/testData/newJ2k/docComments/primaryConstructorDoc.kt

@@ -1,15 +1,13 @@
-internal class CtorComment {
+internal class CtorComment
-    var myA: String
+/**
+ * This constructor is especially useful
+ */
+{
+    var myA = "str"
 
-    /**
-     * This constructor is especially useful
-     */
-    init {
-        myA = "str"
-    }
 }
 
 internal class CtorComment2
 /**
  * This constructor is especially useful
- */
+ */

nj2k/testData/newJ2k/field/needInitializer.kt

@@ -1,22 +1,15 @@
 class Init {
-    var field1: String
+    var field1 = "str"
     var field2: String? = null
-
+    var field3 = 1
-    var field3: Int
     var field4 = 0
 
     init {
-        field1 = "str"
-        field3 = 1
-
         val prop1: String
         prop1 = "aaa"
-
         var prop2: String
-
         val prop3: Int
         prop3 = 1
-
         var prop4: Int
     }
 }

nj2k/testData/newJ2k/field/valOrVar.kt

@@ -4,8 +4,8 @@ internal class A(private val field6: Int, private val field8: Int, a: A) {
     private var field3 = 0
     val field4 = 0
     var field5 = 0
-    private val field7: Int
+    private val field7 = 10
-    private val field9: Int
+    private val field9 = 10
     private var field10 = 0
     private var field11 = 0
     fun foo() {
@@ -13,8 +13,6 @@ internal class A(private val field6: Int, private val field8: Int, a: A) {
     }
 
     init {
-        field7 = 10
-        field9 = 10
         if (field6 > 0) {
             field10 = 10
         }

nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java

@@ -1526,6 +1526,11 @@ public class NewJavaToKotlinConverterSingleFileTestGenerated extends AbstractNew
             runTest("nj2k/testData/newJ2k/detectProperties/kt-31700.java");
         }
 
+        @TestMetadata("kt-32253.java")
+        public void testKt_32253() throws Exception {
+            runTest("nj2k/testData/newJ2k/detectProperties/kt-32253.java");
+        }
+
         @TestMetadata("Overrides.java")
         public void testOverrides() throws Exception {
             runTest("nj2k/testData/newJ2k/detectProperties/Overrides.java");