diff --git a/diktat-analysis.yml b/diktat-analysis.yml index af4695f9e4..bb2e9fa348 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -361,6 +361,9 @@ enabled: true configuration: maxLambdaLength: 10 # max length of lambda without parameters +# Checks that using unnecessary, custom label +- name: CUSTOM_LABEL + 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 5332a3e099..bb6e65e535 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -203,6 +203,10 @@ public object WarningNames { public const val RUN_BLOCKING_INSIDE_ASYNC: String = "RUN_BLOCKING_INSIDE_ASYNC" + public const val TOO_MANY_LINES_IN_LAMBDA: String = "TOO_MANY_LINES_IN_LAMBDA" + + public const val CUSTOM_LABEL: String = "CUSTOM_LABEL" + public const val SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY: String = "SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY" @@ -235,6 +239,4 @@ 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 e8e088167b..478c71fdaa 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 @@ -134,6 +134,7 @@ enum class Warnings( 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"), + CUSTOM_LABEL(false, "5.2.6", "avoid using expression with custom label"), // ======== 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/BlockStructureBraces.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt index e381be9235..b1079ccf71 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt @@ -72,7 +72,7 @@ class BlockStructureBraces(private val configRules: List) : Rule("b FUN, CLASS_INITIALIZER, SECONDARY_CONSTRUCTOR -> checkFun(node, configuration) IF -> checkIf(node, configuration) WHEN -> checkWhen(node, configuration) - FOR, WHILE, DO_WHILE -> checkLoop(node, configuration) + in loopType -> checkLoop(node, configuration) TRY -> checkTry(node, configuration) else -> return } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt index 9275b41aca..ccac24599b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt @@ -5,20 +5,18 @@ import org.cqfn.diktat.ruleset.constants.EmitType import org.cqfn.diktat.ruleset.constants.Warnings.NO_BRACES_IN_CONDITIONALS_AND_LOOPS import org.cqfn.diktat.ruleset.utils.findChildrenMatching import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse +import org.cqfn.diktat.ruleset.utils.loopType import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType import com.pinterest.ktlint.core.ast.ElementType.BLOCK import com.pinterest.ktlint.core.ast.ElementType.DO_KEYWORD -import com.pinterest.ktlint.core.ast.ElementType.DO_WHILE import com.pinterest.ktlint.core.ast.ElementType.ELSE_KEYWORD -import com.pinterest.ktlint.core.ast.ElementType.FOR import com.pinterest.ktlint.core.ast.ElementType.IF import com.pinterest.ktlint.core.ast.ElementType.IF_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.LBRACE import com.pinterest.ktlint.core.ast.ElementType.RBRACE import com.pinterest.ktlint.core.ast.ElementType.WHEN -import com.pinterest.ktlint.core.ast.ElementType.WHILE import com.pinterest.ktlint.core.ast.ElementType.WHILE_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE import com.pinterest.ktlint.core.ast.isPartOfComment @@ -50,7 +48,7 @@ class BracesInConditionalsAndLoopsRule(private val configRules: List checkIfNode(node) WHEN -> checkWhenBranches(node) - FOR, WHILE, DO_WHILE -> checkLoop(node) + in loopType -> checkLoop(node) else -> return } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/CustomLabel.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/CustomLabel.kt new file mode 100644 index 0000000000..37e458224d --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/CustomLabel.kt @@ -0,0 +1,44 @@ +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.CUSTOM_LABEL +import org.cqfn.diktat.ruleset.utils.loopType + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.BREAK +import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.CONTINUE +import com.pinterest.ktlint.core.ast.ElementType.LABEL_QUALIFIER +import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.RETURN +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.psiUtil.parents + +/** + * Rule that checks using custom label + */ +class CustomLabel(private val configRules: List) : Rule("custom-label") { + private var isFixMode: Boolean = false + private val forEachReference = listOf("forEach", "forEachIndexed") + private val labels = listOf("@loop", "@forEach", "@forEachIndexed") + private val stopWords = listOf(RETURN, BREAK, CONTINUE) + private lateinit var emitWarn: EmitType + + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: EmitType) { + emitWarn = emit + isFixMode = autoCorrect + + if (node.elementType == LABEL_QUALIFIER && node.text !in labels && node.treeParent.elementType in stopWords) { + val nestedCount = node.parents().count { + it.elementType in loopType || + (it.elementType == CALL_EXPRESSION && it.findChildByType(REFERENCE_EXPRESSION)?.text in forEachReference) + } + if (nestedCount == 1) { + CUSTOM_LABEL.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) + } + } + } +} 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 60aba56ab6..7860906895 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 @@ -122,6 +122,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS ::PropertyAccessorFields, ::AbstractClassesRule, ::SingleInitRule, + ::CustomLabel, ::VariableGenericTypeDeclarationRule, ::LongNumericalValuesSeparatedRule, ::NestedFunctionBlock, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt index 17601a595b..89293a430a 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstConstants.kt @@ -3,11 +3,14 @@ package org.cqfn.diktat.ruleset.utils import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT +import com.pinterest.ktlint.core.ast.ElementType.DO_WHILE import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT +import com.pinterest.ktlint.core.ast.ElementType.FOR import com.pinterest.ktlint.core.ast.ElementType.KDOC import com.pinterest.ktlint.core.ast.ElementType.LBRACE import com.pinterest.ktlint.core.ast.ElementType.RBRACE import com.pinterest.ktlint.core.ast.ElementType.SEMICOLON +import com.pinterest.ktlint.core.ast.ElementType.WHILE import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE /** @@ -24,6 +27,7 @@ internal const val SET_PREFIX = "set" val emptyBlockList = listOf(LBRACE, WHITE_SPACE, SEMICOLON, RBRACE) val commentType = listOf(BLOCK_COMMENT, EOL_COMMENT, KDOC) +val loopType = listOf(FOR, WHILE, DO_WHILE) val copyrightWords = setOf("copyright", "版权") internal const val EMPTY_BLOCK_TEXT = "{}" 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 ebd6f425b7..0eeec11ce1 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 @@ -568,7 +568,7 @@ fun ASTNode.findLBrace(): ASTNode? = when (this.elementType) { ElementType.THEN, ElementType.ELSE, ElementType.FUN, ElementType.TRY, ElementType.CATCH, ElementType.FINALLY -> this.findChildByType(ElementType.BLOCK)?.findChildByType(LBRACE) ElementType.WHEN -> this.findChildByType(LBRACE)!! - ElementType.FOR, ElementType.WHILE, ElementType.DO_WHILE -> + in loopType -> this.findChildByType(ElementType.BODY) ?.findChildByType(ElementType.BLOCK) ?.findChildByType(LBRACE) diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 58c2966f91..ad2a5e1dcf 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -361,6 +361,9 @@ enabled: true configuration: maxLambdaLength: 10 # max length of lambda without parameters +# Checks that using unnecessary, custom label +- name: CUSTOM_LABEL + 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 84f0a7275f..9af524733e 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -361,6 +361,9 @@ enabled: true configuration: maxLambdaLength: 10 # max length of lambda without parameters +# Checks that using unnecessary, custom label +- name: CUSTOM_LABEL + enabled: true # 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/CustomLabelsTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/CustomLabelsTest.kt new file mode 100644 index 0000000000..8c34f97211 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/CustomLabelsTest.kt @@ -0,0 +1,75 @@ +package org.cqfn.diktat.ruleset.chapter5 + +import org.cqfn.diktat.ruleset.constants.Warnings.CUSTOM_LABEL +import org.cqfn.diktat.ruleset.rules.CustomLabel +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 CustomLabelsTest : LintTestBase(::CustomLabel) { + private val ruleId = "$DIKTAT_RULE_SET_ID:custom-label" + + @Test + @Tag(WarningNames.CUSTOM_LABEL) + fun `should trigger custom label`() { + lintMethod( + """ + fun foo() { + run qwe@ { + q.forEach { + return@qwe + } + } + q.forEachIndexed { index, i -> + return@forEachIndexed + } + loop@ for(i: Int in q) { + println(i) + break@loop + } + qq@ for(i: Int in q) { + println(i) + break@qq + } + q.run { + it.map { + it.foreach{ + break@forEach + } + } + } + } + """.trimMargin(), + LintError(4, 39, ruleId, "${CUSTOM_LABEL.warnText()} @qwe", false), + LintError(16, 34, ruleId, "${CUSTOM_LABEL.warnText()} @qq", false) + ) + } + + @Test + @Tag(WarningNames.CUSTOM_LABEL) + fun `should not trigger custom label in nested expression`() { + lintMethod( + """ + fun foo() { + qq@ for(i: Int in q) { + for (j: Int in q) { + println(i) + break@qq + } + } + + q.forEach outer@ { + it.forEach { + if(it == 21) + return@outer + } + } + } + """.trimMargin() + ) + } +} diff --git a/info/available-rules.md b/info/available-rules.md index 88375589e2..e04df4cb4a 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -98,6 +98,7 @@ | 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 | - | | 5 | 5.2.5 | TOO_MANY_LINES_IN_LAMBDA | Check: that the long lambda has parameters | no | maxLambdaLength | +| 5 | 5.2.6 | CUSTOM_LABEL | Check: that using unnecessary, custom label | 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-TOC.md b/info/guide/guide-TOC.md index e8e7a8c9a9..89233b325f 100644 --- a/info/guide/guide-TOC.md +++ b/info/guide/guide-TOC.md @@ -87,8 +87,10 @@ I [Preface](#c0) * [5.2 Function arguments](#c5.2) * [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.5 Long lambdas should have explicit parameters](#r5.2.4) + * [5.2.3 Use default values for function arguments instead of overloading them](#r5.2.3) + * [5.2.4 Synchronizing code inside asynchronous code](#r5.2.4) + * [5.2.5 Long lambdas should have explicit parameters](#r5.2.5) + * [5.2.6 Avoid using unnecessary, custom label](#r5.2.6) [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 ea27034443..997c53757c 100644 --- a/info/guide/guide-chapter-5.md +++ b/info/guide/guide-chapter-5.md @@ -146,3 +146,29 @@ GlobalScope.async { #### 5.2.5 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. + +#### 5.2.6 Avoid using unnecessary, custom label +Expressions with unnecessary, custom labels generally increase complexity and worsen the maintainability of the code. + +**Invalid example**: +```kotlin +run lab@ { + list.forEach { + return@lab + } +} +``` + +**Valid example**: +```kotlin +list.forEachIndexed { index, i -> + return@forEachIndexed +} + +lab@ for(i: Int in q) { + for (j: Int in q) { + println(i) + break@lab + } +} +```