diff --git a/detekt-rules/src/main/kotlin/ru/otus/detekt/GlobalScopeRule.kt b/detekt-rules/src/main/kotlin/ru/otus/detekt/GlobalScopeRule.kt index 8258aec..68e7dc6 100644 --- a/detekt-rules/src/main/kotlin/ru/otus/detekt/GlobalScopeRule.kt +++ b/detekt-rules/src/main/kotlin/ru/otus/detekt/GlobalScopeRule.kt @@ -1,10 +1,13 @@ package ru.otus.detekt +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 org.jetbrains.kotlin.psi.KtDotQualifiedExpression class GlobalScopeRule(config: Config) : Rule(config) { override val issue: Issue = Issue( @@ -14,5 +17,18 @@ class GlobalScopeRule(config: Config) : Rule(config) { debt = Debt.FIVE_MINS ) - // TODO + override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) { + super.visitDotQualifiedExpression(expression) + val receiverText = expression.receiverExpression.text + val selectorText = expression.selectorExpression?.text + if (receiverText == GLOBAL_SCOPE && (selectorText?.startsWith(LAUNCH) == true || selectorText?.startsWith(ASYNC) == true)) { + report(CodeSmell(issue, Entity.from(expression), issue.description)) + } + } + + companion object { + private const val GLOBAL_SCOPE = "GlobalScope" + private const val LAUNCH = "launch" + private const val ASYNC = "async" + } } diff --git a/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt b/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt index abd3717..500f8bd 100644 --- a/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt +++ b/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt @@ -1,10 +1,22 @@ package ru.otus.detekt +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 org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtLambdaArgument +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe +import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.types.typeUtil.supertypes class TopLevelCoroutineInSuspendFunRule(config: Config) : Rule(config) { override val issue: Issue = Issue( @@ -14,5 +26,68 @@ class TopLevelCoroutineInSuspendFunRule(config: Config) : Rule(config) { debt = Debt.FIVE_MINS ) - // TODO + override fun visitCallExpression(expression: KtCallExpression) { + super.visitCallExpression(expression) + val resolvedCall = expression.getResolvedCall(bindingContext) ?: return + val descriptor = resolvedCall.resultingDescriptor + val name = descriptor.name.asString() + if (name != LAUNCH && name != ASYNC) return + val receiverType = resolvedCall.dispatchReceiver?.type ?: resolvedCall.extensionReceiver?.type ?: return + if (!isCoroutineScope(receiverType)) return + if (!isInsideSuspendFunction(expression)) return + if (isInsideCoroutineScopeBuilder(expression)) { + val parent = expression.parent + if (parent is KtDotQualifiedExpression) { + val receiver = parent.receiverExpression + if (receiver.text != THIS_MODIFIER) { + report(CodeSmell(issue, Entity.from(expression), issue.description)) + } + } + return + } + report(CodeSmell(issue, Entity.from(expression), issue.description)) + } + + private fun isCoroutineScope(type: KotlinType): Boolean { + if (type.constructor.declarationDescriptor?.fqNameSafe?.asString() == COROUTINE_SCOPE_FQ_NAME) return true + return type.supertypes().any { it.constructor.declarationDescriptor?.fqNameSafe?.asString() == COROUTINE_SCOPE_FQ_NAME } + } + + private fun isInsideSuspendFunction(element: KtElement): Boolean { + var current: KtElement? = element + while (current != null) { + if (current is KtNamedFunction) { + if (current.hasModifier(KtTokens.SUSPEND_KEYWORD)) return true + } + current = current.parent as? KtElement + } + return false + } + + private fun isInsideCoroutineScopeBuilder(element: KtElement): Boolean { + var current: KtElement? = element + while (current != null) { + if (current is KtLambdaArgument) { + val call = current.parent as? KtCallExpression + if (call != null) { + val resolved = call.getResolvedCall(bindingContext) + val fqName = resolved?.resultingDescriptor?.fqNameSafe?.asString() + if (fqName == COROUTINE_SCOPE_BUILDER || fqName == SUPERVISOR_SCOPE_BUILDER) { + return true + } + } + } + current = current.parent as? KtElement + } + return false + } + + companion object { + private const val LAUNCH = "launch" + private const val ASYNC = "async" + private const val THIS_MODIFIER = "this" + private const val COROUTINE_SCOPE_FQ_NAME = "kotlinx.coroutines.CoroutineScope" + private const val COROUTINE_SCOPE_BUILDER = "kotlinx.coroutines.coroutineScope" + private const val SUPERVISOR_SCOPE_BUILDER = "kotlinx.coroutines.supervisorScope" + } } diff --git a/detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt b/detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt new file mode 100644 index 0000000..82f03fe --- /dev/null +++ b/detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt @@ -0,0 +1,54 @@ +package ru.otus.detekt + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.test.lint +import io.kotest.matchers.collections.shouldHaveSize +import org.junit.jupiter.api.Test + +class GlobalScopeRuleTest { + private val rule = GlobalScopeRule(Config.empty) + + @Test + fun `reports GlobalScope launch`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.launch + + fun test() { + GlobalScope.launch { } + } + """ + val findings = rule.lint(code) + findings shouldHaveSize 1 + } + + @Test + fun `reports GlobalScope async`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + + fun test() { + GlobalScope.async { } + } + """ + val findings = rule.lint(code) + findings shouldHaveSize 1 + } + + @Test + fun `does not report regular scope launch`() { + val code = """ + import kotlinx.coroutines.CoroutineScope + import kotlinx.coroutines.Dispatchers + import kotlinx.coroutines.launch + + val scope = CoroutineScope(Dispatchers.Default) + fun test() { + scope.launch { } + } + """ + val findings = rule.lint(code) + findings shouldHaveSize 0 + } +} diff --git a/detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRuleTest.kt b/detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRuleTest.kt new file mode 100644 index 0000000..d2baac8 --- /dev/null +++ b/detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRuleTest.kt @@ -0,0 +1,93 @@ +package ru.otus.detekt + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest +import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext +import io.kotest.matchers.collections.shouldHaveSize +import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment +import org.junit.jupiter.api.Test + +@KotlinCoreEnvironmentTest +internal class TopLevelCoroutineInSuspendFunRuleTest(private val env: KotlinCoreEnvironment) { + private val rule = TopLevelCoroutineInSuspendFunRule(Config.empty) + + @Test + fun `reports GlobalScope launch in suspend fun`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.launch + + suspend fun test() { + GlobalScope.launch { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports custom scope launch in suspend fun`() { + val code = """ + import kotlinx.coroutines.CoroutineScope + import kotlinx.coroutines.Dispatchers + import kotlinx.coroutines.launch + + val scope = CoroutineScope(Dispatchers.Default) + + suspend fun test() { + scope.launch { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports explicit scope launch even inside builder`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.coroutineScope + import kotlinx.coroutines.launch + + suspend fun test() { + coroutineScope { + GlobalScope.launch { } + } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `does not report launch in coroutineScope builder`() { + val code = """ + import kotlinx.coroutines.coroutineScope + import kotlinx.coroutines.launch + + suspend fun test() { + coroutineScope { + launch { } + } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 0 + } + + @Test + fun `does not report launch in supervisorScope builder`() { + val code = """ + import kotlinx.coroutines.supervisorScope + import kotlinx.coroutines.launch + + suspend fun test() { + supervisorScope { + launch { } + } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 0 + } +}