diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 61bc1fad94..0b0b332b5e 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -231,4 +231,6 @@ public object WarningNames { public const val OBJECT_IS_PREFERRED: String = "OBJECT_IS_PREFERRED" public const val INVERSE_FUNCTION_PREFERRED: String = "INVERSE_FUNCTION_PREFERRED" + + public const val TOO_MANY_LINES_IN_LAMBDA: String = "TOO_MANY_LINES_IN_LAMBDA" } 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 981b21b565..f18d9d61da 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 @@ -131,6 +131,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"), + TOO_MANY_LINES_IN_LAMBDA(false, "5.2.4", "long lambdas should have a parameter name instead of it"), // ======== 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/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 7640a6f678..7357459d56 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 @@ -142,6 +142,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS ::ImmutableValNoVarRule, ::AvoidNestedFunctionsRule, ::ExtensionFunctionsSameNameRule, + ::LambdaLengthRule, // formatting: moving blocks, adding line breaks, indentations etc. ::BlockStructureBraces, ::ConsecutiveSpacesRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FunctionLength.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FunctionLength.kt index 82d3f02592..a9c619fae2 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FunctionLength.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FunctionLength.kt @@ -43,11 +43,10 @@ class FunctionLength(private val configRules: List) : Rule("functio ?.node ?.clone() ?: return) as ASTNode } - copyNode.findAllNodesWithCondition({ it.elementType in commentType }).forEach { it.treeParent.removeChild(it) } - val functionText = copyNode.text.lines().filter { it.isNotBlank() } - if (functionText.size > configuration.maxFunctionLength) { + val sizeFun = countCodeLines(copyNode) + if (sizeFun > configuration.maxFunctionLength) { TOO_LONG_FUNCTION.warn(configRules, emitWarn, isFixMode, - "max length is ${configuration.maxFunctionLength}, but you have ${functionText.size}", + "max length is ${configuration.maxFunctionLength}, but you have $sizeFun", node.startOffset, node) } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LambdaLengthRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LambdaLengthRule.kt new file mode 100644 index 0000000000..4e8f013eff --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LambdaLengthRule.kt @@ -0,0 +1,66 @@ +package org.cqfn.diktat.ruleset.rules + +import org.cqfn.diktat.common.config.rules.RuleConfiguration +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.common.config.rules.getRuleConfig +import org.cqfn.diktat.ruleset.constants.EmitType +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.utils.* + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.isPartOfComment +import org.jetbrains.kotlin.com.intellij.lang.ASTNode + +/** + * Rule 5.2.4 check lambda length without parameters + */ +class LambdaLengthRule(private val configRules: List) : Rule("lambda-length") { + private var isFixMode: Boolean = false + private lateinit var emitWarn: EmitType + + override fun visit( + node: ASTNode, + autoCorrect: Boolean, + emit: EmitType + ) { + emitWarn = emit + isFixMode = autoCorrect + + val configuration by lazy { + LambdaLengthConfiguration( + configRules.getRuleConfig(Warnings.TOO_MANY_LINES_IN_LAMBDA)?.configuration ?: emptyMap() + ) + } + + if (node.elementType == ElementType.LAMBDA_EXPRESSION) { + checkLambda(node, configuration) + } + } + + private fun checkLambda(node: ASTNode, configuration: LambdaLengthConfiguration) { + val copyNode = node.clone() as ASTNode + val isIt: Boolean = node.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION).map {re -> re.text}.indexOf("it") != -1 + val parameters = node.findChildByType(ElementType.FUNCTION_LITERAL)?.findChildByType(ElementType.VALUE_PARAMETER_LIST) + val sizeLambda = countCodeLines(copyNode) + if (parameters == null && isIt && sizeLambda > configuration.maxLambdaLength) { + Warnings.TOO_MANY_LINES_IN_LAMBDA.warn(configRules, emitWarn, isFixMode, + "max length lambda without arguments is ${configuration.maxLambdaLength}, but you have $sizeLambda", + node.startOffset, node) + } + } + + /** + * [RuleConfiguration] for lambda length + */ + class LambdaLengthConfiguration(config: Map) : RuleConfiguration(config) { + /** + * Maximum allowed lambda length + */ + val maxLambdaLength = config["maxLambdaLength"]?.toLong() ?: MAX_LINES_IN_LAMBDA + } + + companion object { + private const val MAX_LINES_IN_LAMBDA = 10L + } +} diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index 157dddcb5f..4b41869ad1 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -789,3 +789,12 @@ private fun ASTNode.calculateLineNumber() = getRootNode() require(it >= 0) { "Cannot calculate line number correctly, node's offset $startOffset is greater than file length ${getRootNode().textLength}" } it + 1 } + +/** + * @return the number of lines in a block of code. + */ +fun countCodeLines(copyNode: ASTNode): Int { + copyNode.findAllNodesWithCondition({ it.isPartOfComment() }).forEach { it.treeParent.removeChild(it) } + val text = copyNode.text.lines().filter { it.isNotBlank() } + return text.size +} diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 3e7f60fdb6..e19bb10435 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -350,6 +350,11 @@ # Checks that function use default values, instead overloading - name: WRONG_OVERLOADING_FUNCTION_ARGUMENTS enabled: true +# Checks that the long lambda has parameters +- name: TOO_MANY_LINES_IN_LAMBDA + enabled: true + configuration: + maxLambdaLength: 10 # max length of lambda without parameters # 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 0caa29ff1e..aa07910a2e 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -350,6 +350,11 @@ # Checks that property in constructor doesn't contain comment - name: KDOC_NO_CONSTRUCTOR_PROPERTY enabled: true +# Checks that the long lambda has parameters +- name: TOO_MANY_LINES_IN_LAMBDA + enabled: true + configuration: + maxLambdaLength: 10 # max length of lambda without parameters # Checks that property in KDoc present in class - name: KDOC_EXTRA_PROPERTY enabled: true diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/LambdaLengthWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/LambdaLengthWarnTest.kt new file mode 100644 index 0000000000..59b26f0248 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/LambdaLengthWarnTest.kt @@ -0,0 +1,141 @@ +package org.cqfn.diktat.ruleset.chapter5 + +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.LambdaLengthRule +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 LambdaLengthWarnTest : LintTestBase(::LambdaLengthRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:lambda-length" + private val rulesConfigList: List = listOf( + RulesConfig( + Warnings.TOO_MANY_LINES_IN_LAMBDA.name, true, + mapOf("maxLambdaLength" to "3")) + ) + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `less than max`() { + lintMethod( + """ + |fun foo() { + | val x = 10 + | val list = listOf(1, 2, 3, 4, 5) + | .map {element -> element + x} + |} + """.trimMargin(), + rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `less than max without argument`() { + lintMethod( + """ + |fun foo() { + | val x = 10 + | val list = listOf(1, 2, 3, 4, 5) + | .map {it + x} + |} + """.trimMargin(), + rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `more than max with argument`() { + lintMethod( + """ + |fun foo() { + | val calculateX = { x : Int -> + | when(x) { + | in 0..40 -> "Fail" + | in 41..70 -> "Pass" + | in 71..100 -> "Distinction" + | else -> false + | } + | } + |} + """.trimMargin(), + rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `more than maximum without argument`() { + lintMethod( + """ + |fun foo() { + | val list = listOf(1, 2, 3, 4, 5) + | .map { + | val x = 0 + | val y = x + 1 + | val z = y + 1 + | it + z + | + | + | + | + | } + |} + """.trimMargin(), + LintError(3, 13, ruleId, "${Warnings.TOO_MANY_LINES_IN_LAMBDA.warnText()} max length lambda without arguments is 3, but you have 6", false), + rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `two lambda more than maximum without argument`() { + lintMethod( + """ + |fun foo() { + | val list = listOf(1, 2, 3, 4, 5) + | .filter { n -> n % 2 == 1 } + | .map { + | val x = 0 + | val y = x + 1 + | val z = y + 1 + | it + z + | + | + | + | + | } + |} + """.trimMargin(), + LintError(4, 13, ruleId, "${Warnings.TOO_MANY_LINES_IN_LAMBDA.warnText()} max length lambda without arguments is 3, but you have 6", false), + rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.TOO_MANY_LINES_IN_LAMBDA) + fun `lambda in lambda`() { + lintMethod( + """ + |fun foo() { + | val list = listOf(listOf(1,2,3), listOf(4,5,6)) + | .map {l -> l.map { + | val x = 0 + | val y = x + 1 + | val z = y + 1 + | println(it) + | } + | } + | } + """.trimMargin(), + LintError(3, 25, ruleId, "${Warnings.TOO_MANY_LINES_IN_LAMBDA.warnText()} max length lambda without arguments is 3, but you have 6", false), + rulesConfigList = rulesConfigList + ) + } +} diff --git a/info/available-rules.md b/info/available-rules.md index 6af0e09a05..bb665dce45 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -95,6 +95,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 | TOO_MANY_LINES_IN_LAMBDA | Check: that the long lambda has parameters | no | maxLambdaLength | | 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-TOC.md b/info/guide/guide-TOC.md index 5618f99bb2..6dc0207680 100644 --- a/info/guide/guide-TOC.md +++ b/info/guide/guide-TOC.md @@ -88,6 +88,7 @@ I [Preface](#c0) * [5.2.1 The lambda parameter of the function should be placed at the end of the argument list](#r5.2.1) * [5.2.2 Number of function parameters should be limited to five](#r5.2.2) * [5.2.3 Use default values for function arguments instead of overloading them](#r5.2.3) + * [5.2.4 Long lambdas should have explicit parameters](#r5.2.4) [6. Classes, interfaces, and extension functions](#c6) * [6.1 Classes](#c6.1) diff --git a/info/guide/guide-chapter-5.md b/info/guide/guide-chapter-5.md index 294418dce4..31757007c4 100644 --- a/info/guide/guide-chapter-5.md +++ b/info/guide/guide-chapter-5.md @@ -132,3 +132,7 @@ private fun foo() { // ... } ``` + +#### 5.2.4 Long lambdas should have explicit parameters +The lambda without parameters shouldn't be too long. +If a lambda is too long, it can confuse the user. Lambda without parameters should consist of 10 lines (non-empty and non-comment) in total.