Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule 5.2.4 for lambda length #705

Merged
merged 5 commits into from
Jan 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,11 @@
# Checks that using runBlocking inside async block code
- name: RUN_BLOCKING_INSIDE_ASYNC
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
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 @@ -235,4 +235,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 @@ -133,6 +133,7 @@ enum class Warnings(
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"),
TOO_MANY_LINES_IN_LAMBDA(false, "5.2.5", "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 @@ -221,14 +221,14 @@ class BlockStructureBraces(private val configRules: List<RulesConfig>) : Rule("b
allMiddleSpace: List<ASTNode>,
node: ASTNode,
keyword: IElementType) {
allMiddleSpace.forEach {
if (checkBraceNode(it, true)) {
allMiddleSpace.forEach { space ->
if (checkBraceNode(space, true)) {
BRACES_BLOCK_STRUCTURE_ERROR.warnAndFix(configRules, emitWarn, isFixMode, "incorrect new line after closing brace",
it.startOffset, it) {
if (it.elementType != WHITE_SPACE) {
space.startOffset, space) {
if (space.elementType != WHITE_SPACE) {
node.addChild(PsiWhiteSpaceImpl(" "), node.findChildByType(keyword))
} else {
(it as LeafPsiElement).replaceWithText(" ")
(space as LeafPsiElement).replaceWithText(" ")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,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,71 @@
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 org.jetbrains.kotlin.com.intellij.lang.ASTNode

/**
* Rule 5.2.5 check lambda length without parameters
*/
class LambdaLengthRule(private val configRules: List<RulesConfig>) : Rule("lambda-length") {
private val configuration by lazy {
LambdaLengthConfiguration(
this.configRules.getRuleConfig(Warnings.TOO_MANY_LINES_IN_LAMBDA)?.configuration ?: emptyMap()
)
}
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.elementType == ElementType.LAMBDA_EXPRESSION) {
checkLambda(node, configuration)
}
}

private fun checkLambda(node: ASTNode, configuration: LambdaLengthConfiguration) {
val copyNode = node.clone() as ASTNode
Cheshiriks marked this conversation as resolved.
Show resolved Hide resolved
val sizeLambda = countCodeLines(copyNode)
if (sizeLambda > configuration.maxLambdaLength) {
copyNode.findAllNodesWithCondition({ it.elementType == ElementType.LAMBDA_EXPRESSION }).forEachIndexed { index, node ->
if (index > 0) {
node.treeParent.removeChild(node)
}
}
val isIt = copyNode.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION).map {re -> re.text}.contains("it")
val parameters = node.findChildByType(ElementType.FUNCTION_LITERAL)?.findChildByType(ElementType.VALUE_PARAMETER_LIST)
if (parameters == null && isIt) {
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 @@ -84,21 +84,21 @@ class LineLength(private val configRules: List<RulesConfig>) : Rule("line-length
@Suppress("UnsafeCallOnNullableType")
private fun checkLength(node: ASTNode, configuration: LineLengthConfiguration) {
var offset = 0
node.text.lines().forEach {
if (it.length > configuration.lineLength) {
node.text.lines().forEach { line ->
if (line.length > configuration.lineLength) {
val newNode = node.psi.findElementAt(offset + configuration.lineLength.toInt())!!.node
if ((newNode.elementType != KDOC_TEXT && newNode.elementType != KDOC_MARKDOWN_INLINE_LINK) ||
!isKdocValid(newNode)) {
positionByOffset = node.treeParent.calculateLineColByOffset()
val fixableType = isFixable(newNode, configuration)
LONG_LINE.warnAndFix(configRules, emitWarn, isFixMode,
"max line length ${configuration.lineLength}, but was ${it.length}",
"max line length ${configuration.lineLength}, but was ${line.length}",
offset + node.startOffset, node, fixableType != LongLineFixableCases.None) {
fixError(fixableType)
}
}
}
offset += it.length + 1
offset += line.length + 1
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ class MultipleModifiersSequence(private val configRules: List<RulesConfig>) : Ru
node
.getChildren(null)
.filterIndexed { index, astNode -> astNode.elementType == ANNOTATION_ENTRY && index > firstModifierIndex }
.forEach {
.forEach { astNode ->
WRONG_MULTIPLE_MODIFIERS_ORDER.warnAndFix(configRules, emitWarn, isFixMode,
"${it.text} annotation should be before all modifiers",
it.startOffset, it) {
val spaceBefore = it.treePrev
node.removeChild(it)
"${astNode.text} annotation should be before all modifiers",
astNode.startOffset, astNode) {
val spaceBefore = astNode.treePrev
node.removeChild(astNode)
if (spaceBefore != null && spaceBefore.elementType == WHITE_SPACE) {
node.removeChild(spaceBefore)
node.addChild(spaceBefore, node.firstChildNode)
node.addChild(it.clone() as ASTNode, spaceBefore)
node.addChild(astNode.clone() as ASTNode, spaceBefore)
} else {
node.addChild(PsiWhiteSpaceImpl(" "), node.getChildren(null).first())
node.addChild(it.clone() as ASTNode, node.getChildren(null).first())
node.addChild(astNode.clone() as ASTNode, node.getChildren(null).first())
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ class SingleLineStatementsRule(private val configRules: List<RulesConfig>) : Rul
}

private fun checkSemicolon(node: ASTNode) {
node.getChildren(semicolonToken).forEach {
if (!it.isFollowedByNewline()) {
MORE_THAN_ONE_STATEMENT_PER_LINE.warnAndFix(configRules, emitWarn, isFixMode, it.extractLineOfText(),
it.startOffset, it) {
if (it.treeParent.elementType == ENUM_ENTRY) {
node.getChildren(semicolonToken).forEach { astNode ->
if (!astNode.isFollowedByNewline()) {
MORE_THAN_ONE_STATEMENT_PER_LINE.warnAndFix(configRules, emitWarn, isFixMode, astNode.extractLineOfText(),
astNode.startOffset, astNode) {
if (astNode.treeParent.elementType == ENUM_ENTRY) {
node.treeParent.addChild(PsiWhiteSpaceImpl("\n"), node.treeNext)
} else {
if (!it.isBeginByNewline()) {
val nextNode = it.parent({ parent -> parent.treeNext != null }, strict = false)?.treeNext
node.appendNewlineMergingWhiteSpace(nextNode, it)
if (!astNode.isBeginByNewline()) {
val nextNode = astNode.parent({ parent -> parent.treeNext != null }, strict = false)?.treeNext
node.appendNewlineMergingWhiteSpace(nextNode, astNode)
}
node.removeChild(it)
node.removeChild(astNode)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,16 @@ class SmartCastRule(private val configRules: List<RulesConfig>) : Rule("smart-ca
*/
@Suppress("NestedBlockDepth", "TYPE_ALIAS")
private fun handleGroups(groups: Map<KtNameReferenceExpression, List<KtNameReferenceExpression>>) {
groups.keys.forEach {
val parentText = it.node.treeParent.text
groups.keys.forEach { key ->
val parentText = key.node.treeParent.text
if (parentText.contains(" is ")) {
groups.getValue(it).forEach { asCall ->
groups.getValue(key).forEach { asCall ->
if (asCall.node.hasParent(THEN)) {
raiseWarning(asCall.node)
}
}
} else if (parentText.contains(" !is ")) {
groups.getValue(it).forEach { asCall ->
groups.getValue(key).forEach { asCall ->
if (asCall.node.hasParent(ELSE)) {
raiseWarning(asCall.node)
}
Expand Down Expand Up @@ -234,12 +234,12 @@ class SmartCastRule(private val configRules: List<RulesConfig>) : Rule("smart-ca

val identifier = node.getFirstChildWithType(REFERENCE_EXPRESSION)?.text

node.getAllChildrenWithType(WHEN_ENTRY).forEach {
if (it.hasChildOfType(WHEN_CONDITION_IS_PATTERN) && identifier != null) {
val type = it.getFirstChildWithType(WHEN_CONDITION_IS_PATTERN)!!
node.getAllChildrenWithType(WHEN_ENTRY).forEach { entry ->
if (entry.hasChildOfType(WHEN_CONDITION_IS_PATTERN) && identifier != null) {
val type = entry.getFirstChildWithType(WHEN_CONDITION_IS_PATTERN)!!
.getFirstChildWithType(TYPE_REFERENCE)?.text

val callExpr = it.findAllNodesWithSpecificType(BINARY_WITH_TYPE).firstOrNull()
val callExpr = entry.findAllNodesWithSpecificType(BINARY_WITH_TYPE).firstOrNull()
val blocks = listOf(IsExpressions(identifier, type ?: ""))

callExpr?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ class SingleInitRule(private val configRule: List<RulesConfig>) : Rule("multiple
!(property.psi as KtProperty).hasBody() && assignments.size == 1
}
.takeIf { it.isNotEmpty() }
?.let {
?.let { map ->
Warnings.MULTIPLE_INIT_BLOCKS.warnAndFix(configRule, emitWarn, isFixMode,
"`init` block has assignments that can be moved to declarations", initBlock.startOffset, initBlock
) {
it.forEach { (property, assignments) ->
map.forEach { (property, assignments) ->
val assignment = assignments.single()
property.addChild(PsiWhiteSpaceImpl(" "), null)
property.addChild(LeafPsiElement(EQ, "="), null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,15 +471,15 @@ class NewlinesRule(private val configRules: List<RulesConfig>) : Rule("newlines"
}
val callsByNewLine: ListOfList = mutableListOf()
var callsInOneNewLine: MutableList<ASTNode> = mutableListOf()
this.forEach {
if (it.treePrev.isFollowedByNewline() || it.treePrev.isWhiteSpaceWithNewline()) {
this.forEach { node ->
if (node.treePrev.isFollowedByNewline() || node.treePrev.isWhiteSpaceWithNewline()) {
callsByNewLine.add(callsInOneNewLine)
callsInOneNewLine = mutableListOf()
callsInOneNewLine.add(it)
callsInOneNewLine.add(node)
} else {
callsInOneNewLine.add(it)
callsInOneNewLine.add(node)
}
if (it.treePrev.elementType == POSTFIX_EXPRESSION && !it.treePrev.isFollowedByNewline() && configuration.maxCallsInOneLine == 1) {
if (node.treePrev.elementType == POSTFIX_EXPRESSION && !node.treePrev.isFollowedByNewline() && configuration.maxCallsInOneLine == 1) {
return true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -789,3 +789,14 @@ 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
}

/**
* Count number of lines in code block. Note: only *copy* of a node should be passed to this method, because the method changes the node.
*
* @return the number of lines in a block of code.
Cheshiriks marked this conversation as resolved.
Show resolved Hide resolved
*/
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
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,24 @@ private fun convertUnknownCaseToCamel(str: String, isFirstLetterCapital: Boolean
// [p]a[SC]a[_]l -> [P]a[Sc]a[L]
var isPreviousLetterCapital = isFirstLetterCapital
var isPreviousLetterUnderscore = false
return str.map {
if (it.isUpperCase()) {
val result = if (isPreviousLetterCapital && !isPreviousLetterUnderscore) it.toLowerCase() else it
return str.map { char ->
if (char.isUpperCase()) {
val result = if (isPreviousLetterCapital && !isPreviousLetterUnderscore) char.toLowerCase() else char
isPreviousLetterCapital = true
isPreviousLetterUnderscore = false
result.toString()
} else {
val result = if (it == '_') {
val result = if (char == '_') {
isPreviousLetterUnderscore = true
""
} else if (isPreviousLetterUnderscore) {
isPreviousLetterCapital = true
isPreviousLetterUnderscore = false
it.toUpperCase().toString()
char.toUpperCase().toString()
} else {
isPreviousLetterCapital = false
isPreviousLetterUnderscore = false
it.toString()
char.toString()
}
result
}
Expand All @@ -172,17 +172,17 @@ private fun convertUnknownCaseToCamel(str: String, isFirstLetterCapital: Boolean
private fun convertUnknownCaseToUpperSnake(str: String): String {
// [p]a[SC]a[_]l -> [P]A_[SC]_A_[L]
var alreadyInsertedUnderscore = true
return str.map {
if (it.isUpperCase()) {
return str.map { char ->
if (char.isUpperCase()) {
if (!alreadyInsertedUnderscore) {
alreadyInsertedUnderscore = true
"_$it"
"_$char"
} else {
it.toString()
char.toString()
}
} else {
alreadyInsertedUnderscore = (it == '_')
it.toUpperCase().toString()
alreadyInsertedUnderscore = (char == '_')
char.toUpperCase().toString()
}
}.joinToString("")
}
Expand Down
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 @@ -356,6 +356,11 @@
# Checks that using runBlocking inside async block code
- name: RUN_BLOCKING_INSIDE_ASYNC
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 @@ -356,6 +356,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
Loading