Add UseSumOfInsteadOfFlatMapSize rule (#5405)

* Add UseSumOfInsteadOfFlatMapSize rule

* Add tests

detekt-core/src/main/resources/default-detekt-config.yml

@@ -717,6 +717,8 @@ style:
     active: true
   UseRequireNotNull:
     active: true
+  UseSumOfInsteadOfFlatMapSize:
+    active: false
   UselessCallOnNotNull:
     active: true
   UtilityClassWithPublicConstructor:

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt

@@ -76,7 +76,7 @@ class MultilineLambdaItParameter(val config: Config) : Rule(config) {
 
     override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) {
         super.visitLambdaExpression(lambdaExpression)
-        val size = lambdaExpression.collectDescendantsOfType<KtBlockExpression>().flatMap { it.statements }.size
+        val size = lambdaExpression.collectDescendantsOfType<KtBlockExpression>().sumOf { it.statements.size }
         if (size <= 1) return
 
         val parameterNames = lambdaExpression.valueParameters.map { it.name }

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt

@@ -105,6 +105,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
             UnnecessaryBackticks(config),
             MaxChainedCallsOnSameLine(config),
             AlsoCouldBeApply(config),
+            UseSumOfInsteadOfFlatMapSize(config),
         )
     )
 }

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseSumOfInsteadOfFlatMapSize.kt

@@ -0,0 +1,87 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+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.internal.RequiresTypeResolution
+import io.gitlab.arturbosch.detekt.rules.isCalling
+import io.gitlab.arturbosch.detekt.rules.safeAs
+import org.jetbrains.kotlin.builtins.DefaultBuiltIns
+import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtNameReferenceExpression
+import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver
+import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis
+import org.jetbrains.kotlin.resolve.calls.util.getType
+import org.jetbrains.kotlin.resolve.descriptorUtil.isSubclassOf
+
+/**
+ * Turn on this rule to flag `flatMap` and `size/count` calls that can be replaced with a `sumOf` call.
+ *
+ * <noncompliant>
+ * class Foo(val foo: List<Int>)
+ * list.flatMap { it.foo }.size
+ * list.flatMap { it.foo }.count()
+ * list.flatMap { it.foo }.count { it > 2 }
+ * listOf(listOf(1), listOf(2, 3)).flatten().size
+ * </noncompliant>
+ *
+ * <compliant>
+ * list.sumOf { it.foo.size }
+ * list.sumOf { it.foo.count() }
+ * list.sumOf { it.foo.count { foo -> foo > 2 } }
+ * listOf(listOf(1), listOf(2, 3)).sumOf { it.size }
+ * </compliant>
+ */
+@RequiresTypeResolution
+class UseSumOfInsteadOfFlatMapSize(config: Config = Config.empty) : Rule(config) {
+    override val issue: Issue = Issue(
+        javaClass.simpleName,
+        Severity.Style,
+        "Use `sumOf` instead of `flatMap` and `size/count` calls",
+        Debt.FIVE_MINS
+    )
+
+    @Suppress("ReturnCount")
+    override fun visitCallExpression(expression: KtCallExpression) {
+        super.visitCallExpression(expression)
+
+        val calleeText = expression.calleeExpression?.text ?: return
+        if (!expression.isFlatMapOrFlatten()) return
+
+        val receiver = expression.getQualifiedExpressionForSelectorOrThis()
+        val selector = receiver.getQualifiedExpressionForReceiver()?.selectorExpression ?: return
+        if (!selector.isSizeOrCount(receiver)) return
+
+        val selectorText = selector.safeAs<KtCallExpression>()?.calleeExpression?.text ?: selector.text
+        val message = "Use 'sumOf' instead of '$calleeText' and '$selectorText'"
+        report(CodeSmell(issue, Entity.from(expression), message))
+    }
+
+    private fun KtCallExpression.isFlatMapOrFlatten(): Boolean = isCalling(flatMapAndFlattenFqName, bindingContext)
+
+    @Suppress("ReturnCount")
+    private fun KtExpression.isSizeOrCount(receiver: KtExpression): Boolean {
+        if (safeAs<KtNameReferenceExpression>()?.text == "size") {
+            val receiverType = receiver.getType(bindingContext) ?: return false
+            val descriptor = receiverType.constructor.declarationDescriptor?.safeAs<ClassDescriptor>() ?: return false
+            return descriptor.isSubclassOf(DefaultBuiltIns.Instance.list)
+        }
+        return safeAs<KtCallExpression>()?.isCalling(countFqName, bindingContext) == true
+    }
+
+    companion object {
+        private val flatMapAndFlattenFqName = listOf(
+            FqName("kotlin.collections.flatMap"),
+            FqName("kotlin.collections.flatten"),
+        )
+
+        private val countFqName = FqName("kotlin.collections.count")
+    }
+}

detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseSumOfInsteadOfFlatMapSizeSpec.kt

@@ -0,0 +1,161 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
+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.DisplayName
+import org.junit.jupiter.api.Test
+
+@KotlinCoreEnvironmentTest
+class UseSumOfInsteadOfFlatMapSizeSpec(val env: KotlinCoreEnvironment) {
+    val subject = UseSumOfInsteadOfFlatMapSize()
+
+    @Test
+    @DisplayName("Reports flatMap and size")
+    fun reportFlatMapAndSize() {
+        val code = """
+            fun test(list: List<Foo>) {
+                list.flatMap { it.foo }.size
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+        assertThat(actual[0].message).isEqualTo("Use 'sumOf' instead of 'flatMap' and 'size'")
+    }
+
+    @Test
+    @DisplayName("Reports flatMap and count")
+    fun reportFlatMapAndCount() {
+        val code = """
+            fun test(list: List<Foo>) {
+                list.flatMap { it.foo }.count()
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+        assertThat(actual[0].message).isEqualTo("Use 'sumOf' instead of 'flatMap' and 'count'")
+    }
+
+    @Test
+    @DisplayName("Reports flatMap and count with a argument")
+    fun reportFlatMapAndCountWithArgument() {
+        val code = """
+            fun test(list: List<Foo>) {
+                list.flatMap { it.foo }.count { it > 2 }
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+        assertThat(actual[0].message).isEqualTo("Use 'sumOf' instead of 'flatMap' and 'count'")
+    }
+
+    @Test
+    @DisplayName("Reports flatten and size")
+    fun reportFlattenAndSize() {
+        val code = """
+            fun test(list: List<List<Int>>) {
+                list.flatten().size
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+        assertThat(actual[0].message).isEqualTo("Use 'sumOf' instead of 'flatten' and 'size'")
+    }
+
+    @Test
+    @DisplayName("Reports flatMap and size on nullable list")
+    fun reportFlatMapAndSizeOnNullableList() {
+        val code = """
+            fun test(list: List<Foo>?) {
+                list?.flatMap { it.foo }?.size
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+    }
+
+    @Test
+    @DisplayName("Reports flatMap and size on implicit list receiver")
+    fun reportFlatMapAndSizeOnImplicitListReceiver() {
+        val code = """
+            fun List<Foo>.test() {
+                flatMap { it.foo }.size
+                sumOf { it.foo.size }
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+    }
+
+    @Test
+    @DisplayName("Reports flatMap and count on Set")
+    fun reportFlatMapAndCountOnSet() {
+        val code = """
+            fun test(set: Set<Bar>) {
+                set.flatMap { it.bar }.count()
+            }
+            class Bar(val bar: Set<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).hasSize(1)
+    }
+
+    @Test
+    @DisplayName("Does not report flatMap")
+    fun noReportFlatMap() {
+        val code = """
+            fun test(list: List<Foo>) {
+                list.flatMap { it.foo }
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).isEmpty()
+    }
+
+    @Test
+    @DisplayName("Does not report flatMap and first")
+    fun noReportFlatMapAndFirst() {
+        val code = """
+            fun test(list: List<Foo>) {
+                list.flatMap { it.foo }.first()
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).isEmpty()
+    }
+
+    @Test
+    @DisplayName("Does not report flatten")
+    fun noReportFlatten() {
+        val code = """
+            fun test(list: List<List<Int>>) {
+                list.flatten()
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).isEmpty()
+    }
+
+    @Test
+    @DisplayName("Does not report flatten and last")
+    fun noReportFlattenAndLast() {
+        val code = """
+            fun test(list: List<List<Int>>) {
+                list.flatten().last()
+            }
+            class Foo(val foo: List<Int>)
+        """.trimIndent()
+        val actual = subject.compileAndLintWithContext(env, code)
+        assertThat(actual).isEmpty()
+    }
+}