-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rule 5.1.2: Avoid deep nesting of function code blocks (#348)
* Rule 5.1.2 nested block ### What's done: Implemented Rule Added tests and info into documentation
- Loading branch information
Showing
9 changed files
with
343 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/NestedFunctionBlock.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package org.cqfn.diktat.ruleset.rules | ||
|
|
||
| import com.pinterest.ktlint.core.Rule | ||
| import com.pinterest.ktlint.core.ast.ElementType.CLASS | ||
| import com.pinterest.ktlint.core.ast.ElementType.FUN | ||
| import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_LITERAL | ||
| import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER | ||
| import com.pinterest.ktlint.core.ast.ElementType.LBRACE | ||
| import com.pinterest.ktlint.core.ast.ElementType.OBJECT_DECLARATION | ||
| 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.Warnings.NESTED_BLOCK | ||
| import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType | ||
| import org.cqfn.diktat.ruleset.utils.hasChildOfType | ||
| import org.jetbrains.kotlin.com.intellij.lang.ASTNode | ||
| import org.jetbrains.kotlin.psi.psiUtil.parents | ||
|
|
||
| /** | ||
| * Rule 5.1.2 Nested blokcs | ||
| */ | ||
| class NestedFunctionBlock(private val configRules: List<RulesConfig>) : Rule("nested-block") { | ||
|
|
||
| companion object { | ||
| private const val MAX_NESTED_BLOCK_COUNT = 4L | ||
| /** | ||
| * Nodes of these types reset counter of nested blocks | ||
| */ | ||
| private val NULLIFICATION_TYPE = listOf(CLASS, FUN, OBJECT_DECLARATION, FUNCTION_LITERAL) | ||
| } | ||
|
|
||
| val configuration: NestedBlockConfiguration by lazy { | ||
| NestedBlockConfiguration(configRules.getRuleConfig(NESTED_BLOCK)?.configuration ?: mapOf()) | ||
| } | ||
|
|
||
| private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) | ||
| private var isFixMode: Boolean = false | ||
|
|
||
| override fun visit(node: ASTNode, | ||
| autoCorrect: Boolean, | ||
| emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { | ||
| emitWarn = emit | ||
| isFixMode = autoCorrect | ||
|
|
||
| if (node.elementType in NULLIFICATION_TYPE) | ||
| countNestedBlocks(node, configuration.maxNestedBlockQuantity) | ||
| } | ||
|
|
||
| private fun countNestedBlocks(node: ASTNode, maxNestedBlockCount: Long) { | ||
| node.findAllNodesWithSpecificType(LBRACE).reversed().forEach { lbraceNode -> | ||
| val blockParent = lbraceNode | ||
| .parents() | ||
| .takeWhile { it != node } | ||
| .takeIf { parentList -> parentList.map { it.elementType }.none { it in NULLIFICATION_TYPE } } | ||
| ?.count { it.hasChildOfType(LBRACE) } | ||
| ?: return | ||
| if (blockParent > maxNestedBlockCount) { | ||
| NESTED_BLOCK.warn(configRules, emitWarn, isFixMode, node.findChildByType(IDENTIFIER)?.text ?: node.text, | ||
| node.startOffset, node) | ||
| return | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class NestedBlockConfiguration(config: Map<String, String>) : RuleConfiguration(config) { | ||
| val maxNestedBlockQuantity = config["maxNestedBlockQuantity"]?.toLong() ?: MAX_NESTED_BLOCK_COUNT | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
254 changes: 254 additions & 0 deletions
254
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/NestedFunctionBlockWarnTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,254 @@ | ||
| package org.cqfn.diktat.ruleset.chapter5 | ||
|
|
||
| import com.pinterest.ktlint.core.LintError | ||
| import generated.WarningNames | ||
| import org.cqfn.diktat.common.config.rules.RulesConfig | ||
| import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID | ||
| import org.cqfn.diktat.ruleset.constants.Warnings.NESTED_BLOCK | ||
| import org.cqfn.diktat.ruleset.rules.NestedFunctionBlock | ||
| import org.cqfn.diktat.util.LintTestBase | ||
| import org.junit.jupiter.api.Tag | ||
| import org.junit.jupiter.api.Test | ||
|
|
||
| class NestedFunctionBlockWarnTest : LintTestBase(::NestedFunctionBlock) { | ||
|
|
||
| private val ruleId = "$DIKTAT_RULE_SET_ID:nested-block" | ||
|
|
||
| private val rulesConfigList = listOf( | ||
| RulesConfig(NESTED_BLOCK.name, true, mapOf("maxNestedBlockQuantity" to "2")) | ||
| ) | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `should ignore lambda expression`() { | ||
| lintMethod( | ||
| """ | ||
| |fun foo() { | ||
| | while(true) { | ||
| | println() | ||
| | } | ||
| | | ||
| | when(x) { | ||
| | 10 -> {10} | ||
| | else -> { | ||
| | if (true) { | ||
| | println(1) | ||
| | } | ||
| | } | ||
| | } | ||
| | | ||
| | val x = { | ||
| | if (true) { | ||
| | if (false) { | ||
| | while(false) { | ||
| | 10 | ||
| | } | ||
| | } | ||
| | } | ||
| | } | ||
| | | ||
| | for(x in 1..2){ | ||
| | println(x) | ||
| | } | ||
| |} | ||
| """.trimMargin() | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check simple nested block`() { | ||
| lintMethod( | ||
| """ | ||
| |fun foo() { | ||
| | | ||
| | if (true) { | ||
| | if (false) { | ||
| | if (true) { | ||
| | do { | ||
| | println("nested") | ||
| | } while(true) | ||
| | } | ||
| | } | ||
| | } else { | ||
| | println("dscsds") | ||
| | } | ||
| |} | ||
| """.trimMargin(), | ||
| LintError(1,1, ruleId, "${NESTED_BLOCK.warnText()} foo", false) | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check simple nested block with try`() { | ||
| lintMethod( | ||
| """ | ||
| |fun foo() { | ||
| | | ||
| | if (true) { | ||
| | if (false) { | ||
| | try { | ||
| | try{ | ||
| | try{ | ||
| | | ||
| | } catch(ex: Exception){ | ||
| | try{ | ||
| | println("hi") | ||
| | } catch(ex: Exception){} | ||
| | } | ||
| | } catch(ex: Exception){} | ||
| | } catch(ex: Exception){} | ||
| | } | ||
| | } else { | ||
| | println("dscsds") | ||
| | } | ||
| |} | ||
| """.trimMargin(), | ||
| LintError(1,1, ruleId, "${NESTED_BLOCK.warnText()} foo", false) | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check simple nested block of function`() { | ||
| lintMethod( | ||
| """ | ||
| |fun foo() { | ||
| | | ||
| | if (true) { | ||
| | if (false) { | ||
| | fun goo() { | ||
| | if(true) { | ||
| | | ||
| | } | ||
| | } | ||
| | } | ||
| | } else { | ||
| | println("dscsds") | ||
| | } | ||
| |} | ||
| """.trimMargin() | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check simple nested block of local class`() { | ||
| lintMethod( | ||
| """ | ||
| |fun foo() { | ||
| | class A() { | ||
| | fun goo() { | ||
| | if (true) { | ||
| | if (false) { | ||
| | while(true) { | ||
| | if(false) { | ||
| | try { | ||
| | println("ne") | ||
| | } catch (e: Exception) {} | ||
| | } | ||
| | } | ||
| | } | ||
| | } else { | ||
| | println("dscsds") | ||
| | } | ||
| | } | ||
| | } | ||
| | if(true) { | ||
| | while(true) { | ||
| | } | ||
| | } | ||
| |} | ||
| """.trimMargin(), | ||
| LintError(3,8, ruleId, "${NESTED_BLOCK.warnText()} goo", false) | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check with lambda`() { | ||
| lintMethod( | ||
| """ | ||
| private fun findBlocks(node: ASTNode): List<ASTNode> { | ||
| val result = mutableListOf<ASTNode>() | ||
| node.getChildren(null).forEach { | ||
| when (it.elementType) { | ||
| IF -> Pair(it.findChildByType(THEN)?.findChildByType(BLOCK), it.findChildByType(ELSE)?.findChildByType(BLOCK)) | ||
| WHEN -> Pair(it, null) | ||
| WHEN_ENTRY -> Pair(it.findChildByType(BLOCK), null) | ||
| FUN -> Pair(it.findChildByType(BLOCK), null) | ||
| else -> Pair(it.findChildByType(BODY)?.findChildByType(BLOCK), null) | ||
| }.let { pair -> | ||
| pair.let { | ||
| pair.first?.let { it1 -> result.add(it1) } | ||
| pair.second?.let { it2 -> result.add(it2) } | ||
| } | ||
| } | ||
| } | ||
| return result | ||
| } | ||
| """.trimMargin() | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check with anonymous class`() { | ||
| lintMethod( | ||
| """ | ||
| val q = list.filter {it == 0} | ||
| val keyListener = KeyAdapter { keyEvent -> | ||
| if (true) { | ||
| } else if (false) { | ||
| while(true) { | ||
| if(true) { | ||
| println(10) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| val keyListener = object : KeyAdapter() { | ||
| override fun keyPressed(keyEvent : KeyEvent) { | ||
| } | ||
| } | ||
| """.trimMargin(), | ||
| LintError(4,50, ruleId, "${NESTED_BLOCK.warnText()} { keyEvent ->...", false), | ||
| rulesConfigList = rulesConfigList | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @Tag(WarningNames.NESTED_BLOCK) | ||
| fun `check simple nested block inside class`() { | ||
| lintMethod( | ||
| """ | ||
| |class A { | ||
| | fun foo() { | ||
| | if(true) { | ||
| | if(false) { | ||
| | if(true) { | ||
| | when(x) { | ||
| | } | ||
| | } | ||
| | } | ||
| | } | ||
| | } | ||
| | | ||
| | fun goo() { | ||
| | if(true){ | ||
| | if(false){ | ||
| | if(true){ | ||
| | } | ||
| | } | ||
| | } | ||
| | } | ||
| |} | ||
| """.trimMargin(), | ||
| LintError(2,4, ruleId, "${NESTED_BLOCK.warnText()} foo", false) | ||
| ) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1303570There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to retrieve PDD puzzles from the code base and submit them to GitHub. If you think that it's a bug on our side, please submit it to yegor256/0pdd:
Please, copy and paste this stack trace to GitHub: