From f019d0f9e24f1310928150f7e38434ef17c1a602 Mon Sep 17 00:00:00 2001 From: Denis Sh Date: Mon, 30 Jun 2025 02:00:53 +0300 Subject: [PATCH 1/2] Rules --- app/config/detekt/detekt.yml | 6 +- .../java/ru/otus/example/SampleViewModel.kt | 14 ++- .../kotlin/ru/otus/detekt/GlobalScopeRule.kt | 24 +++++- .../TopLevelCoroutineInSuspendFunRule.kt | 86 ++++++++++++++++++- .../ru/otus/detekt/GlobalScopeRuleTest.kt | 73 ++++++++++++++++ .../TopLevelCoroutineInSuspendFunRuleTest.kt | 72 ++++++++++++++++ 6 files changed, 268 insertions(+), 7 deletions(-) create mode 100644 detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt create mode 100644 detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRuleTest.kt diff --git a/app/config/detekt/detekt.yml b/app/config/detekt/detekt.yml index 24edd2b..7fd1b22 100644 --- a/app/config/detekt/detekt.yml +++ b/app/config/detekt/detekt.yml @@ -8,5 +8,9 @@ naming: ignoreAnnotated: - 'Composable' OtusRuleSet: - ComposeModifierRule: + ComposeModifierMissingRule: + active: true + GlobalScopeRule: + active: true + TopLevelCoroutineInSuspendFunRule: active: true diff --git a/app/src/main/java/ru/otus/example/SampleViewModel.kt b/app/src/main/java/ru/otus/example/SampleViewModel.kt index f990b10..864ef4f 100644 --- a/app/src/main/java/ru/otus/example/SampleViewModel.kt +++ b/app/src/main/java/ru/otus/example/SampleViewModel.kt @@ -1,11 +1,11 @@ package ru.otus.example +import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch class SampleViewModel : ViewModel() { @@ -16,8 +16,16 @@ class SampleViewModel : ViewModel() { } suspend fun coroutineLaunchInSuspendViolation() { - viewModelScope.launch { + val globalScope = GlobalScope + globalScope.launch { } + GlobalScope.async { + Log.d("TEST", "TEST") + } + coroutineScope { } + viewModelScope.launch { + println() + } } } 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..7d3df4f 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,24 @@ class GlobalScopeRule(config: Config) : Rule(config) { debt = Debt.FIVE_MINS ) - // TODO + override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) { + super.visitDotQualifiedExpression(expression) + val isGlobalScopeUsing = expression.firstChild.text == "GlobalScope" + if (isGlobalScopeUsing) { + val callingFun = expression.lastChild.firstChild + if (callingFun != null) { + val funText = callingFun.text + if (funText == "launch" || funText == "async") { + report( + CodeSmell( + issue = issue, + entity = Entity.from(expression), + message = "Avoid using GlobalScope " + + "More: https://elizarov.medium.com/the-reason-to-avoid-globalscope-835337445abc" + ) + ) + } + } + } + } } 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..a04129f 100644 --- a/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt +++ b/detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt @@ -1,18 +1,100 @@ 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 io.gitlab.arturbosch.detekt.rules.fqNameOrNull +import org.jetbrains.kotlin.KtNodeTypes +import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor +import org.jetbrains.kotlin.fir.analysis.forEachChildOfType +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtReferenceExpression +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull +import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedClassDescriptor +import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedSimpleFunctionDescriptor class TopLevelCoroutineInSuspendFunRule(config: Config) : Rule(config) { override val issue: Issue = Issue( id = javaClass.simpleName, severity = Severity.CodeSmell, description = "Avoid running top level coroutines inside suspend functions", - debt = Debt.FIVE_MINS + debt = Debt.FIVE_MINS, ) - // TODO + override fun visitNamedFunction(function: KtNamedFunction) { + super.visitNamedFunction(function) + val isSuspend = function.firstChild.text == "suspend" + if (isSuspend) { + function.bodyBlockExpression?.forEachChildOfType( + types = setOf(KtNodeTypes.DOT_QUALIFIED_EXPRESSION) + ) { dotQualifiedExpression -> + var reference = dotQualifiedExpression.firstChild + if (reference is KtCallExpression) { + reference = reference.firstChild + } + val callingFun = dotQualifiedExpression.lastChild.firstChild + if (bindingContext != BindingContext.EMPTY) { + val descriptor = bindingContext[ + BindingContext.REFERENCE_TARGET, + reference as KtReferenceExpression + ] + + // e.g. GlobalScope.launch { + val isCoroutineScopeClass = (descriptor as? DeserializedClassDescriptor) + ?.isCoroutineScope() + // e.g. scopeVal.launch { + val isCoroutineScopeValue = (descriptor as? LocalVariableDescriptor) + ?.isCoroutineScope() + // e.g. CoroutineScope(Dispatchers.IO).launch { + val isCoroutineScopeSimpleFunction = + (descriptor as? DeserializedSimpleFunctionDescriptor) + ?.isCoroutineScope() + + if (isCoroutineScopeClass == true || isCoroutineScopeValue == true || + isCoroutineScopeSimpleFunction == true + ) { + if (callingFun != null) { + val funText = callingFun.text + if (funText == "launch" || funText == "async") { + report( + CodeSmell( + issue = issue, + entity = Entity.from(dotQualifiedExpression), + message = "Avoid running top level coroutines inside suspend functions" + + "More: " + + "https://elizarov.medium.com/structured-concurrency-722d765aa952" + ) + ) + } + } + } + } + } + } + } + + private val coroutineScopeFqName = FqName("kotlinx.coroutines.CoroutineScope") + + private fun DeserializedSimpleFunctionDescriptor.isCoroutineScope() = + this.returnType?.fqNameOrNull() == FqName("kotlinx.coroutines.CoroutineScope") + + private fun DeserializedClassDescriptor.isCoroutineScope() = + this.fqNameOrNull() == coroutineScopeFqName || + this.typeConstructor.supertypes.any { type -> + type.fqNameOrNull() == coroutineScopeFqName + } + + private fun LocalVariableDescriptor.isCoroutineScope() = + this.fqNameOrNull() == coroutineScopeFqName || + this.type.constructor + .supertypes.any { type -> + type.fqNameOrNull() == coroutineScopeFqName + } } 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..02a4c51 --- /dev/null +++ b/detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt @@ -0,0 +1,73 @@ +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 GlobalScopeRuleTest(private val env: KotlinCoreEnvironment) { + private val rule = GlobalScopeRule(Config.empty) + + @Test + fun `should report GlobalScope launch`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + fun test() { + GlobalScope.launch { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `should report GlobalScope async`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + fun test() { + GlobalScope.async { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `should not report GlobalScope toString`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + fun test() { + GlobalScope.toString() + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 0 + } + + @Test + fun `should not report GlobalScope string`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + fun test() { + printLn("GlobalScope.launch") + } + """ + val findings = rule.compileAndLintWithContext(env, 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..c13979c --- /dev/null +++ b/detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRuleTest.kt @@ -0,0 +1,72 @@ +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 `should detect coroutine launch inside suspend function`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + import kotlinx.coroutines.CoroutineScope + import kotlinx.coroutines.Dispatchers + import kotlinx.coroutines.coroutineScope + import kotlinx.coroutines.supervisorScope + + suspend fun test() { + CoroutineScope(Dispatchers.Main).launch { } + GlobalScope.launch { } + GlobalScope.async { } + CoroutineScope.launch { } + + coroutineScope { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 4 + } + + @Test + fun `should detect coroutine launch using scope value`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + suspend fun test() { + println("suspend") + val g = GlobalScope + g.launch { } + + var g1 = GlobalScope + g1.launch { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 2 + } + + @Test + fun `should not detect coroutine launch in ordinary function`() { + val code = """ + import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.async + import kotlinx.coroutines.launch + + fun test3() { + GlobalScope.launch { } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + findings shouldHaveSize 0 + } +} From a338f5b7078e0493a70c6e0c43d3fe1efe18c4df Mon Sep 17 00:00:00 2001 From: Denis Sh Date: Mon, 30 Jun 2025 02:08:39 +0300 Subject: [PATCH 2/2] Rules --- app/src/main/java/ru/otus/example/SampleViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/ru/otus/example/SampleViewModel.kt b/app/src/main/java/ru/otus/example/SampleViewModel.kt index 864ef4f..e70653b 100644 --- a/app/src/main/java/ru/otus/example/SampleViewModel.kt +++ b/app/src/main/java/ru/otus/example/SampleViewModel.kt @@ -19,7 +19,7 @@ class SampleViewModel : ViewModel() { val globalScope = GlobalScope globalScope.launch { } GlobalScope.async { - Log.d("TEST", "TEST") + } coroutineScope {