Report loop problems on main loop keyword.

detekt · May 20, 2023 · 1d532a7 · 1d532a7
1 parent 005ca5a
commit 1d532a7
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 5 deletions.
diff --git a/.../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/LoopWithTooManyJumpStatements.kt b/.../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/LoopWithTooManyJumpStatements.kt
@@ -11,11 +11,17 @@ import io.gitlab.arturbosch.detekt.api.Severity
 import io.gitlab.arturbosch.detekt.api.config
 import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
 import io.gitlab.arturbosch.detekt.api.internal.Configuration
+import org.jetbrains.kotlin.com.intellij.psi.PsiElement
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtBreakExpression
 import org.jetbrains.kotlin.psi.KtContinueExpression
+import org.jetbrains.kotlin.psi.KtDoWhileExpression
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtForExpression
 import org.jetbrains.kotlin.psi.KtLoopExpression
+import org.jetbrains.kotlin.psi.KtPsiUtil
+import org.jetbrains.kotlin.psi.KtWhileExpression
 
 /**
  * Loops which contain multiple `break` or `continue` statements are hard to read and understand.
@@ -48,7 +54,7 @@ class LoopWithTooManyJumpStatements(config: Config = Config.empty) : Rule(config
 
     override fun visitLoopExpression(loopExpression: KtLoopExpression) {
         if (countBreakAndReturnStatements(loopExpression.body) > maxJumpCount) {
-            report(CodeSmell(issue, Entity.from(loopExpression), issue.description))
+            report(CodeSmell(issue, Entity.from(loopExpression.keyword ?: loopExpression), issue.description))
         }
         super.visitLoopExpression(loopExpression)
     }
@@ -71,3 +77,22 @@ class LoopWithTooManyJumpStatements(config: Config = Config.empty) : Rule(config
         return count
     }
 }
+
+/**
+ * For some reason not all keyword properties are exposed on [KtLoopExpression] subclasses, so we have to do it manually.
+ */
+@Suppress("CommentOverPrivateProperty")
+private val KtLoopExpression.keyword: PsiElement?
+    get() =
+        when (this) {
+            is KtForExpression -> this.forKeyword
+            is KtWhileExpression -> this.whileKeyword
+            is KtDoWhileExpression -> this.doKeyword
+            else -> null
+        }
+
+private val KtDoWhileExpression.doKeyword: PsiElement?
+    get() = KtPsiUtil.findChildByType(this, KtTokens.DO_KEYWORD)
+
+private val KtWhileExpression.whileKeyword: PsiElement?
+    get() = KtPsiUtil.findChildByType(this, KtTokens.WHILE_KEYWORD)
diff --git a/.../test/kotlin/io/gitlab/arturbosch/detekt/rules/style/LoopWithTooManyJumpStatementsSpec.kt b/.../test/kotlin/io/gitlab/arturbosch/detekt/rules/style/LoopWithTooManyJumpStatementsSpec.kt
@@ -1,8 +1,8 @@
 package io.gitlab.arturbosch.detekt.rules.style
 
 import io.gitlab.arturbosch.detekt.test.TestConfig
+import io.gitlab.arturbosch.detekt.test.assertThat
 import io.gitlab.arturbosch.detekt.test.compileAndLint
-import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 
 private const val MAX_JUMP_COUNT = "maxJumpCount"
@@ -22,7 +22,7 @@ class LoopWithTooManyJumpStatementsSpec {
                 }
             }
         """.trimIndent()
-        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasSize(1)
+        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasTextLocations(20 to 23)
     }
 
     @Test
@@ -34,7 +34,7 @@ class LoopWithTooManyJumpStatementsSpec {
                 }
             }
         """.trimIndent()
-        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasSize(1)
+        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasTextLocations(20 to 25)
     }
 
     @Test
@@ -46,7 +46,7 @@ class LoopWithTooManyJumpStatementsSpec {
                 } while (i < 1)
             }
         """.trimIndent()
-        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasSize(1)
+        assertThat(LoopWithTooManyJumpStatements().compileAndLint(code)).hasTextLocations(20 to 22)
     }
 
     @Test