Skip to content

Commit

Permalink
Fix false positive on it usages when type parameter is specified (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
atulgpt committed Apr 13, 2024
1 parent f0f7129 commit 5ca4be7
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.rules.IT_LITERAL
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType

/**
* Lambda expressions are one of the core features of the language. They often include very small chunks of
Expand All @@ -28,6 +26,7 @@ import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
*
* <compliant>
* a?.let { it.plus(1) } // Much better to use implicit it
* a?.let { value: Int -> value.plus(1) } // Better as states the type more clearly
* foo.flatMapObservable(Observable::fromIterable) // Here we can have a method reference
*
* // For multiline blocks it is usually better come up with a clear and more meaningful name
Expand All @@ -49,14 +48,17 @@ class ExplicitItLambdaParameter(config: Config) : Rule(

override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) {
super.visitLambdaExpression(lambdaExpression)
if (lambdaExpression.getStrictParentOfType<KtCallableReferenceExpression>() != null) return
val parameterNames = lambdaExpression.valueParameters.map { it.name }
if (IT_LITERAL in parameterNames) {
val message = if (parameterNames.size == 1) {
"This explicit usage of `it` as the lambda parameter name can be omitted."
} else {
"`it` should not be used as name for a lambda parameter."
}
val message =
if (
parameterNames.size == 1 &&
lambdaExpression.valueParameters[0].typeReference == null
) {
"This explicit usage of `it` as the lambda parameter name can be omitted."
} else {
"`it` should not be used as name for a lambda parameter."
}
report(
CodeSmell(
Entity.from(lambdaExpression),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
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.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

Expand All @@ -21,6 +21,9 @@ class ExplicitItLambdaParameterSpec {
""".trimIndent()
)
assertThat(findings).hasSize(1)
assertThat(findings[0]).hasMessage(
"This explicit usage of `it` as the lambda parameter name can be omitted."
)
}

@Test
Expand All @@ -35,20 +38,32 @@ class ExplicitItLambdaParameterSpec {
assertThat(findings).hasSize(1)
}

@Test
fun `does not reports when parameter type is declared explicitly when variable name is not it`() {
val findings = subject.compileAndLint(
"""
fun f() {
val lambda = { value: Int -> value.toString() }
}
""".trimIndent()
)
assertThat(findings).isEmpty()
}

@Test
fun `does not report when parameter type is declared explicitly for un-inferrable lambda`() {
val findings = subject.compileAndLint(
"""
fun f1(): (Int) -> Int {
return { it: Int -> it.inc() }::invoke
return { value: Int -> value.inc() }::invoke
}
fun f2(): (Int) -> Int {
return { value: Int -> value.inc() }::invoke
}
fun f3(): (((Int) -> Int) -> Unit) -> (Int) -> Int {
return { it: Int -> it.inc() }::also
return { value: Int -> value.inc() }::also
}
""".trimIndent()
)
Expand All @@ -60,7 +75,7 @@ class ExplicitItLambdaParameterSpec {
val findings = subject.compileAndLint(
"""
fun f(): (Int) -> Int {
return ({ it: Int -> it.inc() })::invoke
return ({ value: Int -> value.inc() })::invoke
}
""".trimIndent()
)
Expand Down Expand Up @@ -112,15 +127,28 @@ class ExplicitItLambdaParameterSpec {
}

@Test
fun `does not report when parameter type is declared explicitly for multi params un-inferrable lambda`() {
fun `does not report when parameter type with is declared explicitly for multi params un-inferrable lambda`() {
val findings = subject.compileAndLint(
"""
fun f(): (Int, Int) -> Int {
return { it: Int, a: Int -> (it + a).inc() }::invoke
return { value: Int, a: Int -> (value + a).inc() }::invoke
}
""".trimIndent()
)
assertThat(findings).isEmpty()
}

@Test
fun `does report when parameter type with name it when declared explicitly for multi params un-inferrable lambda`() {
val findings = subject.compileAndLint(
"""
fun f(): (Int, Int) -> Int {
return { it: Int, a: Int -> (it + a).inc() }::invoke
}
""".trimIndent()
)
assertThat(findings).hasSize(1)
assertThat(findings[0]).hasMessage("`it` should not be used as name for a lambda parameter.")
}
}
}

0 comments on commit 5ca4be7

Please sign in to comment.