Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Thread.sleep for block expression #5699

Merged
merged 2 commits into from
Feb 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,20 @@ import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import org.jetbrains.kotlin.builtins.isSuspendFunctionType
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtFunctionLiteral
import org.jetbrains.kotlin.psi.KtLambdaArgument
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.hasSuspendModifier
import org.jetbrains.kotlin.psi.KtValueArgument
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypes
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.fqNameOrNull

Expand Down Expand Up @@ -48,44 +55,91 @@ class SleepInsteadOfDelay(config: Config = Config.empty) : Rule(config) {
Debt.FIVE_MINS
)

override fun visitNamedFunction(function: KtNamedFunction) {
if (function.modifierList?.hasSuspendModifier() == true) {
function.checkDescendants(SUSPEND_FUN_MESSAGE)
}
super.visitNamedFunction(function)
override fun visitCallExpression(expression: KtCallExpression) {
super.visitCallExpression(expression)
checkAndReport(expression)
}

override fun visitCallableReferenceExpression(expression: KtCallableReferenceExpression) {
super.visitCallableReferenceExpression(expression)
checkAndReport(expression)
}

override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
val fqName = expression.getResolvedCall(bindingContext)
?.resultingDescriptor
?.fqNameOrNull()
?.asString()
if (fqName in COROUTINE_NAMES) {
expression.checkDescendants(COROUTINE_MESSAGE)
private fun checkAndReport(expression: KtExpression) {
if (expression.isThreadSleepFunction() && expression.isInSuspendBlock()) {
report(CodeSmell(issue, Entity.from(expression), SUSPEND_FUN_MESSAGE))
}
super.visitQualifiedExpression(expression)
}

private fun PsiElement.checkDescendants(message: String) {
forEachDescendantOfType<KtCallExpression> { it.verifyExpression(message) }
private fun KtExpression.isThreadSleepFunction(): Boolean {
fun KtCallableReferenceExpression.isSleepCallableRef(): Boolean {
return if (this.parent is KtValueArgument) {
// Only checking if this is used as for invocation
this.callableReference.isThreadSleepFunction()
} else {
false
}
}
return if (this is KtCallableReferenceExpression) {
this.isSleepCallableRef()
} else {
getResolvedCall(bindingContext)
?.resultingDescriptor
?.fqNameOrNull()
?.asString() == FQ_NAME
}
}

private fun KtExpression.verifyExpression(message: String) {
val fqName = getResolvedCall(bindingContext)
?.resultingDescriptor
?.fqNameOrNull()
?.asString()
if (fqName == FQ_NAME) {
report(CodeSmell(issue, Entity.from(this), message))
private fun PsiElement.isSuspendingBlock(): Boolean {
return when (this) {
is KtFunctionLiteral -> {
val psiParent = this.parent.parent
if (psiParent is KtLambdaArgument) {
psiParent.isSuspendAllowedLambdaArgument()
} else {
this.isSuspendScope()
}
}
is KtNamedFunction -> {
this.isSuspendScope()
}
else -> {
false
}
}
}

@Suppress("ReturnCount")
private fun KtLambdaArgument.isSuspendAllowedLambdaArgument(): Boolean {
val callExpression = this.getParentOfType<KtCallExpression>(true)
val callDescriptor = callExpression?.getResolvedCall(bindingContext) ?: return false
val functionDescriptor = callDescriptor.resultingDescriptor as? FunctionDescriptor ?: return false
val valueParameterDescriptor = callDescriptor.getParameterForArgument(this) ?: return false

return (
functionDescriptor.isInline &&
(valueParameterDescriptor.isNoinline.not() && valueParameterDescriptor.isCrossinline.not())
) || valueParameterDescriptor.returnType?.isSuspendFunctionType == true
}

private fun PsiElement.isSuspendScope(): Boolean {
val functionDescriptor = bindingContext[BindingContext.FUNCTION, this] ?: return false
return functionDescriptor.isSuspend
}

private fun KtExpression.isInSuspendBlock(): Boolean {
val containingBlockExpression =
this.getParentOfTypes<PsiElement>(
true,
KtNamedFunction::class.java,
KtFunctionLiteral::class.java
) ?: return false
return containingBlockExpression.isSuspendingBlock()
}

companion object {
private const val SUSPEND_FUN_MESSAGE =
"This use of Thread.sleep() inside a suspend function should be replaced by delay()."
private const val COROUTINE_MESSAGE =
"This use of Thread.sleep() inside a coroutine should be replaced by delay()."
private const val FQ_NAME = "java.lang.Thread.sleep"
private val COROUTINE_NAMES = listOf("kotlinx.coroutines.launch", "kotlinx.coroutines.async")
}
}