From 6496d2a63e746e8d728b783c44fdcb21b009cc2c Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 3 Jan 2024 15:44:57 +0530 Subject: [PATCH] Fix by checking the existence of label in previous statements (#6671) --- .../bugs/UnconditionalJumpStatementInLoop.kt | 22 +++- .../UnconditionalJumpStatementInLoopSpec.kt | 102 ++++++++++++++++++ 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoop.kt b/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoop.kt index 0144f5c3b86..47a442dbe20 100644 --- a/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoop.kt +++ b/detekt-rules-errorprone/src/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() + labelExpression == null || labelExpression.getLabelName() != label + } else { + true + } + }.any { it.isJumpStatement() } + } + + else -> { + body.isJumpStatement() + } } } diff --git a/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoopSpec.kt b/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoopSpec.kt index 05e01869a0c..7fe5436b611 100644 --- a/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/UnconditionalJumpStatementInLoopSpec.kt +++ b/detekt-rules-errorprone/src/test/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) + } }