Skip to content

Commit

Permalink
Implement AlsoCouldBeApply Rule (#5333)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tommyten committed Oct 4, 2022
1 parent 375d509 commit 24dce51
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 0 deletions.
2 changes: 2 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -499,6 +499,8 @@ potential-bugs:

style:
active: true
AlsoCouldBeApply:
active: false
CanBeNonNullable:
active: false
CascadingCallWrapping:
Expand Down
@@ -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.
*
* <noncompliant>
* Buzz().also {
* it.init()
* it.block()
* }
* </noncompliant>
*
* <compliant>
* Buzz().apply {
* init()
* block()
* }
*
* // Also compliant
* fun foo(a: Int): Int {
* return a.also { println(it) }
* }
* </compliant>
*/
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<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))
}
}
}
}
Expand Up @@ -104,6 +104,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
UseAnyOrNoneInsteadOfFind(config),
UnnecessaryBackticks(config),
MaxChainedCallsOnSameLine(config),
AlsoCouldBeApply(config),
)
)
}
@@ -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)
}
}

0 comments on commit 24dce51

Please sign in to comment.