-
-
Notifications
You must be signed in to change notification settings - Fork 755
/
SuspendFunSwallowedCancellation.kt
179 lines (167 loc) · 7.02 KB
/
SuspendFunSwallowedCancellation.kt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
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
*
* See reference, [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 = "`runCatching` does not propagate `CancellationException`, don't use it with `suspend` lambda " +
"blocks.",
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
val shouldTraverseInside: (PsiElement) -> Boolean = {
expression == it || shouldTraverseInside(it, bindingContext)
}
expression.forEachDescendantOfType<KtCallExpression>(shouldTraverseInside) { 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>(shouldTraverseInside) { descendant ->
if (descendant.hasSuspendingIterators()) {
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 {
return when (psiElement) {
is KtCallExpression -> {
val callableDescriptor =
(psiElement.getResolvedCall(bindingContext)?.resultingDescriptor as? FunctionDescriptor)
?: return false
callableDescriptor.fqNameSafe != RUN_CATCHING_FQ && callableDescriptor.isInline
}
is KtValueArgument -> {
val callExpression = psiElement.getParentOfType<KtCallExpression>(true) ?: return false
val valueParameterDescriptor =
callExpression.getResolvedCall(bindingContext)?.getParameterForArgument(psiElement) ?: return false
valueParameterDescriptor.isCrossinline.not() && valueParameterDescriptor.isNoinline.not()
}
else -> true
}
}
private fun KtForExpression.hasSuspendingIterators(): 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")
}
}