Repository: analysis-dev/diktat (kotlin)

Issue:
Avoid using runBlocking in asynchronous code
https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/run-blocking.html

Some people occasionally use runBlocking coroutine in asynchronous code. We need to have a warning for it, because it leads to a bug.

Pull request:
### What's done:
   Implemented rule, added tests and documentation

## Actions checklist
* [x] Implemented Rule, added Warnings
* [x] Added tests on checks
* [x] Updated diktat-analysis.yml
* [x] Updated available-rules.md
* [x] closes #691

Patch:
diff --git a/diktat-analysis.yml b/diktat-analysis.yml
index 0bfeaabaae..969a9f8f50 100644
--- a/diktat-analysis.yml
+++ b/diktat-analysis.yml
@@ -353,6 +353,9 @@
 # Checks that function use default values, instead overloading
 - name: WRONG_OVERLOADING_FUNCTION_ARGUMENTS
   enabled: true
+# Checks that using runBlocking inside async block code
+- name: RUN_BLOCKING_INSIDE_ASYNC
+  enabled: true
 # Checks that property in constructor doesn't contains comment
 - name: KDOC_NO_CONSTRUCTOR_PROPERTY
   enabled: true
diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt
index 277a04125d..ffe5893e2a 100644
--- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt
+++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt
@@ -201,6 +201,8 @@ public object WarningNames {
     public const val WRONG_OVERLOADING_FUNCTION_ARGUMENTS: String =
             "WRONG_OVERLOADING_FUNCTION_ARGUMENTS"
 
+    public const val RUN_BLOCKING_INSIDE_ASYNC: String = "RUN_BLOCKING_INSIDE_ASYNC"
+
     public const val SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY: String =
             "SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY"
 
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
index 3fb16abd45..6725faede4 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
@@ -132,6 +132,7 @@ enum class Warnings(
     TOO_MANY_PARAMETERS(false, "5.2.2", "function has too many parameters"),
     NESTED_BLOCK(false, "5.1.2", "function has too many nested blocks and should be simplified"),
     WRONG_OVERLOADING_FUNCTION_ARGUMENTS(false, "5.2.3", "use default argument instead of function overloading"),
+    RUN_BLOCKING_INSIDE_ASYNC(false, "5.2.4", "avoid using runBlocking in asynchronous code"),
 
     // ======== chapter 6 ========
     SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY(true, "6.1.1", "if a class has single constructor, it should be converted to a primary constructor"),
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AsyncAndSyncRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AsyncAndSyncRule.kt
new file mode 100644
index 0000000000..7f7f08f7c0
--- /dev/null
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AsyncAndSyncRule.kt
@@ -0,0 +1,48 @@
+package org.cqfn.diktat.ruleset.rules
+
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.EmitType
+import org.cqfn.diktat.ruleset.constants.Warnings.RUN_BLOCKING_INSIDE_ASYNC
+import org.cqfn.diktat.ruleset.utils.hasChildOfType
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
+import com.pinterest.ktlint.core.ast.ElementType.FUN
+import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_ARGUMENT
+import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
+import com.pinterest.ktlint.core.ast.parent
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.psi.KtFunction
+import org.jetbrains.kotlin.psi.psiUtil.hasSuspendModifier
+
+/**
+ * This rule finds if using runBlocking in asynchronous code
+ */
+class AsyncAndSyncRule(private val configRules: List<RulesConfig>) : Rule("sync-in-async") {
+    private val asyncList = listOf("async", "launch")
+    private var isFixMode: Boolean = false
+    private lateinit var emitWarn: EmitType
+
+    override fun visit(node: ASTNode,
+                       autoCorrect: Boolean,
+                       emit: EmitType) {
+        emitWarn = emit
+        isFixMode = autoCorrect
+
+        if (node.isRunBlocking()) {
+            checkRunBlocking(node)
+        }
+    }
+
+    private fun checkRunBlocking(node: ASTNode) {
+        node.parent({it.isAsync() || it.isSuspend()})?.let {
+            RUN_BLOCKING_INSIDE_ASYNC.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node)
+        }
+    }
+
+    private fun ASTNode.isAsync() = this.elementType == CALL_EXPRESSION && this.findChildByType(REFERENCE_EXPRESSION)?.text in asyncList
+
+    private fun ASTNode.isSuspend() = this.elementType == FUN && (this.psi as KtFunction).modifierList?.hasSuspendModifier() ?: false
+
+    private fun ASTNode.isRunBlocking() = this.elementType == REFERENCE_EXPRESSION && this.text == "runBlocking" && this.treeParent.hasChildOfType(LAMBDA_ARGUMENT)
+}
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
index 7640a6f678..f05a80f906 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
@@ -137,6 +137,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
             ::FunctionArgumentsSize,
             ::BlankLinesRule,
             ::FileSize,
+            ::AsyncAndSyncRule,
             ::NullableTypeRule,
             ::NullChecksRule,
             ::ImmutableValNoVarRule,
diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
index dcec48653b..281df43f9b 100644
--- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
+++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
@@ -353,6 +353,9 @@
 # Checks that function use default values, instead overloading
 - name: WRONG_OVERLOADING_FUNCTION_ARGUMENTS
   enabled: true
+# Checks that using runBlocking inside async block code
+- name: RUN_BLOCKING_INSIDE_ASYNC
+  enabled: true
 # Checks that property in constructor doesn't contains comment
 - name: KDOC_NO_CONSTRUCTOR_PROPERTY
   enabled: true
diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml
index bd942dc57f..cdff6358f4 100644
--- a/diktat-rules/src/main/resources/diktat-analysis.yml
+++ b/diktat-rules/src/main/resources/diktat-analysis.yml
@@ -350,6 +350,9 @@
 # Checks that function use default values, instead overloading
 - name: WRONG_OVERLOADING_FUNCTION_ARGUMENTS
   enabled: true
+# Checks that using runBlocking inside async block code
+- name: RUN_BLOCKING_INSIDE_ASYNC
+  enabled: true
 # Checks that property in constructor doesn't contain comment
 - name: KDOC_NO_CONSTRUCTOR_PROPERTY
   enabled: true
diff --git a/info/available-rules.md b/info/available-rules.md
index 68ad3c0ed3..f2ce918049 100644
--- a/info/available-rules.md
+++ b/info/available-rules.md
@@ -96,6 +96,7 @@
 |    5  |     5.2.1  |  LAMBDA_IS_NOT_LAST_PARAMETER            |  Checks that lambda inside function parameters isn't in the end                                                                                                                |  no   |  no |
 |    5  |     5.2.2  |  TOO_MANY_PARAMETERS                     |  Check: if function contains more parameters than allowed                                                                                                                      |   no  |  maxParameterListSize |
 |    5  |     5.2.3  |  WRONG_OVERLOADING_FUNCTION_ARGUMENTS    |  Check: function has overloading instead use default arguments                                                                                                                 |   no  |  no |
+|    5  |     5.2.4  |  RUN_BLOCKING_INSIDE_ASYNC               |  Check: using runBlocking inside async block code                                                                                                                              |   no  |  no | - |
 |    6  |     6.1.1  |  SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY    |  Check: warns if there is only one secondary constructor in a class<br>Fix: converts it to a primary constructor  |  yes  |  no  |   Support more complicated logic of constructor conversion |
 |    6  |     6.1.2  |  USE_DATA_CLASS                          |  Check: if class can be made as data class                                                                                                                                     |  no  |  no  |  yes |
 |    6  |     6.1.3  |  EMPTY_PRIMARY_CONSTRUCTOR               |  Check: if there is empty primary constructor                                                                                                                                  |  yes |  no  |  yes |
diff --git a/info/guide/guide-chapter-5.md b/info/guide/guide-chapter-5.md
index 294418dce4..6eef6ca3a7 100644
--- a/info/guide/guide-chapter-5.md
+++ b/info/guide/guide-chapter-5.md
@@ -132,3 +132,14 @@ private fun foo() {
      // ...
  }
 ``` 
+#### <a name="r5.2.4"></a> 5.2.4 Synchronizing code inside asynchronous code
+Try to avoid using runBlocking in asynchronous code
+
+**Invalid example**:
+```kotlin
+GlobalScope.async {
+    runBlocking {
+        count++
+    }   
+}
+```
\ No newline at end of file


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/AsyncAndSyncRuleTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/AsyncAndSyncRuleTest.kt
new file mode 100644
index 0000000000..8d904c8ea1
--- /dev/null
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/AsyncAndSyncRuleTest.kt
@@ -0,0 +1,74 @@
+package org.cqfn.diktat.ruleset.chapter5
+
+import org.cqfn.diktat.ruleset.constants.Warnings.RUN_BLOCKING_INSIDE_ASYNC
+import org.cqfn.diktat.ruleset.rules.AsyncAndSyncRule
+import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
+import org.cqfn.diktat.util.LintTestBase
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class AsyncAndSyncRuleTest : LintTestBase(::AsyncAndSyncRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:sync-in-async"
+
+    @Test
+    @Tag(WarningNames.RUN_BLOCKING_INSIDE_ASYNC)
+    fun `test wrong case`() {
+        lintMethod(
+            """
+                    |fun foo() {
+                    |   GlobalScope.launch {
+                    |       c.addAndGet(i)
+                    |   }
+                    |   
+                    |   GlobalScope.async {
+                    |       n
+                    |   }
+                    |   
+                    |   GlobalScope.async {
+                    |       runBlocking {
+                    |           
+                    |       }
+                    |   }
+                    |}
+                    |
+                    |suspend fun foo() {
+                    |   runBlocking {
+                    |       delay(2000)
+                    |   }
+                    |}
+                    |
+            """.trimMargin(),
+            LintError(11, 8, ruleId, "${RUN_BLOCKING_INSIDE_ASYNC.warnText()} runBlocking", false),
+            LintError(18, 4, ruleId, "${RUN_BLOCKING_INSIDE_ASYNC.warnText()} runBlocking", false)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.RUN_BLOCKING_INSIDE_ASYNC)
+    fun `test dot qualified expression case`() {
+        lintMethod(
+            """
+                    |fun foo() {
+                    |   GlobalScope.async {
+                    |       node.runBlocking()
+                    |       runBlocking {
+                    |           n++
+                    |       }
+                    |   }
+                    |}
+                    |
+                    |fun goo() {
+                    |   runBlocking {
+                    |       GlobalScope.async {
+                    |           n++
+                    |       }
+                    |   }
+                    |}
+            """.trimMargin(),
+            LintError(4, 8, ruleId, "${RUN_BLOCKING_INSIDE_ASYNC.warnText()} runBlocking", false)
+        )
+    }
+}