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.

Sign up for GitHub

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

Jump to bottom

Fix false negative MultilineLambdaItParameter on complex multiline single statement #5397

Merged
merged 1 commit into from Oct 9, 2022
Merged

Fix false negative MultilineLambdaItParameter on complex multiline single statement #5397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...yle/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt
View file
Open in desktop
Expand Up @@ -11,7 +11,9 @@ import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import io.gitlab.arturbosch.detekt.rules.IT_LITERAL
import io.gitlab.arturbosch.detekt.rules.hasImplicitParameterReference
import io.gitlab.arturbosch.detekt.rules.implicitParameter
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType

/**
* Lambda expressions are very useful in a lot of cases, and they often include very small chunks of
Expand Down Expand Up @@ -74,8 +76,8 @@ class MultilineLambdaItParameter(val config: Config) : Rule(config) {

override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) {
super.visitLambdaExpression(lambdaExpression)
val size = lambdaExpression.bodyExpression?.statements?.size
if (size == null || size <= 1) return
val size = lambdaExpression.collectDescendantsOfType<KtBlockExpression>().flatMap { it.statements }.size
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-kameyama thanks for the quick fix!

Small note: unnecessary creation of intermediate list...

val size = lambdaExpression.collectDescendantsOfType<KtBlockExpression>().sumOf { it.statements.size }

looks like we could have a UseSumInsteadOfSize similar to UseAnyOrNoneInsteadOfFind, I'll open an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5399

if (size <= 1) return

val parameterNames = lambdaExpression.valueParameters.map { it.name }
// Explicit `it`
Expand Down
15 changes: 15 additions & 0 deletions ...src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt
View file
Open in desktop
Expand Up @@ -114,6 +114,21 @@ class MultilineLambdaItParameterSpec(val env: KotlinCoreEnvironment) {
val findings = subject.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `reports when statement is spanning multiple lines`() {
val code = """
fun f() {
val digits = 1234.let {
check(it > 0) {
println(it)
}
}
}
""".trimIndent()
val findings = subject.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}
}

@Nested
Expand Down