diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 764ba811aa5..098e8e0a955 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -499,6 +499,8 @@ potential-bugs: style: active: true + AlsoCouldBeApply: + active: false CanBeNonNullable: active: false CascadingCallWrapping: 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 new file mode 100644 index 00000000000..e135c3dfe2b --- /dev/null +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApply.kt @@ -0,0 +1,69 @@ +package io.gitlab.arturbosch.detekt.rules.style + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +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 org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtLambdaExpression +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType + +/** + * Detects when an `also` block contains only `it`-started expressions. + * + * By refactoring the `also` block to an `apply` block makes it so that all `it`s can be removed + * thus making the block more concise and easier to read. + * + * + * Buzz().also { + * it.init() + * it.block() + * } + * + * + * + * Buzz().apply { + * init() + * block() + * } + * + * // Also compliant + * fun foo(a: Int): Int { + * return a.also { println(it) } + * } + * + */ +class AlsoCouldBeApply(config: Config = Config.empty) : Rule(config) { + + override val issue = Issue( + "AlsoCouldBeApply", + Severity.Style, + "When an `also` block contains only `it`-started expressions, simplify it to the `apply` block.", + Debt.FIVE_MINS + ) + + 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)) + } + } + } +} diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt index f505fac8a03..cdab5191a92 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt @@ -104,6 +104,7 @@ class StyleGuideProvider : DefaultRuleSetProvider { UseAnyOrNoneInsteadOfFind(config), UnnecessaryBackticks(config), MaxChainedCallsOnSameLine(config), + AlsoCouldBeApply(config), ) ) } 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 new file mode 100644 index 00000000000..a8f6bcbcf7b --- /dev/null +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/AlsoCouldBeApplySpec.kt @@ -0,0 +1,121 @@ +package io.gitlab.arturbosch.detekt.rules.style + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.test.assertThat +import io.gitlab.arturbosch.detekt.test.compileAndLint +import org.junit.jupiter.api.Test + +class AlsoCouldBeApplySpec { + val subject = AlsoCouldBeApply(Config.empty) + + @Test + fun `does not report when no also is used`() { + val code = """ + fun f(a: Int) { + a.let { + it.plus(5) + it.minus(10) + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).isEmpty() + } + + @Test + fun `reports an also where only it is used in block`() { + val code = """ + fun f(a: Int) { + a.also { + it.plus(5) + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `report is focused on also keyword`() { + val code = """ + fun f(a: Int) { + a.also { + it.plus(5) + } + } + """.trimIndent() + + val findings = subject.compileAndLint(code) + + assertThat(findings).hasSize(1) + assertThat(findings).hasStartSourceLocation(2, 7) + assertThat(findings).hasEndSourceLocation(2, 11) + } + + @Test + fun `reports an also on nullable type`() { + val code = """ + fun f(a: Int?) { + a?.also { + it.plus(5) + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `reports an also with lambda passed as Argument in parenthesis`() { + val code = """ + fun f(a: Int?) { + a?.also({ + it.plus(5) + }) + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `does not report if it is not used in also`() { + val code = """ + fun f(a: Int?, b: Int) { + a?.also { + b.plus(5) + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).isEmpty() + } + + @Test + fun `does not report if it is renamed`() { + val code = """ + fun f(x: Int, y: Int) { + x.also { named -> + named.plus(5) + named.minus(y) + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).isEmpty() + } + + @Test + fun `does report if it is on one line separated by semicolon`() { + val code = """ + fun f(a: Int) { + a.also { it.plus(5); it.minus(10) } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `detect violation in also nested in also`() { + val code = """ + fun f(a: Int) { + a.also { x -> x.also { it.plus(10) } } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } +}