New rule: SuspendFunWithFlowReturnType

README.md

@@ -212,6 +212,7 @@ If you contributed to detekt but your name is not in the list, please feel free
 - [Veyndan Stuart](https://github.com/veyndan) - New rule: UseEmptyCounterpart; Rule improvement: UselessCallOnNotNull
 - [Parimatch Tech](https://github.com/parimatchtech) - New rule: LibraryEntitiesShouldNotBePublic
 - [Chao Zhang](https://github.com/chao2zhang) - Rule improvement: ImplicitDefaultLocale, ModifierOrder.
+- [Marcelo Hernandez](https://github.com/mhernand40) - New rule: SuspendFunWithFlowReturnType
 
 ### Mentions
 

detekt-bom/build.gradle.kts

@@ -21,7 +21,7 @@ dependencies {
         api("com.pinterest.ktlint:ktlint-core:${version.ktlint}")
         api("com.pinterest.ktlint:ktlint-ruleset-experimental:${version.ktlint}")
         api("org.jetbrains.kotlinx:kotlinx-html-jvm:0.7.2")
-        api("org.assertj:assertj-core:3.16.1")
+        api("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.8")
     }
 }
 

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

@@ -127,6 +127,8 @@ coroutines:
     active: false
   RedundantSuspendModifier:
     active: false
+  SuspendFunWithFlowReturnType:
+    active: false
 
 empty-blocks:
   active: true

detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/CoroutinesProvider.kt

@@ -17,7 +17,8 @@ class CoroutinesProvider : DefaultRuleSetProvider {
         ruleSetId,
         listOf(
             GlobalCoroutineUsage(config),
-            RedundantSuspendModifier(config)
+            RedundantSuspendModifier(config),
+            SuspendFunWithFlowReturnType(config)
         )
     )
 }

detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithFlowReturnType.kt

@@ -0,0 +1,97 @@
+package io.gitlab.arturbosch.detekt.rules.coroutines
+
+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 org.jetbrains.kotlin.js.descriptorUtils.getJetTypeFqName
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.types.KotlinType
+import org.jetbrains.kotlin.types.typeUtil.supertypes
+
+/**
+ * Functions that return `Flow` from `kotlinx.coroutines.flow` should not be marked as `suspend`.
+ * `Flows` are intended to be cold observable streams. The act of simply invoking a function that
+ * returns a `Flow`, should not have any side effects. Only once collection begins against the
+ * returned `Flow`, should work actually be done.
+ *
+ * See https://kotlinlang.org/docs/reference/coroutines/flow.html#flows-are-cold
+ *
+ * <noncompliant>
+ * suspend fun observeSignals(): Flow<Unit> {
+ *     val pollingInterval = getPollingInterval() // Done outside of the flow builder block.
+ *     return flow {
+ *         while (true) {
+ *             delay(pollingInterval)
+ *             emit(Unit)
+ *         }
+ *     }
+ * }
+ *
+ * private suspend fun getPollingInterval(): Long {
+ *     // Return the polling interval from some repository
+ *     // in a suspending manner.
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * fun observeSignals(): Flow<Unit> {
+ *     return flow {
+ *         val pollingInterval = getPollingInterval() // Moved into the flow builder block.
+ *         while (true) {
+ *             delay(pollingInterval)
+ *             emit(Unit)
+ *         }
+ *     }
+ * }
+ *
+ * private suspend fun getPollingInterval(): Long {
+ *     // Return the polling interval from some repository
+ *     // in a suspending manner.
+ * }
+ * </compliant>
+ *
+ * @requiresTypeResolution
+ */
+class SuspendFunWithFlowReturnType(config: Config) : Rule(config) {
+
+    override val issue = Issue(
+        id = "SuspendFunWithFlowReturnType",
+        severity = Severity.Minor,
+        description = "`suspend` modifier should not be used for functions returning Coroutines " +
+            "Flow. Flows are cold streams and invoking a function that returns one should not " +
+            "produce any side effects.",
+        debt = Debt.TEN_MINS
+    )
+
+    override fun visitNamedFunction(function: KtNamedFunction) {
+        if (bindingContext == BindingContext.EMPTY) return
+        val suspendModifier = function.modifierList?.getModifier(KtTokens.SUSPEND_KEYWORD) ?: return
+        bindingContext[BindingContext.FUNCTION, function]
+            ?.returnType
+            ?.takeIf { it.isCoroutineFlow() }
+            ?.also {
+                report(
+                    CodeSmell(
+                        issue = issue,
+                        entity = Entity.from(suspendModifier),
+                        message = "`suspend` function returns Coroutines Flow."
+                    )
+                )
+            }
+    }
+
+    private fun KotlinType.isCoroutineFlow(): Boolean {
+        return sequence {
+            yield(this@isCoroutineFlow)
+            yieldAll(this@isCoroutineFlow.supertypes())
+        }
+            .map { it.getJetTypeFqName(printTypeArguments = false) }
+            .contains("kotlinx.coroutines.flow.Flow")
+    }
+}