Simplify logic to have single traversal

Rename rule to SuspendFunSwallowedCancellation
Add new TCs

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

@@ -190,7 +190,7 @@ coroutines:
     active: true
   SleepInsteadOfDelay:
     active: true
-  SuspendFunInsideRunCatching:
+  SuspendFunSwallowedCancellation:
     active: false
   SuspendFunWithCoroutineScopeReceiver:
     active: false

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

@@ -22,7 +22,7 @@ class CoroutinesProvider : DefaultRuleSetProvider {
             SleepInsteadOfDelay(config),
             SuspendFunWithFlowReturnType(config),
             SuspendFunWithCoroutineScopeReceiver(config),
-            SuspendFunInsideRunCatching(config),
+            SuspendFunSwallowedCancellation(config),
         )
     )
 }

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

@@ -0,0 +1,180 @@
+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 org.jetbrains.kotlin.backend.common.descriptors.isSuspend
+import org.jetbrains.kotlin.com.intellij.psi.PsiElement
+import org.jetbrains.kotlin.descriptors.FunctionDescriptor
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtForExpression
+import org.jetbrains.kotlin.psi.KtValueArgument
+import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
+import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.util.getParameterForArgument
+import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
+
+/**
+`suspend` functions should not be called inside `runCatching`'s lambda block, because `runCatching` catches all the
+`Exception`s. For Coroutines to work in all cases, developers should make sure to propagate `CancellationException`
+exceptions. This means `CancellationException` should never be:
+ * caught and swallowed (even if logged)
+ * caught and propagated to external systems
+ * caught and shown to the user
+
+they must always be rethrown in the same context.
+
+Using `runCatching` increases this risk of mis-handling cancellation. If you catch and don't rethrow all the
+`CancellationException`, your coroutines are not cancelled even if you cancel their `CoroutineScope`.
+
+This can very easily lead to:
+ * unexpected crashes
+ * extremely hard to diagnose bugs
+ * memory leaks
+ * performance issues
+ * battery drain
+
+For reference, see [Kotlin doc](https://kotlinlang.org/docs/cancellation-and-timeouts.html#cancellation-is-cooperative).
+
+If your project wants to use `runCatching` and `Result` objects, it is recommended to write a `coRunCatching`
+utility function which immediately re-throws `CancellationException`; and forbid `runCatching` and `suspend`
+combinations by activating this rule.
+ *
+ * <noncompliant>
+ * @@Throws(IllegalStateException::class)
+ * suspend fun bar(delay: Long) {
+ *     check(delay <= 1_000L)
+ *     delay(delay)
+ * }
+ *
+ * suspend fun foo() {
+ *     runCatching {
+ *         bar(1_000L)
+ *     }
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * @@Throws(IllegalStateException::class)
+ * suspend fun bar(delay: Long) {
+ *     check(delay <= 1_000L)
+ *     delay(delay)
+ * }
+ *
+ * suspend fun foo() {
+ *     try {
+ *         bar(1_000L)
+ *     } catch (e: IllegalStateException) {
+ *         // handle error
+ *     }
+ * }
+ *
+ * // Alternate
+ * @@Throws(IllegalStateException::class)
+ * suspend fun foo() {
+ *     bar(1_000L)
+ * }
+ * </compliant>
+ *
+ */
+@RequiresTypeResolution
+class SuspendFunSwallowedCancellation(config: Config) : Rule(config) {
+    override val issue = Issue(
+        id = "SuspendFunInsideRunCatching",
+        severity = Severity.Minor,
+        description = "`suspend` functions should not be called inside `runCatching`'s lambda block, because " +
+            "`runCatching` catches all the `Exception`s.",
+        debt = Debt.TEN_MINS
+    )
+
+    override fun visitCallExpression(expression: KtCallExpression) {
+        super.visitCallExpression(expression)
+
+        val resultingDescriptor = expression.getResolvedCall(bindingContext)?.resultingDescriptor
+        resultingDescriptor ?: return
+        if (resultingDescriptor.fqNameSafe != RUN_CATCHING_FQ) return
+
+        expression.forEachDescendantOfType<KtCallExpression>({
+            expression == it || shouldTraverseInside(it, bindingContext)
+        }) { descendant ->
+            val callableDescriptor = descendant.getResolvedCall(bindingContext)?.resultingDescriptor
+            if (callableDescriptor?.isSuspend == true) {
+                report(
+                    message = "The suspend function call ${callableDescriptor.fqNameSafe.shortName()} called inside " +
+                        "`runCatching`. You should either use specific `try-catch` only catching exception that you " +
+                        "are expecting or rethrow the `CancellationException` if already caught.",
+                    expression
+                )
+            }
+        }
+
+        expression.forEachDescendantOfType<KtForExpression> { descendant ->
+            if (descendant.doesLoopHasSuspendingIterators()) {
+                report(
+                    message = "The for-loop expression has suspending operator which is called inside " +
+                        "`runCatching`. You should either use specific `try-catch` only catching exception that you " +
+                        "are expecting or rethrow the `CancellationException` if already caught.",
+                    expression
+                )
+            }
+        }
+    }
+
+    @Suppress("ReturnCount")
+    private fun shouldTraverseInside(psiElement: PsiElement, bindingContext: BindingContext): Boolean {
+        if (psiElement is KtCallExpression) {
+            val callableDescriptor =
+                psiElement.getResolvedCall(bindingContext)?.resultingDescriptor as? FunctionDescriptor
+            callableDescriptor ?: return false
+
+            if (callableDescriptor.fqNameSafe == RUN_CATCHING_FQ) return false
+
+            if (callableDescriptor.isInline.not()) return false
+        }
+
+        if (psiElement is KtValueArgument) {
+            val callExpression = psiElement.getParentOfType<KtCallExpression>(true)
+            callExpression ?: return false
+            val valueParameterDescriptor =
+                callExpression.getResolvedCall(bindingContext)?.getParameterForArgument(psiElement)
+            valueParameterDescriptor ?: return false
+            if (valueParameterDescriptor.isCrossinline || valueParameterDescriptor.isNoinline) return false
+        }
+        return true
+    }
+
+    private fun KtForExpression.doesLoopHasSuspendingIterators(): Boolean {
+        val iteratorResolvedCall = bindingContext[BindingContext.LOOP_RANGE_ITERATOR_RESOLVED_CALL, this.loopRange]
+        val loopRangeHasNextResolvedCall =
+            bindingContext[BindingContext.LOOP_RANGE_HAS_NEXT_RESOLVED_CALL, this.loopRange]
+        val loopRangeNextResolvedCall = bindingContext[BindingContext.LOOP_RANGE_NEXT_RESOLVED_CALL, this.loopRange]
+        return listOf(iteratorResolvedCall, loopRangeHasNextResolvedCall, loopRangeNextResolvedCall).any {
+            it?.resultingDescriptor?.isSuspend == true
+        }
+    }
+
+    private fun report(
+        message: String,
+        expression: KtCallExpression,
+    ) {
+        report(
+            CodeSmell(
+                issue,
+                Entity.from(expression),
+                message
+            )
+        )
+    }
+
+    companion object {
+        private val RUN_CATCHING_FQ = FqName("kotlin.runCatching")
+    }
+}