AlsoCouldBeApply: fix false positive when all statements are not it…

…-started expressions (#5376)
detekt · Oct 5, 2022 · f87a72d · f87a72d
1 parent b4710a2
commit f87a72d
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 17 deletions.
diff --git a/...t-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt b/...t-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt
@@ -8,10 +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.rules.IT_LITERAL
+import io.gitlab.arturbosch.detekt.rules.safeAs
 import org.jetbrains.kotlin.psi.KtCallExpression
-import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
-import org.jetbrains.kotlin.psi.KtLambdaExpression
-import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
+import org.jetbrains.kotlin.psi.KtQualifiedExpression
 
 /**
  * Detects when an `also` block contains only `it`-started expressions.
@@ -47,23 +46,17 @@ class AlsoCouldBeApply(config: Config = Config.empty) : Rule(config) {
         Debt.FIVE_MINS
     )
 
+    @Suppress("ReturnCount")
     override fun visitCallExpression(expression: KtCallExpression) {
         super.visitCallExpression(expression)
 
-        if (expression.calleeExpression?.text == "also") {
-            val alsoExpression = expression.calleeExpression ?: return
-
-            val lambda = expression.lambdaArguments.singleOrNull() ?: expression.valueArguments.single()
-                .collectDescendantsOfType<KtLambdaExpression>()
-                .single()
-            val dotQualifiedsInLambda = lambda.collectDescendantsOfType<KtDotQualifiedExpression>()
-
-            if (
-                dotQualifiedsInLambda.isNotEmpty() &&
-                dotQualifiedsInLambda.all { it.receiverExpression.textMatches(IT_LITERAL) }
-            ) {
-                report(CodeSmell(issue, Entity.from(alsoExpression), issue.description))
-            }
+        val callee = expression.calleeExpression?.takeIf { it.text == "also" } ?: return
+        val lambda = expression.lambdaArguments.singleOrNull()?.getLambdaExpression()
+            ?: expression.valueArguments.singleOrNull()?.getArgumentExpression()?.safeAs()
+            ?: return
+        val statements = lambda.bodyExpression?.statements.orEmpty().ifEmpty { return }
+        if (statements.all { it.safeAs<KtQualifiedExpression>()?.receiverExpression?.text == IT_LITERAL }) {
+            report(CodeSmell(issue, Entity.from(callee), issue.description))
         }
     }
 }
diff --git a/...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt b/...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt
@@ -118,4 +118,36 @@ class AlsoCouldBeApplySpec {
         """.trimIndent()
         assertThat(subject.compileAndLint(code)).hasSize(1)
     }
+
+    @Test
+    fun `does not report when all statements are not 'it'-started expressions`() {
+        val code = """
+            fun test(foo: Foo) {
+                foo.also {
+                    it.bar()
+                    println(it)
+                    it.baz()
+                }
+            }
+
+            class Foo {
+                fun bar() {}
+                fun baz() {}
+            }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report when no statements`() {
+        val code = """
+            fun test(foo: Foo) {
+                foo.also {
+                }
+            }
+
+            class Foo
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
 }