Fix false negative SafeCast with no braces (#5479)

detekt · Oct 26, 2022 · e014ef8 · e014ef8
1 parent ecbc7c6
commit e014ef8
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 5 deletions.
diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/SafeCast.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/SafeCast.kt
@@ -8,7 +8,9 @@ import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.Severity
 import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
+import io.gitlab.arturbosch.detekt.rules.safeAs
 import org.jetbrains.kotlin.KtNodeTypes
+import org.jetbrains.kotlin.psi.KtBlockExpression
 import org.jetbrains.kotlin.psi.KtConstantExpression
 import org.jetbrains.kotlin.psi.KtExpression
 import org.jetbrains.kotlin.psi.KtIfExpression
@@ -61,13 +63,19 @@ class SafeCast(config: Config = Config.empty) : Rule(config) {
         }
     }
 
-    private fun isIfElseNull(thenClause: KtExpression?, elseClause: KtExpression?, identifier: String): Boolean {
-        val hasIdentifier = thenClause?.children?.firstOrNull()?.text == identifier
-        val elseStatement = elseClause?.children?.firstOrNull()
-        val hasNull = elseStatement is KtConstantExpression && elseStatement.node.elementType == KtNodeTypes.NULL
-        return hasIdentifier && hasNull
+    private fun isIfElseNull(thenClause: KtExpression?, elseClause: KtExpression?, identifier: String): Boolean =
+        thenClause.isIdentifier(identifier) && elseClause.isNullConstant()
+
+    private fun KtExpression?.isIdentifier(identifier: String): Boolean = singleExpression()?.text == identifier
+
+    private fun KtExpression?.isNullConstant(): Boolean {
+        val singleExpression = singleExpression() ?: return false
+        return singleExpression is KtConstantExpression && singleExpression.node.elementType == KtNodeTypes.NULL
     }
 
+    private fun KtExpression?.singleExpression(): KtExpression? =
+        if (this is KtBlockExpression) children.singleOrNull()?.safeAs() else this
+
     private fun addReport(expression: KtIfExpression) {
         report(CodeSmell(issue, Entity.from(expression), "This cast should be replaced with a safe cast: as?"))
     }

diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/SafeCastSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/SafeCastSpec.kt
@@ -35,6 +35,26 @@ class SafeCastSpec {
         assertThat(subject.compileAndLint(code)).hasSize(1)
     }
 
+    @Test
+    fun `reports negated expression with no braces`() {
+        val code = """
+            fun test(element: Int) {
+                val cast = if (element !is Number) null else element
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `reports expression with no braces`() {
+        val code = """
+            fun test(element: Int) {
+                val cast = if (element is Number) element else null
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
     @Test
     fun `does not report wrong condition`() {
         val code = """