Skip to content

Commit

Permalink
New rule 5.2.4 for lambda length
Browse files Browse the repository at this point in the history
### What's done:
* Added rule logic
* Added new warning
* Added new test
  • Loading branch information
Cheshiriks committed Jan 14, 2021
1 parent 241f45e commit feef0d8
Show file tree
Hide file tree
Showing 12 changed files with 239 additions and 4 deletions.
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ class FunctionLength(private val configRules: List<RulesConfig>) : 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)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<RulesConfig>) : 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<String, String>) : 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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
5 changes: 5 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<RulesConfig> = 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
)
}
}
1 change: 1 addition & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<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 |
Expand Down
1 change: 1 addition & 0 deletions info/guide/guide-TOC.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions info/guide/guide-chapter-5.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,7 @@ private fun foo() {
// ...
}
```

#### <a name="r5.2.4"></a> 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.

0 comments on commit feef0d8

Please sign in to comment.