Bugfix for incorrect movement of expressions in init block (#1133)

### What's done: * Fixed bug * Added a test (#1132)
saveourtool · Dec 10, 2021 · d55ce53 · d55ce53
1 parent 4eaf14f
commit d55ce53
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 11 deletions.
diff --git a/...s/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt b/...s/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,17 +201,19 @@ 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)
             }
         }
 
         if (otherStatements.isNotEmpty() || nonTrivialAssignments.isNotEmpty()) {
             findChildByType(CLASS_BODY)?.run {
                 val classInitializer = kotlinParser.createNodeForInit(
                     """|init {
-                       |    ${initBody.joinToString("\n")}
+                       |    ${initBodyWithComments.joinToString("\n")}
                        |}
                     """.trimMargin())
                 addChild(classInitializer, secondaryCtor)

diff --git a/...at-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt b/...at-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/...src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderExpected.kt b/...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
+}
diff --git a/...les/src/test/resources/test/chapter6/classes/ConstructorShouldKeepExpressionsOrderTest.kt b/...les/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
+}