Report if/else as issue location instead of block (#5407)

Report mandatory braces issues on the actual if and/or else keywords instead of the then or else code blocks to prevent hiding potential other issues in those code blocks. Closes #5317
detekt · Oct 13, 2022 · 48f5615 · 48f5615
1 parent 134a8ea
commit 48f5615
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 9 deletions.
diff --git a/...in/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatements.kt b/...in/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatements.kt
@@ -43,12 +43,12 @@ class MandatoryBracesIfStatements(config: Config = Config.empty) : Rule(config)
 
         val thenExpression = expression.then ?: return
         if (thenExpression !is KtBlockExpression && hasNewLineAfter(expression.rightParenthesis)) {
-            report(CodeSmell(issue, Entity.from(thenExpression), issue.description))
+            report(CodeSmell(issue, Entity.from(expression.ifKeyword), issue.description))
         }
 
         val elseExpression = expression.`else` ?: return
         if (mustBeOnSameLine(elseExpression) && hasNewLineAfter(expression.elseKeyword)) {
-            report(CodeSmell(issue, Entity.from(elseExpression), issue.description))
+            report(CodeSmell(issue, Entity.from(expression.elseKeyword ?: elseExpression), issue.description))
         }
     }
 

diff --git a/...otlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatementsSpec.kt b/...otlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatementsSpec.kt
@@ -24,7 +24,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(1)
-            assertThat(findings).hasTextLocations(32 to 41)
+            assertThat(findings).hasTextLocations(14 to 16)
         }
 
         @Test
@@ -54,6 +54,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(2)
+            assertThat(findings).hasTextLocations(11 to 13, 44 to 48)
         }
 
         @Test
@@ -70,7 +71,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(2)
-            assertThat(findings).hasTextLocations(32 to 41, 59 to 68)
+            assertThat(findings).hasTextLocations(14 to 16, 46 to 50)
         }
 
         @Test
@@ -89,7 +90,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(3)
-            assertThat(findings).hasTextLocations(32 to 41, 70 to 79, 97 to 106)
+            assertThat(findings).hasTextLocations(14 to 16, 51 to 53, 84 to 88)
         }
 
         @Test
@@ -106,7 +107,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(1)
-            assertThat(findings).hasTextLocations(63 to 72)
+            assertThat(findings).hasTextLocations(50 to 54)
         }
 
         @Test
@@ -124,7 +125,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(1)
-            assertThat(findings).hasTextLocations(32 to 41)
+            assertThat(findings).hasTextLocations(14 to 16)
         }
 
         @Test
@@ -139,7 +140,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(1)
-            assertThat(findings).hasTextLocations(24 to 33)
+            assertThat(findings).hasTextLocations(14 to 16)
         }
 
         @Test
@@ -154,7 +155,7 @@ class MandatoryBracesIfStatementsSpec {
             )
 
             assertThat(findings).hasSize(1)
-            assertThat(findings).hasTextLocations(47 to 56)
+            assertThat(findings).hasTextLocations(34 to 38)
         }
     }