Fix by checking the existence of label in previous statements (#6671)

detekt · Jan 3, 2024 · 6496d2a · 6496d2a
1 parent 44f22f1
commit 6496d2a
Show file tree

Hide file tree

Showing 2 changed files with 120 additions and 4 deletions.
diff --git a/...rc/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoop.kt b/...rc/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoop.kt
@@ -11,9 +11,11 @@ import org.jetbrains.kotlin.psi.KtBinaryExpression
 import org.jetbrains.kotlin.psi.KtBlockExpression
 import org.jetbrains.kotlin.psi.KtBreakExpression
 import org.jetbrains.kotlin.psi.KtContinueExpression
+import org.jetbrains.kotlin.psi.KtLabeledExpression
 import org.jetbrains.kotlin.psi.KtLoopExpression
 import org.jetbrains.kotlin.psi.KtReturnExpression
 import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
+import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType
 import org.jetbrains.kotlin.psi.psiUtil.siblings
 
 /**
@@ -40,7 +42,7 @@ class UnconditionalJumpStatementInLoop(config: Config = Config.empty) : Rule(con
     )
 
     override fun visitLoopExpression(loopExpression: KtLoopExpression) {
-        if (loopExpression.hasJumpStatements()) {
+        if (loopExpression.hasJumpStatements((loopExpression.parent as? KtLabeledExpression)?.getLabelName())) {
             report(
                 CodeSmell(
                     issue,
@@ -54,11 +56,23 @@ class UnconditionalJumpStatementInLoop(config: Config = Config.empty) : Rule(con
         super.visitLoopExpression(loopExpression)
     }
 
-    private fun KtLoopExpression.hasJumpStatements(): Boolean {
+    private fun KtLoopExpression.hasJumpStatements(label: String?): Boolean {
         val body = this.body ?: return false
         return when (body) {
-            is KtBlockExpression -> body.children.any { it.isJumpStatement() }
-            else -> body.isJumpStatement()
+            is KtBlockExpression -> {
+                body.children.takeWhile {
+                    if (label != null) {
+                        val labelExpression = it.findDescendantOfType<KtContinueExpression>()
+                        labelExpression == null || labelExpression.getLabelName() != label
+                    } else {
+                        true
+                    }
+                }.any { it.isJumpStatement() }
+            }
+
+            else -> {
+                body.isJumpStatement()
+            }
         }
     }
 

diff --git a/...est/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoopSpec.kt b/...est/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoopSpec.kt
@@ -365,4 +365,106 @@ class UnconditionalJumpStatementInLoopSpec {
         """.trimIndent()
         assertThat(subject.compileAndLint(code)).isEmpty()
     }
+
+    // https://github.com/detekt/detekt/issues/6657
+    @Test
+    fun `does not report a break in while when break execution depends on previous expression`() {
+        val code = """
+            fun main() {
+                val lastIndex = 3
+                val cycles = intArrayOf(1, 2, 3, 4)
+                outer@ while (true) {
+                    for (i in lastIndex downTo 0) {
+                        if (cycles[i] != 0) {
+                            continue@outer
+                        }
+                        println(cycles[i])
+                    }
+                    break
+                }
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report a break in while when break execution depends on previous to previous expression`() {
+        val code = """
+            fun main() {
+                val lastIndex = 3
+                val cycles = intArrayOf(1, 2, 3, 4)
+                outer@ while (true) {
+                    for (i in lastIndex downTo 0) {
+                        if (cycles[i] != 0) {
+                            continue@outer
+                        }
+                        println(cycles[i])
+                    }
+                    println("test")
+                    break
+                }
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does report a break in while when break and previous expression doesn't use labels`() {
+        val code = """
+            fun main() {
+                val lastIndex = 3
+                val cycles = intArrayOf(1, 2, 3, 4)
+                outer@ while (true) {
+                    for (i in lastIndex downTo 0) {
+                        if (cycles[i] != 0) {
+                            println("not zero")
+                        }
+                        println(cycles[i])
+                    }
+                    break
+                }
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `does report a break in while when previous statement continue refer to different label`() {
+        val code = """
+            fun main() {
+                val lastIndex = 3
+                val cycles = intArrayOf(1, 2, 3, 4)
+                outer@ while (true) {
+                    break
+                    for (i in lastIndex downTo 0) {
+                        if (cycles[i] != 0) {
+                            continue@outer
+                        }
+                        println(cycles[i])
+                    }
+                }
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `does report a break in while when break is before the rest of the statements`() {
+        val code = """
+            fun main() {
+                val lastIndex = 3
+                val cycles = intArrayOf(1, 2, 3, 4)
+                outer@ while (true) {
+                    inner@ for (i in lastIndex downTo 0) {
+                        if (cycles[i] != 0) {
+                            continue@inner
+                        }
+                        println(cycles[i])
+                    }
+                    break
+                }
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
 }