diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index d1f23451b23..17c114392c4 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -367,6 +367,9 @@ performance: active: true ArrayPrimitive: active: true + CouldBeSequence: + active: false + threshold: 3 ForEachOnRange: active: true excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] diff --git a/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequence.kt b/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequence.kt new file mode 100644 index 00000000000..178cded16eb --- /dev/null +++ b/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequence.kt @@ -0,0 +1,102 @@ +package io.gitlab.arturbosch.detekt.rules.performance + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Rule +import io.gitlab.arturbosch.detekt.api.Severity +import io.gitlab.arturbosch.detekt.api.config +import io.gitlab.arturbosch.detekt.api.internal.Configuration +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.KtExpression +import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver +import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull + +/** + * Long chains of collection operations will have a performance penalty due to a new list being created for each call. Consider using sequences instead. Read more about this in the [documentation](https://kotlinlang.org/docs/sequences.html) + * + * + * listOf(1, 2, 3, 4).map { it*2 }.filter { it < 4 }.map { it*it } + * + * + * + * listOf(1, 2, 3, 4).asSequence().map { it*2 }.filter { it < 4 }.map { it*it }.toList() + * + * listOf(1, 2, 3, 4).map { it*2 } + * + */ +@RequiresTypeResolution +class CouldBeSequence(config: Config = Config.empty) : Rule(config) { + + override val issue: Issue = Issue( + "CouldBeSequence", + Severity.Performance, + "Several chained collection operations that should be a sequence.", + Debt.FIVE_MINS + ) + + @Configuration("the number of chained collection operations required to trigger rule") + private val threshold: Int by config(defaultValue = 3) + + private var visitedCallExpressions = mutableListOf() + + @Suppress("ReturnCount") + override fun visitCallExpression(expression: KtCallExpression) { + super.visitCallExpression(expression) + + if (bindingContext == BindingContext.EMPTY) return + + if (visitedCallExpressions.contains(expression)) return + + if (!expression.isCalling(operationsFqNames)) return + + var counter = 1 + var nextCall = expression.nextChainedCall() + while (counter < threshold && nextCall != null) { + visitedCallExpressions += nextCall + if (!nextCall.isCalling(operationsFqNames)) { + break + } + + counter++ + nextCall = nextCall.nextChainedCall() + } + + if (counter >= threshold) { + val message = "${expression.text} could be .asSequence().${expression.text}" + report(CodeSmell(issue, Entity.from(expression), message)) + } + } + + private fun KtExpression.nextChainedCall(): KtExpression? { + val expression = this.getQualifiedExpressionForSelectorOrThis() + return expression.getQualifiedExpressionForReceiver()?.selectorExpression + } + + 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 + } + + companion object { + private val operationsFqNames = listOf( + FqName("kotlin.collections.filter"), + FqName("kotlin.collections.filterIndexed"), + FqName("kotlin.collections.map"), + FqName("kotlin.collections.mapIndexed"), + FqName("kotlin.collections.flatMap"), + FqName("kotlin.collections.flatMapIndexed"), + FqName("kotlin.collections.reduce"), + FqName("kotlin.collections.zip") + ) + } +} diff --git a/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/PerformanceProvider.kt b/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/PerformanceProvider.kt index c7ad20c54c8..869647d0e7e 100644 --- a/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/PerformanceProvider.kt +++ b/detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/PerformanceProvider.kt @@ -19,7 +19,8 @@ class PerformanceProvider : DefaultRuleSetProvider { ForEachOnRange(config), SpreadOperator(config), UnnecessaryTemporaryInstantiation(config), - ArrayPrimitive(config) + ArrayPrimitive(config), + CouldBeSequence(config) ) ) } diff --git a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequenceSpec.kt b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequenceSpec.kt new file mode 100644 index 00000000000..1deab2c4678 --- /dev/null +++ b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/CouldBeSequenceSpec.kt @@ -0,0 +1,69 @@ +package io.gitlab.arturbosch.detekt.rules.performance + +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest +import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext +import org.assertj.core.api.Assertions.assertThat +import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment +import org.junit.jupiter.api.Test + +@KotlinCoreEnvironmentTest +class CouldBeSequenceSpec(val env: KotlinCoreEnvironment) { + val subject = CouldBeSequence(TestConfig(mapOf("threshold" to 3))) + + @Test + fun `long collection chain should be sequence`() { + val code = """ + val myCollection = listOf(1, 2, 3, 4, 5, 6, 7, 8) + val processed = myCollection.filter { + it % 2 == 0 + }.map { + it*2 + }.filter { + it > 5 + } + """ + assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) + } + + @Test + fun `one collection operation should not trigger rule`() { + val code = """ + val myCollection = listOf(1, 2, 3, 4, 5, 6, 7, 8) + val processed = myCollection.filter { + it % 2 == 0 + } + """ + assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() + } + + @Test + fun `sequence operations should not trigger rule`() { + val code = """ + val myCollection = listOf(1, 2, 3, 4, 5, 6, 7, 8) + val processed = myCollection.asSequence().filter { + it % 2 == 0 + }.map { + it*2 + }.filter { + it > 5 + } + """ + assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() + } + + @Test + fun `sequence should not trigger rule`() { + val code = """ + val mySequence = sequenceOf(1,10,4,6,8,39) + val processed = mySequence.filter { + it % 2 == 0 + }.map { + it*2 + }.filter { + it > 5 + }.toList() + """ + assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() + } +}