From 2a81f8c7c4d97746fbc4264b8276e5876c5d00ff Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 6 Jan 2023 04:33:27 +0530 Subject: [PATCH] Simplify logic to have single traversal Rename rule to SuspendFunSwallowedCancellation Add new TCs --- .../main/resources/default-detekt-config.yml | 2 +- .../rules/coroutines/CoroutinesProvider.kt | 2 +- .../coroutines/SuspendFunInsideRunCatching.kt | 156 --------------- .../SuspendFunSwallowedCancellation.kt | 180 +++++++++++++++++ ...=> SuspendFunSwallowedCancellationSpec.kt} | 182 +++++++++++++----- 5 files changed, 321 insertions(+), 201 deletions(-) delete mode 100644 detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatching.kt create mode 100644 detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellation.kt rename detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/{SuspendFunInsideRunCatchingSpec.kt => SuspendFunSwallowedCancellationSpec.kt} (73%) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index eb4d620d923c..f5706832ce59 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/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 diff --git a/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/CoroutinesProvider.kt b/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/CoroutinesProvider.kt index 79b477b77f86..b4661c86d041 100644 --- a/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/CoroutinesProvider.kt +++ b/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), ) ) } diff --git a/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatching.kt b/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatching.kt deleted file mode 100644 index f464538023c6..000000000000 --- a/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatching.kt +++ /dev/null @@ -1,156 +0,0 @@ -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.descriptors.CallableDescriptor -import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.psi.KtCallExpression -import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType -import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType -import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypesAndPredicate -import org.jetbrains.kotlin.resolve.BindingContext -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 documentation](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. - * - * - * @@Throws(IllegalStateException::class) - * suspend fun bar(delay: Long) { - * check(delay <= 1_000L) - * delay(delay) - * } - * - * suspend fun foo() { - * runCatching { - * bar(1_000L) - * } - * } - * - * - * - * @@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 - * suspend fun foo() { - * bar(1_000L) - * } - * - * - */ -@RequiresTypeResolution -class SuspendFunInsideRunCatching(config: Config) : Rule(config) { - override val issue = Issue( - id = "SuspendFunInsideRunCatching", - severity = Severity.Minor, - description = "The `suspend` functions should be called inside `runCatching` block as it also swallows " + - "`CancellationException` which is important for cooperative cancellation." + - "You should either use specific `try-catch` only catching exception that you are expecting" + - " or rethrow the `CancellationException` if already caught", - 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 { descendant -> - if (descendant.getResolvedCall(bindingContext)?.resultingDescriptor?.isSuspend == true && shouldReport( - resultingDescriptor, - descendant, - bindingContext, - ) - ) { - val message = - "The suspend function call ${descendant.text} is inside `runCatching`. You should either " + - "use specific `try-catch` only catching exception that you are expecting or rethrow the " + - "`CancellationException` if already caught." - report( - CodeSmell( - issue, - Entity.from(expression), - message - ) - ) - } - } - } - - private fun shouldReport( - runCatchingCallableDescriptor: CallableDescriptor, - callExpression: KtCallExpression, - bindingContext: BindingContext, - ): Boolean { - val firstNonInlineOrRunCatchingParent = - callExpression.getParentOfTypesAndPredicate(true, KtCallExpression::class.java) { parentCallExp -> - val parentCallFunctionDescriptor = - parentCallExp.getResolvedCall(bindingContext)?.resultingDescriptor as? FunctionDescriptor - parentCallFunctionDescriptor ?: return@getParentOfTypesAndPredicate false - - val isParentRunCatching = parentCallFunctionDescriptor.fqNameSafe == RUN_CATCHING_FQ - val isInline = parentCallFunctionDescriptor.isInline - val noInlineAndCrossInlineValueParametersIndex = - parentCallFunctionDescriptor.valueParameters.filter { valueParameterDescriptor -> - valueParameterDescriptor.isCrossinline || valueParameterDescriptor.isNoinline - }.map { - it.index - } - val callExpressionIndexInParentCall = parentCallExp.valueArguments.indexOfFirst { valueArgument -> - valueArgument?.findDescendantOfType { - it == callExpression - } != null - } - isParentRunCatching || - isInline.not() || - noInlineAndCrossInlineValueParametersIndex.contains(callExpressionIndexInParentCall) - } - return firstNonInlineOrRunCatchingParent.getResolvedCall(bindingContext)?.resultingDescriptor == - runCatchingCallableDescriptor - } - - companion object { - private val RUN_CATCHING_FQ = FqName("kotlin.runCatching") - } -} diff --git a/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellation.kt b/detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellation.kt new file mode 100644 index 000000000000..47ff5dd6ff16 --- /dev/null +++ b/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. + * + * + * @@Throws(IllegalStateException::class) + * suspend fun bar(delay: Long) { + * check(delay <= 1_000L) + * delay(delay) + * } + * + * suspend fun foo() { + * runCatching { + * bar(1_000L) + * } + * } + * + * + * + * @@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) + * } + * + * + */ +@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({ + 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 { 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(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") + } +} diff --git a/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatchingSpec.kt b/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellationSpec.kt similarity index 73% rename from detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatchingSpec.kt rename to detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellationSpec.kt index 7fb0171f893b..c5eb349fe083 100644 --- a/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunInsideRunCatchingSpec.kt +++ b/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunSwallowedCancellationSpec.kt @@ -9,14 +9,14 @@ import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.junit.jupiter.api.Test @KotlinCoreEnvironmentTest -class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { +class SuspendFunSwallowedCancellationSpec(private val env: KotlinCoreEnvironment) { - private val subject = SuspendFunInsideRunCatching(Config.empty) + private val subject = SuspendFunSwallowedCancellation(Config.empty) @Test fun `does report suspend function call in runCatching`() { val code = """ - import kotlinx.coroutines.delay + import kotlinx.coroutines.delay suspend fun foo() { runCatching { @@ -25,31 +25,33 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { } """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000L)") + assertFindings(findings, "delay") } @Test fun `does report for in case of nested runCatching`() { val code = """ - import kotlinx.coroutines.delay + import kotlinx.coroutines.delay + + suspend fun bar() = delay(2000) suspend fun foo() { runCatching { delay(1000L) runCatching { - delay(2000L) + bar() } } } """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000L)", "delay(2000L)") + assertFindings(findings, "delay", "bar") } @Test fun `does report for delay() in suspend functions`() { val code = """ - import kotlinx.coroutines.delay + import kotlinx.coroutines.delay suspend fun foo() { runCatching { @@ -58,7 +60,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { } """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000L)") + assertFindings(findings, "delay") } @Test @@ -74,10 +76,54 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { assertFindings(findings) } + @Test + fun `does report when _when_ is used in result`() { + val code = """ + import kotlinx.coroutines.delay + suspend fun bar() = delay(1000L) + suspend fun foo(): Result<*> { + val result = runCatching { bar() } + when(result.isSuccess) { + true -> TODO() + false -> TODO() + } + return result + } + """.trimIndent() + val findings = subject.compileAndLintWithContext(env, code) + assertFindings(findings, "bar") + } + + @Test + fun `does report when onSuccess is used in result`() { + val code = """ + import kotlinx.coroutines.delay + suspend fun bar() = delay(1000L) + suspend fun foo() { + runCatching { bar() }.onSuccess { + TODO() + } + } + """.trimIndent() + val findings = subject.compileAndLintWithContext(env, code) + assertFindings(findings, "bar") + } + + @Test + fun `does report when runCatching is used as function expression`() { + val code = """ + import kotlinx.coroutines.delay + suspend fun bar() = delay(1000L) + suspend fun foo() = runCatching { bar() } + """.trimIndent() + val findings = subject.compileAndLintWithContext(env, code) + assertFindings(findings, "bar") + } + @Test fun `does not report when try catch is used`() { val code = """ - import kotlinx.coroutines.delay + import kotlinx.coroutines.delay suspend fun foo() { try { @@ -94,7 +140,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { @Test fun `does report when suspend fun is called inside inline function`() { val code = """ - import kotlinx.coroutines.delay + import kotlinx.coroutines.delay suspend fun foo() { runCatching { @@ -106,7 +152,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(it)") + assertFindings(findings, "delay") } @Test @@ -123,9 +169,9 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { crossinline crossinlineBlock: suspend () -> Unit, ) = inlineBlock().toString() + MainScope().launch { noinlineBlock() - }.toString() + runBlocking { + } + runBlocking { crossinlineBlock() - }.toString() + noinlineBlock() + }.toString() suspend fun bar() { runCatching { @@ -142,10 +188,9 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000L)") + assertFindings(findings, "delay") } - // Failing @Test fun `does report when inside inline function with noinline and cross inline parameters not in same order`() { val code = """ @@ -158,11 +203,11 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { noinline noinlineBlock: suspend () -> Unit, inlineBlock: () -> Unit, crossinline crossinlineBlock: suspend () -> Unit, - ) = - inlineBlock().toString() + - MainScope().launch { noinlineBlock() }.toString() + - runBlocking { crossinlineBlock() }.toString() + - noinlineBlock() + ) = inlineBlock().toString() + MainScope().launch { + noinlineBlock() + } + runBlocking { + crossinlineBlock() + }.toString() suspend fun bar() { @@ -180,7 +225,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000L)") + assertFindings(findings, "delay") } @Test @@ -206,6 +251,60 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { assertFindings(findings) } + @Test + fun `does not report when lambda parameter chain has noinline function call`() { + val code = """ + import kotlinx.coroutines.MainScope + import kotlinx.coroutines.delay + import kotlinx.coroutines.launch + + inline fun inline(block: () -> R) = block() + fun noInline(block: suspend () -> R) = MainScope().launch { block() } + + suspend fun bar() + { + runCatching { + inline { + noInline { + inline { + delay(1000) + } + } + + } + } + } + """.trimIndent() + + val findings = subject.compileAndLintWithContext(env, code) + assertFindings(findings) + } + + @Test + fun `does report when lambda parameter chain has all inlined function call`() { + val code = """ + import kotlinx.coroutines.MainScope + import kotlinx.coroutines.delay + import kotlinx.coroutines.launch + + inline fun inline(block: () -> R) = block() + + suspend fun bar() + { + runCatching { + inline { + inline { + delay(1000) + } + } + } + } + """.trimIndent() + + val findings = subject.compileAndLintWithContext(env, code) + assertFindings(findings, "delay") + } + @Test fun `does not report when lambda in inline function is passed as noinline`() { val code = """ @@ -252,10 +351,9 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { val findings = subject.compileAndLintWithContext(env, code) assertThat(findings).hasSize(1) - assertFindings(findings, "await()") + assertFindings(findings, "await") } - // Failing @Test fun `does report when suspending iterator is used`() { val code = """ @@ -275,7 +373,12 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "iterator.next()") + assertThat(findings).hasSize(1) + assertThat(findings[0].message).isEqualTo( + "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." + ) } @Test @@ -292,7 +395,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "suspendBlock()") + assertFindings(findings, "invoke") } @Test @@ -312,14 +415,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { val findings = subject.compileAndLintWithContext(env, code) assertThat(findings).hasSize(1) - assertFindings( - findings, - """ - suspendCancellableCoroutine { - it.resume(Unit) - } - """.trimIndent() - ) + assertFindings(findings, "suspendCancellableCoroutine") } @Test @@ -336,7 +432,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "invoke()") + assertFindings(findings, "invoke") } @Test @@ -354,7 +450,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { val findings = subject.compileAndLintWithContext(env, code) assertThat(findings).hasSize(1) - assertFindings(findings, "localFun()") + assertFindings(findings, "localFun") } @Test @@ -375,7 +471,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "C()()", "invoke()") + assertFindings(findings, "invoke", "invoke") } @Test @@ -415,7 +511,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "join()") + assertFindings(findings, "join") } @Test @@ -455,7 +551,7 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "await()") + assertFindings(findings, "await") } @Test @@ -514,16 +610,16 @@ class SuspendFunInsideRunCatchingSpec(private val env: KotlinCoreEnvironment) { """.trimIndent() val findings = subject.compileAndLintWithContext(env, code) - assertFindings(findings, "delay(1000)") + assertFindings(findings, "delay") } private fun assertFindings(findings: List, vararg funCallExpression: String) { assertThat(findings).hasSize(funCallExpression.size) assertThat(findings.map { it.message }).containsExactlyInAnyOrder( *funCallExpression.map { - "The suspend function call $it is inside `runCatching`. You should either " + - "use specific `try-catch` only catching exception that you are expecting or rethrow the " + - "`CancellationException` if already caught" + "The suspend function call $it called inside " + + "`runCatching`. You should either use specific `try-catch` only catching exception that you " + + "are expecting or rethrow the `CancellationException` if already caught." }.toTypedArray() ) }