Repository: analysis-dev/diktat (kotlin)

Issue:
SingleConstructorRule: incorrect movement of expressions in init block
Here is the code:
```
class Test {
    constructor() {
        str = "ABC"
        println(str)
        str = str.reversed()
        println(str)
    }
    var str: String
}
```
After diktat fix we will get this code:
```
class Test {
    init {
        println(str)
        println(str)
        str = "ABC"
        str = str.reversed()
    }
    var str: String
}
```
which is obviously wrong.

Pull request description:
## SingleConstructorRule

This pull request closes #1132 

## Actions checklist
* [ ] Fixed bug
* [ ] Added test

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
index 9df0499c23..6791bc0f39 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
@@ -78,7 +78,10 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
      * - Create init block with other statements from the secondary constructor, including initialization of properties that require local variables or complex calls.
      * - Finally, remove the secondary constructor.
      */
-    @Suppress("GENERIC_VARIABLE_WRONG_DECLARATION")
+    @Suppress(
+        "GENERIC_VARIABLE_WRONG_DECLARATION",
+        "TOO_LONG_FUNCTION"
+    )
     private fun convertConstructorToPrimary(classNode: ASTNode, secondaryCtor: ASTNode) {
         val secondaryCtorArguments = (secondaryCtor.psi as KtSecondaryConstructor).valueParameters
 
@@ -122,7 +125,15 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             }
             .distinct()
 
-        classNode.convertSecondaryConstructorToPrimary(secondaryCtor, declarationsAssignedInCtor, nonTrivialAssignments, otherStatements, comments)
+        // future init body
+        val expressions = (secondaryCtor.psi as KtSecondaryConstructor)
+            .bodyBlockExpression
+            ?.statements
+            ?.map { it.text }
+            ?.filter { expr -> expr in otherStatements.map { it.text } || expr in nonTrivialAssignments.keys.map { it.text } }
+            ?: emptyList()
+
+        classNode.convertSecondaryConstructorToPrimary(secondaryCtor, declarationsAssignedInCtor, nonTrivialAssignments, otherStatements, comments, expressions)
     }
 
     @Suppress("UnsafeCallOnNullableType")
@@ -152,14 +163,17 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
     @Suppress(
         "NestedBlockDepth",
         "GENERIC_VARIABLE_WRONG_DECLARATION",
-        "TOO_LONG_FUNCTION"
+        "TOO_LONG_FUNCTION",
+        "TOO_MANY_PARAMETERS",
+        "LongParameterList",
     )
     private fun ASTNode.convertSecondaryConstructorToPrimary(
         secondaryCtor: ASTNode,
         declarationsAssignedInCtor: List<KtProperty>,
         nonTrivialAssignments: Map<KtBinaryExpression, KtNameReferenceExpression>,
         otherStatements: List<KtExpression>,
-        comments: Map<String, ASTNode?>?
+        comments: Map<String, ASTNode?>?,
+        initBody: List<String>
     ) {
         require(elementType == CLASS)
 
@@ -169,10 +183,6 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
 
         val primaryCtorNode = createPrimaryCtor(secondaryCtor, declarationsAssignedInCtor, nonTrivialSecondaryCtorParameters)
 
-        val initBody: MutableList<String> = mutableListOf()
-        initBody.addAll(otherStatements.map { it.text })
-        initBody.addAll(nonTrivialAssignments.keys.map { it.text })
-
         val newArgumentListOfSecondaryCtor: List<KtParameter> = (secondaryCtor.psi as KtSecondaryConstructor)
             .valueParameters
             .filter { arg -> arg.name !in nonTrivialSecondaryCtorParameters.map { it.name } }  // get rid of ctor arguments
@@ -191,9 +201,11 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             }
         }
 
+        // adding comments to init body
+        val initBodyWithComments = initBody.toMutableList()
         comments?.forEach { (comment, nextExpression) ->
-            if (initBody.indexOf(nextExpression?.text) != -1) {
-                initBody.add(initBody.indexOf(nextExpression?.text), comment)
+            if (initBodyWithComments.indexOf(nextExpression?.text) != -1) {
+                initBodyWithComments.add(initBodyWithComments.indexOf(nextExpression?.text), comment)
             }
         }
 
@@ -201,7 +213,7 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             findChildByType(CLASS_BODY)?.run {
                 val classInitializer = kotlinParser.createNodeForInit(
                     """|init {
-                       |    ${initBody.joinToString("\n")}
+                       |    ${initBodyWithComments.joinToString("\n")}
                        |}
                     """.trimMargin())
                 addChild(classInitializer, secondaryCtor)


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
index f956271e02..9616300dea 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
@@ -49,4 +49,10 @@ class SingleConstructorRuleFixTest : FixTestBase("test/chapter6/classes", ::Sing
     fun `should not replace constructor with init block`() {
         fixAndCompare("ConstructorWithComplexAssignmentsExpected.kt", "ConstructorWithComplexAssignmentsTest.kt")
     }
+
+    @Test
+    @Tag(WarningNames.SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY)
+    fun `should keep expression order`() {
+        fixAndCompare("ConstructorShouldKeepExpressionsOrderExpected.kt", "ConstructorShouldKeepExpressionsOrderTest.kt")
+    }
 }
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderExpected.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderExpected.kt
new file mode 100644
index 0000000000..a1bdf60eea
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderExpected.kt
@@ -0,0 +1,11 @@
+package test.chapter6.classes
+
+class Test (){
+    init {
+    str = "ABC"
+println(str)
+str = str.reversed()
+println(str)
+}
+    var str: String
+}
\ No newline at end of file
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderTest.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderTest.kt
new file mode 100644
index 0000000000..4936abb189
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderTest.kt
@@ -0,0 +1,11 @@
+package test.chapter6.classes
+
+class Test {
+    constructor() {
+        str = "ABC"
+        println(str)
+        str = str.reversed()
+        println(str)
+    }
+    var str: String
+}
\ No newline at end of file