diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt index e135c3dfe2b..fc6cb2689c4 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt +++ b/detekt-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() - .single() - val dotQualifiedsInLambda = lambda.collectDescendantsOfType() - - 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()?.receiverExpression?.text == IT_LITERAL }) { + report(CodeSmell(issue, Entity.from(callee), issue.description)) } } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt index a8f6bcbcf7b..56a0ac35b38 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt +++ b/detekt-rules-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() + } }