mirror of
https://github.com/jlengrand/detekt.git
synced 2026-03-10 08:11:23 +00:00
Fix false positives for UnnecessaryFilter (#3695)
This commit is contained in:
committed by
GitHub
parent
57ffa78248
commit
f290bf71eb
@@ -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<FqName>): 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")),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String>, a: List<String>, b: List<String>, c: List<String>) {
|
||||
val dbQueryParts = queryParts.filter { it.length > 1 }.take(3)
|
||||
a.size
|
||||
b.isEmpty()
|
||||
c.isNotEmpty()
|
||||
}
|
||||
"""
|
||||
val findings = subject.compileAndLintWithContext(env, code)
|
||||
assertThat(findings).isEmpty()
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user