From 373b609cb8b112e142ca6055707297d5fb5c002e Mon Sep 17 00:00:00 2001 From: Denis Kumar Date: Mon, 18 Jan 2021 11:27:03 +0300 Subject: [PATCH] AsyncAndSyncRule (#707) * AsyncAndSyncRule ### What's done: Implemented rule, added tests and documentation --- diktat-analysis.yml | 3 + .../src/main/kotlin/generated/WarningNames.kt | 2 + .../cqfn/diktat/ruleset/constants/Warnings.kt | 1 + .../diktat/ruleset/rules/AsyncAndSyncRule.kt | 48 ++++++++++++ .../ruleset/rules/DiktatRuleSetProvider.kt | 1 + .../main/resources/diktat-analysis-huawei.yml | 3 + .../src/main/resources/diktat-analysis.yml | 3 + .../ruleset/chapter5/AsyncAndSyncRuleTest.kt | 74 +++++++++++++++++++ info/available-rules.md | 1 + info/guide/guide-chapter-5.md | 11 +++ 10 files changed, 147 insertions(+) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AsyncAndSyncRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/AsyncAndSyncRuleTest.kt 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) : 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/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) + ) + } +} 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
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() { // ... } ``` +#### 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