Amir Sadykov - Completed Detekt Homework

detekt-rules/build.gradle.kts

@@ -15,6 +15,7 @@ dependencies {
     testImplementation(libs.detekt.test)
     testImplementation(libs.kotest)
     testImplementation(libs.jupiter)
+    testImplementation(kotlin("test"))
 }
 
 java {

detekt-rules/src/main/kotlin/ru/otus/detekt/GlobalScopeRule.kt

@@ -1,10 +1,14 @@
 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
+import org.jetbrains.kotlin.resolve.calls.util.getCalleeExpressionIfAny
 
 class GlobalScopeRule(config: Config) : Rule(config) {
     override val issue: Issue = Issue(
@@ -14,5 +18,18 @@ class GlobalScopeRule(config: Config) : Rule(config) {
         debt = Debt.FIVE_MINS
     )
 
-    // TODO
+    override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
+        super.visitDotQualifiedExpression(expression)
+        if (expression.receiverExpression.text.contains("GlobalScope") &&
+            expression.getCalleeExpressionIfAny()?.text in listOf("launch", "async")
+        ) {
+            report(
+                CodeSmell(
+                    issue = issue,
+                    entity = Entity.from(expression),
+                    message = "Avoid using GlobalScope for Coroutines"
+                )
+            )
+        }
+    }
 }

detekt-rules/src/main/kotlin/ru/otus/detekt/TopLevelCoroutineInSuspendFunRule.kt

@@ -1,10 +1,24 @@
 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.com.intellij.psi.PsiElement
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
+import org.jetbrains.kotlin.psi.KtFunction
+import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression
+import org.jetbrains.kotlin.psi.psiUtil.hasSuspendModifier
+import org.jetbrains.kotlin.psi.psiUtil.parents
+import org.jetbrains.kotlin.resolve.calls.util.getType
+import org.jetbrains.kotlin.types.typeUtil.supertypes
 
 class TopLevelCoroutineInSuspendFunRule(config: Config) : Rule(config) {
     override val issue: Issue = Issue(
@@ -14,5 +28,63 @@ class TopLevelCoroutineInSuspendFunRule(config: Config) : Rule(config) {
         debt = Debt.FIVE_MINS
     )
 
-    // TODO
+    override fun visitCallExpression(expression: KtCallExpression) {
+        super.visitCallExpression(expression)
+
+        if (expression.containingFunction()?.modifierList?.hasSuspendModifier() != true) return
+
+        val coroutineBuilders = setOf("launch", "async")
+        val callName = expression.getCallNameExpression()?.text
+        if (callName !in coroutineBuilders) return
+
+        var parent = expression.parent
+        while (parent != null) {
+            when (parent) {
+                is KtDotQualifiedExpression -> {
+                    val receiver = parent.receiverExpression
+                    val receiverType = receiver.getType(bindingContext)?.fqNameOrNull()
+                    if (
+                        receiverType == FqName("kotlinx.coroutines.CoroutineScope") ||
+                        receiver.text == "viewModelScope"
+                    ) {
+                        if (isInsideSuspendFunction(parent)) {
+                            reportCodeSmell(expression)
+                            return
+                        }
+                    }
+                }
+                is KtCallExpression -> {
+                    if (
+                        parent.getType(bindingContext)
+                            ?.supertypes()
+                            ?.any { it.fqNameOrNull() == FqName("kotlinx.coroutines.CoroutineScope") } == true
+                    ) {
+                        if (isInsideSuspendFunction(parent)) {
+                            reportCodeSmell(expression)
+                            return
+                        }
+                    }
+                }
+            }
+            parent = parent.parent
+        }
+    }
+
+    private fun KtCallExpression.containingFunction(): KtFunction? =
+        this.parents.filterIsInstance<KtFunction>().firstOrNull()
+
+    private fun isInsideSuspendFunction(element: PsiElement): Boolean {
+        return element.parents.filterIsInstance<KtNamedFunction>().firstOrNull()?.modifierList?.hasSuspendModifier() == true
+    }
+
+    private fun reportCodeSmell(expression: KtCallExpression) {
+        report(
+            CodeSmell(
+                issue = issue,
+                entity = Entity.from(expression),
+                message = "Avoid running top level coroutines inside suspend functions"
+            )
+        )
+    }
+
 }

detekt-rules/src/test/kotlin/ru/otus/detekt/GlobalScopeRuleTest.kt

@@ -0,0 +1,63 @@
+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)
+
+    private fun assertFindings(code: String, expectedSize: Int) {
+        val findings = rule.compileAndLintWithContext(env, code.trimIndent())
+        findings shouldHaveSize expectedSize
+    }
+
+    @Test
+    fun `should report GlobalScope launch`() = assertFindings("""
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.launch
+        fun loadInfo() {
+            GlobalScope.launch { }
+        }
+    """, 1)
+
+    @Test
+    fun `should report GlobalScope async`() = assertFindings("""
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.async
+        fun loadInfo() {
+            GlobalScope.async { }
+        }
+    """, 1)
+
+    @Test
+    fun `should not report 'val scope = GlobalScope'`() = assertFindings("""
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.GlobalScope
+        fun loadInfo(scope: CoroutineScope) = Unit
+        fun fetchData() {
+            val scope = GlobalScope
+            loadInfo(scope)
+        }
+    """, 0)
+
+    @Test
+    fun `should not report CoroutineScope launch`() = assertFindings("""
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.launch
+        fun example(scope: CoroutineScope) {
+            scope.launch { println("This is a coroutine") }
+        }
+    """, 0)
+
+    @Test
+    fun `should not report non-coroutine code`() = assertFindings("""
+        fun example() {
+            println("This is not a coroutine")
+        }
+    """, 0)
+}

