Skip to content

Commit

Permalink
Fix ReturnCount false positive when excludeReturnFromLambda is enabled (
Browse files Browse the repository at this point in the history
#5459)

* fix: ReturnCount false positive

* check ci

* fix: make test code snippet compile

* refactor: process excluding with or operator

* refactor: improve readability

* fix: count -> any for prevent full evaluation
  • Loading branch information
sanggggg committed Oct 26, 2022
1 parent 90915e5 commit 2ad733a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 12 deletions.
Expand Up @@ -13,8 +13,7 @@ import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.api.simplePatternToRegex
import io.gitlab.arturbosch.detekt.rules.parentsOfTypeUntil
import io.gitlab.arturbosch.detekt.rules.yieldStatementsSkippingGuardClauses
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtNameReferenceExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtReturnExpression
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
Expand Down Expand Up @@ -93,11 +92,9 @@ class ReturnCount(config: Config = Config.empty) : Rule(config) {
private fun shouldBeIgnored(function: KtNamedFunction) = function.name in excludedFunctions

private fun countReturnStatements(function: KtNamedFunction): Int {
fun KtReturnExpression.isExcluded(): Boolean = when {
excludeLabeled && labeledExpression != null -> true
excludeReturnFromLambda && isNamedReturnFromLambda() -> true
else -> false
}
fun KtReturnExpression.isExcluded(): Boolean =
(excludeReturnFromLambda && isNamedReturnFromLambda()) ||
(excludeLabeled && labeledExpression != null)

val statements = if (excludeGuardClauses) {
function.yieldStatementsSkippingGuardClauses<KtReturnExpression>()
Expand All @@ -113,11 +110,7 @@ class ReturnCount(config: Config = Config.empty) : Rule(config) {
private fun KtReturnExpression.isNamedReturnFromLambda(): Boolean {
val label = this.labeledExpression
if (label != null) {
return this.parentsOfTypeUntil<KtCallExpression, KtNamedFunction>()
.map { it.calleeExpression }
.filterIsInstance<KtNameReferenceExpression>()
.map { it.text }
.any { it in label.text }
return this.parentsOfTypeUntil<KtLambdaExpression, KtNamedFunction>().any()
}
return false
}
Expand Down
Expand Up @@ -512,4 +512,42 @@ class ReturnCountSpec {
assertThat(findings).isEmpty()
}
}

@Nested
inner class `function with lambda which has explicit label` {
val code = """
fun test() {
listOf(1, 2, 3, 4, 5).forEach lit@{
if (it == 3) return@lit
if (it == 4) return@lit
}
return
}
""".trimIndent()

@Test
fun `should count labeled return of lambda with explicit label`() {
val findings = ReturnCount(TestConfig(mapOf(EXCLUDE_RETURN_FROM_LAMBDA to "false"))).compileAndLint(code)
assertThat(findings).hasSize(1)
}

@Test
fun `should not count labeled return of lambda with explicit label when deactivated by default`() {
val findings = ReturnCount().compileAndLint(code)
assertThat(findings).isEmpty()
}

@Test
fun `excludeReturnFromLambda should take precedence over excludeLabeled`() {
val findings = ReturnCount(
TestConfig(
mapOf(
EXCLUDE_RETURN_FROM_LAMBDA to "true",
EXCLUDE_LABELED to "false"
)
)
).compileAndLint(code)
assertThat(findings).isEmpty()
}
}
}

0 comments on commit 2ad733a

Please sign in to comment.