From e014ef8194838b6b64e015dd23205bc705f611d8 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 26 Oct 2022 18:01:41 +0900 Subject: [PATCH] Fix false negative `SafeCast` with no braces (#5479) --- .../arturbosch/detekt/rules/style/SafeCast.kt | 18 ++++++++++++----- .../detekt/rules/style/SafeCastSpec.kt | 20 +++++++++++++++++++ 2 files changed, 33 insertions(+), 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 index 042f719a956..440daea96e1 100644 --- 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 index fdb603d2f8b..f177992c3f2 100644 --- 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 = """