Don't collapse statements, if nested if have else node

### What's done: * Add new check: don't collapse statements, if nested if have else node * Separate collapse funtion to two ones * Add more tests
saveourtool · Apr 1, 2021 · 355c426 · 355c426
1 parent a556a05
commit 355c426
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 4 deletions.
diff --git a/...-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/CollapseIfStatementsRule.kt b/...-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/CollapseIfStatementsRule.kt
@@ -81,6 +81,10 @@ class CollapseIfStatementsRule(configRules: List<RulesConfig>) : DiktatRule(
     private fun findNestedIf(parentNode: ASTNode) : ASTNode? {
         val parentThenNode = (parentNode.psi as KtIfExpression).then?.node
         val nestedIfNode = parentThenNode?.findChildByType(IF) ?: return null
+        // We won't collapse statements, if nested `if` statement have `else` node
+        (nestedIfNode.psi as KtIfExpression).`else`?.node?.let {
+            return null
+        }
         // We monitor which types of nodes are followed before nested `if`
         // and we allow only a limited number of types to pass through.
         // Otherwise discovered `if` it is not nested
@@ -93,6 +97,11 @@ class CollapseIfStatementsRule(configRules: List<RulesConfig>) : DiktatRule(
     }
 
     private fun collapse(parentNode : ASTNode, nestedNode : ASTNode) {
+        collapseConditions(parentNode, nestedNode)
+        collapseThenBlocks(parentNode, nestedNode)
+    }
+
+    private fun collapseConditions(parentNode : ASTNode, nestedNode : ASTNode) {
         // Merge parent and nested conditions
         val parentCondition = (parentNode.psi as KtIfExpression).condition?.text
         val nestedCondition = (nestedNode.psi as KtIfExpression).condition
@@ -123,6 +132,9 @@ class CollapseIfStatementsRule(configRules: List<RulesConfig>) : DiktatRule(
                 shift--
             }
         }
+    }
+
+    private fun collapseThenBlocks(parentNode : ASTNode, nestedNode : ASTNode) {
         // Merge parent and nested `THEN` blocks
         val nestedThenNode = (nestedNode.psi as KtIfExpression).then
         val nestedThenText = (nestedThenNode as KtBlockExpression).statements.joinToString("\n") { it.text }

diff --git a/...ules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/CollapseIfStatementsRuleWarnTest.kt b/...ules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/CollapseIfStatementsRuleWarnTest.kt
@@ -117,6 +117,51 @@ class CollapseIfStatementsRuleWarnTest : LintTestBase(::CollapseIfStatementsRule
         )
     }
 
+    // TODO: should such statements be collapsed in some manner (?), guess not
+    @Test
+    @Tag(WarningNames.COLLAPSE_IF_STATEMENTS)
+    fun `not nested if 2`() {
+        lintMethod(
+            """
+            |fun foo () {
+            |     if (cond1) {
+            |         if (cond2) {
+            |             firstAction()
+            |             secondAction()
+            |         } else {
+            |             firstAction()
+            |         }
+            |     } else {
+            |         secondAction()
+            |     }
+            |} 
+            """.trimMargin(),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.COLLAPSE_IF_STATEMENTS)
+    fun `not nested if 3`() {
+        lintMethod(
+            """
+            |fun foo () {
+            |     if (cond1) {
+            |         if (cond2) {
+            |             firstAction()
+            |             secondAction()
+            |         } else if (cond3) {
+            |             firstAction()
+            |         } else {
+            |             val a = 5
+            |         }
+            |     } else {
+            |         secondAction()
+            |     }
+            |} 
+            """.trimMargin()
+        )
+    }
+
     @Test
     @Tag(WarningNames.COLLAPSE_IF_STATEMENTS)
     fun `three if statements`() {

diff --git a/diktat-rules/src/test/resources/test/paragraph3/collapse_if/CollapseIfStatementsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/collapse_if/CollapseIfStatementsExpected.kt
@@ -112,3 +112,31 @@ fun foo() {
         someAction()
     }
 }
+
+fun foo() {
+    if (cond1) {
+        if (cond2) {
+            firstAction()
+            secondAction()
+        } else {
+            firstAction()
+        }
+    } else {
+        secondAction()
+    }
+}
+
+fun foo () {
+    if (cond1) {
+        if (cond2) {
+            firstAction()
+            secondAction()
+        } else if (cond3) {
+            firstAction()
+        } else {
+            val a = 5
+        }
+    } else {
+        secondAction()
+    }
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/collapse_if/CollapseIfStatementsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/collapse_if/CollapseIfStatementsTest.kt
@@ -150,3 +150,31 @@ fun foo() {
         }
     }
 }
+
+fun foo() {
+    if (cond1) {
+        if (cond2) {
+            firstAction()
+            secondAction()
+        } else {
+            firstAction()
+        }
+    } else {
+        secondAction()
+    }
+}
+
+fun foo () {
+    if (cond1) {
+        if (cond2) {
+            firstAction()
+            secondAction()
+        } else if (cond3) {
+            firstAction()
+        } else {
+            val a = 5
+        }
+    } else {
+        secondAction()
+    }
+}
diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example3Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example3Expected.kt
@@ -17,10 +17,8 @@ class HttpClient(var name: String) {
 
 class Example {
     fun foo() {
-        if (condition1) {
-            if (condition2) {
-                bar()
-            }
+        if (condition1 && condition2) {
+            bar()
         }
 
         if (condition3) {