detekt-rules/src/test/kotlin/ru/otus/detekt/TopLevelCoroutineRuleTest.kt

@@ -0,0 +1,158 @@
+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 TopLevelCoroutineRuleTest(private val env: KotlinCoreEnvironment) {
+    private val rule = TopLevelCoroutineInSuspendFunRule(Config.empty)
+
+    private fun assertFindings(
+        code: String,
+        expectedSize: Int
+    ) {
+        val findings = rule.compileAndLintWithContext(env, code.trimIndent())
+        findings shouldHaveSize expectedSize
+    }
+
+    @Test
+    fun `reports launch in suspend fun`() = assertFindings(
+        """
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        suspend fun loadInfo() {
+            CoroutineScope(Dispatchers.Default).launch { }
+        }
+        """,
+        1
+    )
+
+    @Test
+    fun `reports launch and async in suspend fun with apply`() = assertFindings(
+        """
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        suspend fun loadInfo() {
+            CoroutineScope(Dispatchers.Default).apply {
+                launch { }
+                async { }
+            }
+        }
+        """,
+        2
+    )
+
+    @Test
+    fun `reports launch in suspend fun with val scope`() = assertFindings(
+        """
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        suspend fun loadInfo() {
+            val scope = CoroutineScope(Dispatchers.Default)
+            scope.launch { }
+        }
+        """,
+        1
+    )
+
+    @Test
+    fun `reports launch in suspend fun with viewModelScope`() = assertFindings(
+        """
+        import androidx.lifecycle.viewModelScope
+        import kotlinx.coroutines.launch
+        suspend fun loadInfo() {
+            viewModelScope.launch { }
+        }
+        """,
+        1
+    )
+
+    @Test
+    fun `reports launch in suspend fun with scope from parameter`() = assertFindings(
+        """
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        suspend fun loadInfo(scope: CoroutineScope) {
+            scope.launch { }
+        }
+        """,
+        1
+    )
+
+    @Test
+    fun `reports async in suspend fun`() = assertFindings(
+        """
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        suspend fun loadInfo() {
+            CoroutineScope(Dispatchers.Default).async { }
+        }
+        """,
+        1
+    )
+
+    @Test
+    fun `no report launch in coroutineScope`() = assertFindings(
+        """
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        import kotlinx.coroutines.coroutineScope
+        suspend fun loadInfo() {
+            coroutineScope {
+                launch {}
+                async {}
+            }
+        }
+        """,
+        0
+    )
+
+    @Test
+    fun `no report launch in supervisor scope`() = assertFindings(
+        """
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        import kotlinx.coroutines.supervisorScope
+        suspend fun loadInfo() {
+            supervisorScope {
+                launch {}
+                async {}
+            }
+        }
+        """,
+        0
+    )
+
+    @Test
+    fun `no reports launch in not suspend fun`() = assertFindings(
+        """
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        fun loadInfo() {
+            CoroutineScope(Dispatchers.Default).launch { }
+        }
+        """,
+        0
+    )
+
+    @Test
+    fun `no reports async in not suspend fun`() = assertFindings(
+        """
+        import kotlinx.coroutines.GlobalScope
+        import kotlinx.coroutines.CoroutineScope
+        import kotlinx.coroutines.Dispatchers
+        fun loadInfo() {
+            CoroutineScope(Dispatchers.Default).async { }
+        }
+        """,
+        0
+    )
+
+}