From 4dac97990640efdd0ed3ea9bc103a3a6a7f60203 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 4 Nov 2022 18:02:29 +0900 Subject: [PATCH 1/2] MultilineLambdaItParameter: fix false positive for one-line statements with a lambda argument --- .../rules/style/MultilineLambdaItParameter.kt | 15 +++++++-- .../style/MultilineLambdaItParameterSpec.kt | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt index 5e352f64459..63a705d2e1e 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt @@ -12,8 +12,10 @@ 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.KtExpression import org.jetbrains.kotlin.psi.KtLambdaExpression -import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType +import org.jetbrains.kotlin.utils.ifEmpty /** * Lambda expressions are very useful in a lot of cases, and they often include very small chunks of @@ -76,8 +78,10 @@ class MultilineLambdaItParameter(val config: Config) : Rule(config) { override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) { super.visitLambdaExpression(lambdaExpression) - val size = lambdaExpression.collectDescendantsOfType().sumOf { it.statements.size } - if (size <= 1) return + + val statements = lambdaExpression.bodyExpression?.statements.orEmpty().ifEmpty { return } + val single = statements.singleOrNull() + if (single != null && (single.hasNoLineBreak() || single.hasNoStatements())) return val parameterNames = lambdaExpression.valueParameters.map { it.name } // Explicit `it` @@ -106,4 +110,9 @@ class MultilineLambdaItParameter(val config: Config) : Rule(config) { } } } + + private fun KtExpression.hasNoLineBreak() = !textContains('\n') + + private fun KtExpression.hasNoStatements() = + !anyDescendantOfType { it.statements.isNotEmpty() } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt index 21cff6dbac6..bb5f3726a2f 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt @@ -129,6 +129,24 @@ class MultilineLambdaItParameterSpec(val env: KotlinCoreEnvironment) { val findings = subject.compileAndLintWithContext(env, code) assertThat(findings).hasSize(1) } + + @Test + fun `does not report when statement has no nested statements`() { + val code = """ + data class Foo(val x: Int, val y: Int) + + fun f(i: Int?) { + val foo = i?.let { + Foo( + x = it, + y = 2 + ) + } + } + """.trimIndent() + val findings = subject.compileAndLintWithContext(env, code) + assertThat(findings).isEmpty() + } } @Nested @@ -171,6 +189,19 @@ class MultilineLambdaItParameterSpec(val env: KotlinCoreEnvironment) { val code = """ fun f() { val digits = 1234.let { param -> listOf(param) } + } + """.trimIndent() + val findings = subject.compileAndLintWithContext(env, code) + assertThat(findings).isEmpty() + } + + @Test + fun `does not report when statement has one-line lambda argument`() { + val code = """ + fun f() { + val digits = 1234.let { + check(it > 0) { println(it) } + } } """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) From 993cb9c255e5f60e41b075476e670fd4e0e4c63b Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 4 Nov 2022 21:41:08 +0900 Subject: [PATCH 2/2] Clarify tests --- .../detekt/rules/style/MultilineLambdaItParameterSpec.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt index bb5f3726a2f..db55814b059 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameterSpec.kt @@ -121,7 +121,7 @@ class MultilineLambdaItParameterSpec(val env: KotlinCoreEnvironment) { fun f() { val digits = 1234.let { check(it > 0) { - println(it) + println("error") } } } @@ -200,7 +200,7 @@ class MultilineLambdaItParameterSpec(val env: KotlinCoreEnvironment) { val code = """ fun f() { val digits = 1234.let { - check(it > 0) { println(it) } + check(it > 0) { println("error") } } } """.trimIndent()