From 727aff39dab163a2f31889e506dcfe0160bbf1fe Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 13:18:39 +0300 Subject: [PATCH 1/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../ruleset/rules/chapter3/EmptyBlock.kt | 33 ++++++++++++++++--- .../ruleset/chapter3/EmptyBlockWarnTest.kt | 17 ++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt index f0405440c5..c3e08dfeb6 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt @@ -9,13 +9,19 @@ import org.cqfn.diktat.ruleset.utils.* import com.pinterest.ktlint.core.ast.ElementType import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_LITERAL import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER +import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.LPAR import com.pinterest.ktlint.core.ast.ElementType.RBRACE +import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl +import org.jetbrains.kotlin.psi.psiUtil.parents /** * Rule that checks if empty code blocks (`{ }`) are used and checks their formatting. @@ -28,6 +34,9 @@ class EmptyBlock(configRules: List) : DiktatRule( val configuration = EmptyBlockStyleConfiguration( configRules.getRuleConfig(EMPTY_BLOCK_STRUCTURE_ERROR)?.configuration ?: emptyMap() ) + if (node.elementType == FILE) { + println(node.prettyPrint()) + } searchNode(node, configuration) } @@ -41,7 +50,7 @@ class EmptyBlock(configRules: List) : DiktatRule( @Suppress("UnsafeCallOnNullableType", "TOO_LONG_FUNCTION") private fun checkEmptyBlock(node: ASTNode, configuration: EmptyBlockStyleConfiguration) { - if (node.treeParent.isOverridden() || isAnonymousSamClass(node)) { + if (node.treeParent.isOverridden() || isAnonymousSamClass(node) || isLambdaUsedAsFunction(node)) { return } if (node.isBlockEmpty()) { @@ -82,11 +91,11 @@ class EmptyBlock(configRules: List) : DiktatRule( } } - @Suppress("UnsafeCallOnNullableType", "WRONG_WHITESPACE") - private fun isAnonymousSamClass(node: ASTNode) : Boolean = + @Suppress("UnsafeCallOnNullableType") + private fun isAnonymousSamClass(node: ASTNode): Boolean = if (node.elementType == FUNCTION_LITERAL && node.hasParent(CALL_EXPRESSION)) { - // We are checking identifier because it is not class in AST - // , SAM conversions are indistinguishable from lambdas. + // We are checking identifier because it is not class in AST, + // SAM conversions are indistinguishable from lambdas. // So we just verify that identifier is in PascalCase val valueArgument = node.findParentNodeWithSpecificType(CALL_EXPRESSION)!! valueArgument.findLeafWithSpecificType(IDENTIFIER)?.text?.isPascalCase() ?: false @@ -94,6 +103,20 @@ class EmptyBlock(configRules: List) : DiktatRule( false } + private fun isLambdaUsedAsFunction(node: ASTNode): Boolean { + val parents = node.parents() + if (parents.any { it.elementType == CALL_EXPRESSION }) { + val callExpression = parents.find { it.elementType == CALL_EXPRESSION }!! + // excepting cases like list.map { } + return callExpression.treeParent.elementType != DOT_QUALIFIED_EXPRESSION + } else if (parents.any { it.elementType == LAMBDA_EXPRESSION }) { + val lambdaExpression = parents.find { it.elementType == LAMBDA_EXPRESSION }!! + // cases like A({}). Here Lambda expression is used as a value parameter. + return lambdaExpression.treeParent.elementType == VALUE_PARAMETER + } + return false + } + /** * [RuleConfiguration] for empty blocks formatting */ diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt index 66e82eb25e..25fea72ca0 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt @@ -204,4 +204,21 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) { rulesConfigList = rulesConfigListEmptyBlockExist ) } + + @Test + @Tag(WarningNames.EMPTY_BLOCK_STRUCTURE_ERROR) + fun `should not trigger on empty lambdas as a functions`() { + lintMethod( + """ + |fun foo(bar: () -> Unit = {}) + | + |class Some { + | fun bar() { + | A({}) + | } + |} + """.trimMargin(), + rulesConfigList = rulesConfigListEmptyBlockExist + ) + } } From 71aa40695bddcaee393bb10ef79628c432dbf0d3 Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 13:21:59 +0300 Subject: [PATCH 2/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt index 25fea72ca0..acce1ce7ad 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt @@ -209,7 +209,7 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) { @Tag(WarningNames.EMPTY_BLOCK_STRUCTURE_ERROR) fun `should not trigger on empty lambdas as a functions`() { lintMethod( - """ + """ |fun foo(bar: () -> Unit = {}) | |class Some { @@ -218,7 +218,7 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) { | } |} """.trimMargin(), - rulesConfigList = rulesConfigListEmptyBlockExist + rulesConfigList = rulesConfigListEmptyBlockExist ) } } From cff24e6f4ac279e61bb43ce2910e0ee4947d1385 Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 13:26:37 +0300 Subject: [PATCH 3/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt index c3e08dfeb6..b5c411b299 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt @@ -103,6 +103,7 @@ class EmptyBlock(configRules: List) : DiktatRule( false } + @Suppress("UnsafeCallOnNullableType") private fun isLambdaUsedAsFunction(node: ASTNode): Boolean { val parents = node.parents() if (parents.any { it.elementType == CALL_EXPRESSION }) { From 430b0bdfe2496c35c11eb3aad4c7f5f94214052c Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 14:54:29 +0300 Subject: [PATCH 4/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../ruleset/rules/chapter3/EmptyBlock.kt | 26 ++++++++++--------- info/guide/guide-chapter-3.md | 6 +++-- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt index b5c411b299..51c5c5baf0 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt @@ -34,9 +34,6 @@ class EmptyBlock(configRules: List) : DiktatRule( val configuration = EmptyBlockStyleConfiguration( configRules.getRuleConfig(EMPTY_BLOCK_STRUCTURE_ERROR)?.configuration ?: emptyMap() ) - if (node.elementType == FILE) { - println(node.prettyPrint()) - } searchNode(node, configuration) } @@ -106,16 +103,21 @@ class EmptyBlock(configRules: List) : DiktatRule( @Suppress("UnsafeCallOnNullableType") private fun isLambdaUsedAsFunction(node: ASTNode): Boolean { val parents = node.parents() - if (parents.any { it.elementType == CALL_EXPRESSION }) { - val callExpression = parents.find { it.elementType == CALL_EXPRESSION }!! - // excepting cases like list.map { } - return callExpression.treeParent.elementType != DOT_QUALIFIED_EXPRESSION - } else if (parents.any { it.elementType == LAMBDA_EXPRESSION }) { - val lambdaExpression = parents.find { it.elementType == LAMBDA_EXPRESSION }!! - // cases like A({}). Here Lambda expression is used as a value parameter. - return lambdaExpression.treeParent.elementType == VALUE_PARAMETER + return when { + parents.any { it.elementType == CALL_EXPRESSION } -> { + val callExpression = parents.find { it.elementType == CALL_EXPRESSION }!! + // excepting cases like list.map { } + callExpression.treeParent.elementType != DOT_QUALIFIED_EXPRESSION + } + parents.any { it.elementType == LAMBDA_EXPRESSION } -> { + val lambdaExpression = parents.find { it.elementType == LAMBDA_EXPRESSION }!! + // cases like A({}). Here Lambda expression is used as a value parameter. + lambdaExpression.treeParent.elementType == VALUE_PARAMETER + } + else -> { + false + } } - return false } /** diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index e764e325af..3797e04060 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -1,4 +1,4 @@ -# 3. General formatting (typesetting) +__# 3. General formatting (typesetting) ### 3.1 File-related rules This section describes the rules related to using files in your code. @@ -305,7 +305,7 @@ class A Avoid empty blocks, and ensure braces start on a new line. An empty code block can be closed immediately on the same line and the next line. However, a newline is recommended between opening and closing braces `{}` (see the examples below.) Generally, empty code blocks are prohibited; using them is considered a bad practice (especially for catch block). -They are only appropriate for overridden functions when the base class's functionality is not needed in the class-inheritor. +They are appropriate for overridden functions, when the base class's functionality is not needed in the class-inheritor, for lambdas used as a function and for empty function in implementation of functional interface. ```kotlin override fun foo() { } @@ -318,6 +318,8 @@ fun doNothing() {} fun doNothingElse() { } + +fun foo(bar: () -> Unit = {})__ ``` **Invalid examples:** From bceb7be57b075656d5ba7d116cc7b5f2215763d6 Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 14:58:46 +0300 Subject: [PATCH 5/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt index 51c5c5baf0..4cd3b50426 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt @@ -114,9 +114,7 @@ class EmptyBlock(configRules: List) : DiktatRule( // cases like A({}). Here Lambda expression is used as a value parameter. lambdaExpression.treeParent.elementType == VALUE_PARAMETER } - else -> { - false - } + else -> false } } From cda1e913de85f52579826583e3787d62b9c4e01a Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 15:38:32 +0300 Subject: [PATCH 6/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../ruleset/rules/chapter3/EmptyBlock.kt | 7 +++++-- .../cqfn/diktat/ruleset/utils/KotlinParser.kt | 2 +- .../ruleset/chapter3/EmptyBlockWarnTest.kt | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt index 4cd3b50426..8a1dd69960 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/EmptyBlock.kt @@ -16,6 +16,8 @@ import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.LPAR import com.pinterest.ktlint.core.ast.ElementType.RBRACE +import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT +import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE import org.jetbrains.kotlin.com.intellij.lang.ASTNode @@ -106,8 +108,9 @@ class EmptyBlock(configRules: List) : DiktatRule( return when { parents.any { it.elementType == CALL_EXPRESSION } -> { val callExpression = parents.find { it.elementType == CALL_EXPRESSION }!! - // excepting cases like list.map { } - callExpression.treeParent.elementType != DOT_QUALIFIED_EXPRESSION + // excepting cases like list.map { }. In this case call expression will not have value argument list + // And in this case: Parser.parse({}, some, thing) it will have value argument list + callExpression.hasChildOfType(VALUE_ARGUMENT_LIST) } parents.any { it.elementType == LAMBDA_EXPRESSION } -> { val lambdaExpression = parents.find { it.elementType == LAMBDA_EXPRESSION }!! diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt index 3a994703f7..d17c3ec885 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt @@ -50,7 +50,7 @@ class KotlinParser { } } // I don't really understand what's going on here, but thanks to this, you can use this node in the future val project = KotlinCoreEnvironment.createForProduction( - Disposable { }, + { }, compilerConfiguration, EnvironmentConfigFiles.JVM_CONFIG_FILES ).project // create project diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt index acce1ce7ad..c2a87615f0 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/EmptyBlockWarnTest.kt @@ -8,6 +8,8 @@ import org.cqfn.diktat.util.LintTestBase import com.pinterest.ktlint.core.LintError import generated.WarningNames +import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles +import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test @@ -221,4 +223,21 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) { rulesConfigList = rulesConfigListEmptyBlockExist ) } + + @Test + @Tag(WarningNames.EMPTY_BLOCK_STRUCTURE_ERROR) + fun `should not trigger on empty lambdas as a functions #2`() { + lintMethod( + """ + |fun some() { + | val project = KotlinCoreEnvironment.createForProduction( + | { }, + | compilerConfiguration, + | EnvironmentConfigFiles.JVM_CONFIG_FILES + | ).project + |} + """.trimMargin(), + rulesConfigList = rulesConfigListEmptyBlockExist + ) + } } From 14bbcb93469416dd70a396c03b25b737d52a0625 Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 16:29:19 +0300 Subject: [PATCH 7/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt | 3 +-- info/guide/guide-chapter-3.md | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt index d17c3ec885..bdef6a8dc1 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt @@ -10,7 +10,6 @@ import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.mock.MockProject -import org.jetbrains.kotlin.com.intellij.openapi.Disposable import org.jetbrains.kotlin.com.intellij.openapi.project.Project import org.jetbrains.kotlin.com.intellij.openapi.util.UserDataHolderBase import org.jetbrains.kotlin.com.intellij.pom.PomModel @@ -50,7 +49,7 @@ class KotlinParser { } } // I don't really understand what's going on here, but thanks to this, you can use this node in the future val project = KotlinCoreEnvironment.createForProduction( - { }, + {}, compilerConfiguration, EnvironmentConfigFiles.JVM_CONFIG_FILES ).project // create project diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index 3797e04060..69d855e7c5 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -1,4 +1,4 @@ -__# 3. General formatting (typesetting) +# 3. General formatting (typesetting) ### 3.1 File-related rules This section describes the rules related to using files in your code. @@ -319,7 +319,7 @@ fun doNothing() {} fun doNothingElse() { } -fun foo(bar: () -> Unit = {})__ +fun foo(bar: () -> Unit = {}) ``` **Invalid examples:** From 6c759e479efd65dee90bea1f14c28c7cc668b5d7 Mon Sep 17 00:00:00 2001 From: aktsay6 Date: Fri, 19 Mar 2021 16:46:45 +0300 Subject: [PATCH 8/8] bugfix/empty-lambdas(#796) ### What's done: * Fixed bug --- .../main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt index bdef6a8dc1..abeb23661b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt @@ -48,8 +48,7 @@ class KotlinParser { return null } } // I don't really understand what's going on here, but thanks to this, you can use this node in the future - val project = KotlinCoreEnvironment.createForProduction( - {}, + val project = KotlinCoreEnvironment.createForProduction({}, compilerConfiguration, EnvironmentConfigFiles.JVM_CONFIG_FILES ).project // create project