Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement AlsoCouldBeApply Rule #5333

Merged
merged 10 commits into from Oct 4, 2022
2 changes: 2 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -608,6 +608,8 @@ style:
excludeRawStrings: true
MayBeConst:
active: true
MisusedAlso:
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
active: false
ModifierOrder:
active: true
MultilineLambdaItParameter:
Expand Down
@@ -0,0 +1,62 @@
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 {
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
* init()
* block()
* }
* </compliant>
*/
class MisusedAlso(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
"MisusedAlso",
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) {
if (expression.calleeExpression?.text == "also") {
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(expression), issue.description))
}
} else {
super.visitCallExpression(expression)
}
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
}
}
Expand Up @@ -104,6 +104,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
UseAnyOrNoneInsteadOfFind(config),
UnnecessaryBackticks(config),
MaxChainedCallsOnSameLine(config),
MisusedAlso(config)
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
)
)
}
@@ -0,0 +1,92 @@
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 MisusedAlsoSpec {
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
val subject = MisusedAlso(Config.empty)

@Test
fun `does not report when no also is used`() {
val code = """
class Buzz {
fun init() {}
fun block() {}
}

fun foo(block: Buzz.() -> Unit): Buzz =
Buzz().let {
init()
block()
}
""".trimIndent()
assertThat(subject.compileAndLint(code)).isEmpty()
}

@Test
fun `reports an also in init of class`() {
val code = """
class Test {
private var a = 5

init {
a.also {
it.plus(5)
}
}
}
""".trimIndent()
assertThat(subject.compileAndLint(code)).hasSize(1)
}

@Test
fun `reports an also on nullable type`() {
val code = """
class Test {
private var a: Int? = 5

init {
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 = """
class Test {
private var a: Int? = 5

init {
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 = """
class Test {
private var a: Int? = 5
private var b: Int = 0

init {
a?.also({
b.plus(5)
})
Tommyten marked this conversation as resolved.
Show resolved Hide resolved
}
}
""".trimIndent()
assertThat(subject.compileAndLint(code)).isEmpty()
}
}