Add SuspendFunWithCoroutineScopeReceiver Rule

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

@@ -168,6 +168,8 @@ coroutines:
     active: false
   SleepInsteadOfDelay:
     active: false
+  SuspendFunWithCoroutineScopeReceiver:
+    active: false
   SuspendFunWithFlowReturnType:
     active: false
 

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

@@ -20,7 +20,8 @@ class CoroutinesProvider : DefaultRuleSetProvider {
             InjectDispatcher(config),
             RedundantSuspendModifier(config),
             SleepInsteadOfDelay(config),
-            SuspendFunWithFlowReturnType(config)
+            SuspendFunWithFlowReturnType(config),
+            SuspendFunWithCoroutineScopeReceiver(config)
         )
     )
 }

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

@@ -0,0 +1,108 @@
+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 io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+import io.gitlab.arturbosch.detekt.rules.fqNameOrNull
+import org.jetbrains.kotlin.builtins.getReceiverTypeFromFunctionType
+import org.jetbrains.kotlin.builtins.isSuspendFunctionType
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.psi.KtParameter
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.types.KotlinType
+import org.jetbrains.kotlin.types.typeUtil.supertypes
+
+/**
+ * Suspend functions that use `CoroutineScope` as receiver should not be marked as `suspend`.
+ * A `CoroutineScope` provides structured concurrency via its `coroutineContext`. A `suspend`
+ * function also has its own `coroutineContext`, which is now ambiguous and mixed with the
+ * receiver`s.
+ *
+ * See https://kotlinlang.org/docs/coroutines-basics.html#scope-builder-and-concurrency
+ *
+ * <noncompliant>
+ * suspend fun CoroutineScope.foo() {
+ *     launch {
+ *       delay(1.seconds)
+ *     }
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * fun CoroutineScope.foo() {
+ *     launch {
+ *       delay(1.seconds)
+ *     }
+ * }
+ *
+ * // Alternative
+ * suspend fun foo() = coroutineScope {
+ *     launch {
+ *       delay(1.seconds)
+ *     }
+ * }
+ * </compliant>
+ *
+ */
+@RequiresTypeResolution
+class SuspendFunWithCoroutineScopeReceiver(config: Config) : Rule(config) {
+
+    override val issue = Issue(
+        id = "SuspendFunWithCoroutineScopeReceiver",
+        severity = Severity.Minor,
+        description = "The `suspend` modifier should not be used for functions that use a " +
+            "CoroutinesScope as receiver. You should use suspend functions without the receiver or use plain " +
+            "functions and use coroutineScope { } instead.",
+        debt = Debt.TEN_MINS
+    )
+
+    override fun visitNamedFunction(function: KtNamedFunction) {
+        if (bindingContext == BindingContext.EMPTY) return
+        checkReceiver(function)
+        checkLambdaParameters(function.valueParameters)
+    }
+
+    private fun checkLambdaParameters(parameters: List<KtParameter>) {
+        for (it in parameters) {
+            val type = bindingContext[BindingContext.VALUE_PARAMETER, it]
+                ?.type?.takeIf { it.isSuspendFunctionType } ?: continue
+            if (type.getReceiverTypeFromFunctionType()?.isCoroutineScope() == true) {
+                report(
+                    CodeSmell(
+                        issue = issue,
+                        entity = Entity.Companion.from(it),
+                        message = "`suspend` function uses CoroutineScope as receiver."
+                    )
+                )
+            }
+        }
+    }
+
+    private fun checkReceiver(function: KtNamedFunction) {
+        val suspendModifier = function.modifierList?.getModifier(KtTokens.SUSPEND_KEYWORD) ?: return
+        val receiver = bindingContext[BindingContext.FUNCTION, function]
+            ?.extensionReceiverParameter?.value?.type ?: return
+        if (receiver.isCoroutineScope()) {
+            report(
+                CodeSmell(
+                    issue = issue,
+                    entity = Entity.from(suspendModifier),
+                    message = "`suspend` function uses CoroutineScope as receiver."
+                )
+            )
+        }
+    }
+
+    private fun KotlinType.isCoroutineScope() = sequence {
+        yield(this@isCoroutineScope)
+        yieldAll(this@isCoroutineScope.supertypes())
+    }
+        .mapNotNull { it.fqNameOrNull()?.asString() }
+        .contains("kotlinx.coroutines.CoroutineScope")
+}

detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiverSpec.kt

@@ -0,0 +1,229 @@
+package io.gitlab.arturbosch.detekt.rules.coroutines
+
+import io.gitlab.arturbosch.detekt.api.Config
+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.Nested
+import org.junit.jupiter.api.Test
+
+@KotlinCoreEnvironmentTest
+class SuspendFunWithCoroutineScopeReceiverSpec(val env: KotlinCoreEnvironment) {
+
+    val subject = SuspendFunWithCoroutineScopeReceiver(Config.empty)
+
+    @Nested
+    inner class `SuspendFunWithCoroutineScopeReceiver rule` {
+
+        @Test
+        fun `reports when top-level suspend function has explicit CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+
+                suspend fun CoroutineScope.foo() {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `reports when top-level suspend function has explicit CoroutineScope receiver type and star import used`() {
+            val code = """
+                import kotlinx.coroutines.*
+
+                suspend fun CoroutineScope.foo() {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `reports when top-level suspend function has explicit FQN CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+
+                suspend fun kotlinx.coroutines.CoroutineScope.foo() {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `reports when suspend function has a receiver which inherits from CoroutineScope`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+
+                interface TestScope: CoroutineScope
+
+                suspend fun TestScope.foo() {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `no reports when plain function has a CoroutineScope as receiver`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+
+                fun CoroutineScope.foo() {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+
+        @Test
+        fun `no reports when suspend function has no receiver`() {
+            val code = """
+                import kotlinx.coroutines.coroutineScope
+                import kotlinx.coroutines.delay
+                import kotlin.time.Duration.Companion.seconds
+
+                suspend fun foo() = coroutineScope {
+                    launch {
+                        delay(timeMillis = 1000)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+
+        @Test
+        fun `no reports when suspend function has Long as receiver`() {
+            val code = """
+                import kotlinx.coroutines.delay
+
+                suspend fun Long.foo() = coroutineScope {
+                    launch {
+                        delay(timeMillis = this@foo)
+                    }
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+    }
+
+    @Nested
+    inner class SuspendFunWithCoroutineScopeLambda {
+
+        @Test
+        fun `reports when lambda parameter has suspend and explicit CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                suspend fun foo(action: suspend CoroutineScope.() -> Unit) = coroutineScope {
+                    action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `reports when lambda parameter has suspend and inherited CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                interface TestScope: CoroutineScope
+
+                suspend fun foo(action: suspend TestScope.() -> Unit) = coroutineScope {
+                    val scope = object: TestScope, CoroutineScope by this { }
+                    scope.action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+        }
+
+        @Test
+        fun `no report when lambda parameter has only CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                suspend fun foo(action: CoroutineScope.() -> Unit) = coroutineScope {
+                    action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+
+        @Test
+        fun `no report when suspend lambda parameter has no CoroutineScope receiver type`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                suspend fun foo(action: suspend Int.() -> Unit) {
+                    1.action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+
+        @Test
+        fun `no report when suspend lambda parameter has no receiver`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                suspend fun foo(action: suspend () -> Unit) {
+                    action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+        }
+    }
+
+    @Nested
+    inner class SuspendCoroutineFunWithCoroutineScopeLambda {
+
+        @Test
+        fun `reports when lambda parameter has suspend CoroutineScope receiver type and its lambda too`() {
+            val code = """
+                import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.launch
+                import kotlinx.coroutines.delay
+                import kotlinx.coroutines.coroutineScope
+
+                suspend fun CoroutineScope.foo(action: suspend CoroutineScope.() -> Unit) {
+                    action()
+                }
+            """
+            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(2)
+        }
+    }
+}