Add new rule CouldBeSequence (#4855)

* Add new rule CouldBeSequence

* Add test case for CouldBeSequence

* Fix inconsistency in compliant example for CoudlBeSequence

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/**']

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)
+ *
+ * <noncompliant>
+ * listOf(1, 2, 3, 4).map { it*2 }.filter { it < 4 }.map { it*it }
+ * </noncompliant>
+ *
+ * <compliant>
+ * listOf(1, 2, 3, 4).asSequence().map { it*2 }.filter { it < 4 }.map { it*it }.toList()
+ *
+ * listOf(1, 2, 3, 4).map { it*2 }
+ * </compliant>
+ */
+@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<KtExpression>()
+
+    @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<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
+    }
+
+    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")
+        )
+    }
+}

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)
         )
     )
 }

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()
+    }
+}