diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilter.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilter.kt index b5412e282..a1a77e92d 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilter.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilter.kt @@ -10,9 +10,10 @@ import io.gitlab.arturbosch.detekt.api.Severity import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtCallExpression -import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtLambdaExpression -import org.jetbrains.kotlin.psi.psiUtil.nextLeaf +import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver +import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis import org.jetbrains.kotlin.psi.unpackFunctionLiteral import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall @@ -56,34 +57,18 @@ class UnnecessaryFilter(config: Config = Config.empty) : Rule(config) { super.visitCallExpression(expression) if (bindingContext == BindingContext.EMPTY) return - val calleeExpression = expression.calleeExpression - if (calleeExpression?.text != "filter") return + if (!expression.isCalling(filterFqNames)) return + val lambdaArgumentText = expression.lambda()?.text ?: return - val resolvedCall = expression.getResolvedCall(bindingContext) ?: return - if (resolvedCall.resultingDescriptor.fqNameOrNull() !in filterFqNames) return + val qualifiedOrCall = expression.getQualifiedExpressionForSelectorOrThis() + val nextCall = qualifiedOrCall.getQualifiedExpressionForReceiver()?.selectorExpression ?: return - expression.checkNextLeaf(sizeFqName) - expression.checkNextLeaf(collectionCountFqName) - expression.checkNextLeaf(sequencesCountFqName) - expression.checkNextLeaf(isEmptyFqName, "any") - expression.checkNextLeaf(isNotEmptyFqName, "none") - } - - @Suppress("ReturnCount") - private fun KtCallExpression.checkNextLeaf(leafName: FqName, correctOperator: String? = null) { - val shortName = leafName.shortName().toString() - val nextLeaf = this.nextLeaf { it.text == shortName }?.parent as? KtElement ?: return - val resolvedCall = nextLeaf.getResolvedCall(bindingContext) ?: return - - if (resolvedCall.resultingDescriptor.fqNameOrNull() != leafName) return - - report( - CodeSmell( - issue, - Entity.from(this), - "'${this.text}' can be replaced by '${correctOperator ?: shortName} ${this.lambda()?.text}'" - ) - ) + secondCalls.forEach { + if (nextCall.isCalling(it.fqName)) { + val message = "'${expression.text}' can be replaced by '${it.correctOperator} $lambdaArgumentText'" + report(CodeSmell(issue, Entity.from(expression), message)) + } + } } private fun KtCallExpression.lambda(): KtLambdaExpression? { @@ -91,12 +76,29 @@ class UnnecessaryFilter(config: Config = Config.empty) : Rule(config) { return argument?.getArgumentExpression()?.unpackFunctionLiteral() } + private fun KtExpression.isCalling(fqNames: List): Boolean { + val calleeText = (this as? KtCallExpression)?.calleeExpression?.text ?: this.text + val targetFqNames = fqNames.filter { it.shortName().asString() == calleeText } + if (targetFqNames.isEmpty()) return false + return getResolvedCall(bindingContext)?.resultingDescriptor?.fqNameOrNull() in targetFqNames + } + + private fun KtExpression.isCalling(fqName: FqName) = isCalling(listOf(fqName)) + + private data class SecondCall(val fqName: FqName, val correctOperator: String = fqName.shortName().asString()) + companion object { - private val sizeFqName = FqName("kotlin.collections.List.size") - private val isEmptyFqName = FqName("kotlin.collections.List.isEmpty") - private val isNotEmptyFqName = FqName("kotlin.collections.isNotEmpty") - private val collectionCountFqName = FqName("kotlin.collections.count") - private val sequencesCountFqName = FqName("kotlin.sequences.count") - private val filterFqNames = listOf(FqName("kotlin.collections.filter"), FqName("kotlin.sequences.filter")) + private val filterFqNames = listOf( + FqName("kotlin.collections.filter"), + FqName("kotlin.sequences.filter"), + ) + + private val secondCalls = listOf( + SecondCall(FqName("kotlin.collections.List.size")), + SecondCall(FqName("kotlin.collections.List.isEmpty"), "any"), + SecondCall(FqName("kotlin.collections.isNotEmpty"), "none"), + SecondCall(FqName("kotlin.collections.count")), + SecondCall(FqName("kotlin.sequences.count")), + ) } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilterSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilterSpec.kt index 97d132b82..ba4124f63 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilterSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryFilterSpec.kt @@ -155,5 +155,35 @@ class UnnecessaryFilterSpec : Spek({ val findings = subject.compileAndLintWithContext(env, code) assertThat(findings).isEmpty() } + + // https://github.com/detekt/detekt/issues/3541#issuecomment-815136831 + it("Size in another statement") { + val code = """ + fun foo() { + val strings = listOf("abc", "cde", "ader") + val filteredStrings = strings.filter { "a" in it } + filteredStrings.forEach { println(it) } + if (filteredStrings.size > 2) { + println("more than two") + } + } + """ + val findings = subject.compileAndLintWithContext(env, code) + assertThat(findings).isEmpty() + } + + // https://github.com/detekt/detekt/issues/3541 + it("Size/isEmpty()/isNotEmpty() in another statement") { + val code = """ + fun test(queryParts: List, a: List, b: List, c: List) { + val dbQueryParts = queryParts.filter { it.length > 1 }.take(3) + a.size + b.isEmpty() + c.isNotEmpty() + } + """ + val findings = subject.compileAndLintWithContext(env, code) + assertThat(findings).isEmpty() + } } })