From 194d47f90ca48729cb8d2418a6bd25a8dd962d30 Mon Sep 17 00:00:00 2001 From: DrAlexD Date: Wed, 20 Sep 2023 14:20:04 +0300 Subject: [PATCH 1/3] ### What's done: * Provide fix * Add test Closes #1270 --- .../BracesInConditionalsAndLoopsRule.kt | 10 ++--- .../ruleset/chapter3/BracesRuleFixTest.kt | 6 +++ ...LoopsBracesInsideScopeFunctionsExpected.kt | 41 +++++++++++++++++++ .../LoopsBracesInsideScopeFunctionsTest.kt | 31 ++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt create mode 100644 diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt index 013c5a5678..54bb4589ce 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt @@ -132,11 +132,11 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR if (loopBodyNode == null || loopBodyNode.elementType != BLOCK) { NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode, node.elementType.toString(), node.startOffset, node) { // fixme proper way to calculate indent? or get step size (instead of hardcoded 4) - val indent = node.prevSibling { it.elementType == WHITE_SPACE }!! - .text - .lines() - .last() - .count { it == ' ' } + val indent = node.prevSibling { it.elementType == WHITE_SPACE } + ?.text + ?.lines() + ?.last() + ?.count { it == ' ' } ?: 0 loopBody?.run { replaceWithBlock(indent) } diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt index 39f2be256e..2c877f4d59 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt @@ -22,6 +22,12 @@ class BracesRuleFixTest : FixTestBase("test/paragraph3/braces", ::BracesInCondit fixAndCompare("LoopsBracesExpected.kt", "LoopsBracesTest.kt") } + @Test + @Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS) + fun `should add braces to loops inside scope functions`() { + fixAndCompare("LoopsBracesInsideScopeFunctionsExpected.kt", "LoopsBracesInsideScopeFunctionsTest.kt") + } + @Test @Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS) @Disabled("https://github.com/saveourtool/diktat/issues/1737") diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt new file mode 100644 index 0000000000..48768a6830 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt @@ -0,0 +1,41 @@ +package test.paragraph3.braces + +fun foo1() { + str.apply { + for (i in 1..100) { + println(i) +} + } +} + +fun foo2() { + str.let { + for (i in 1..100) { + println(i) +} + } +} + +fun foo3() { + str.run { + for (i in 1..100) { + println(i) +} + } +} + +fun foo4() { + str.with { + for (i in 1..100) { + println(i) +} + } +} + +fun foo5() { + str.also { + for (i in 1..100) { + println(i) +} + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt new file mode 100644 index 0000000000..4fac8a7899 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt @@ -0,0 +1,31 @@ +package test.paragraph3.braces + +fun foo1() { + str.apply { + for (i in 1..100) println(i) + } +} + +fun foo2() { + str.let { + for (i in 1..100) println(i) + } +} + +fun foo3() { + str.run { + for (i in 1..100) println(i) + } +} + +fun foo4() { + str.with { + for (i in 1..100) println(i) + } +} + +fun foo5() { + str.also { + for (i in 1..100) println(i) + } +} From 77567472822fc240bd7c949531b5976b59b07890 Mon Sep 17 00:00:00 2001 From: DrAlexD Date: Fri, 22 Sep 2023 18:22:25 +0300 Subject: [PATCH 2/3] Added indent handling for the case when condition or loop inside scope function ### What's done: * added findIndentBeforeNode() function * reworked test for loops * added test for conditions --- .../BracesInConditionalsAndLoopsRule.kt | 29 +++++++---- .../ruleset/chapter3/BracesRuleFixTest.kt | 6 +++ ...fElseBracesInsideScopeFunctionsExpected.kt | 51 +++++++++++++++++++ .../IfElseBracesInsideScopeFunctionsTest.kt | 39 ++++++++++++++ ...LoopsBracesInsideScopeFunctionsExpected.kt | 33 ++++++------ .../LoopsBracesInsideScopeFunctionsTest.kt | 15 +++--- 6 files changed, 141 insertions(+), 32 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt create mode 100644 diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt index 54bb4589ce..a449fbcf94 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.KtNodeTypes.BLOCK import org.jetbrains.kotlin.KtNodeTypes.CALL_EXPRESSION import org.jetbrains.kotlin.KtNodeTypes.IF +import org.jetbrains.kotlin.KtNodeTypes.LAMBDA_EXPRESSION import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION import org.jetbrains.kotlin.KtNodeTypes.SAFE_ACCESS_EXPRESSION import org.jetbrains.kotlin.KtNodeTypes.WHEN @@ -66,12 +67,7 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR val thenNode = ifPsi.then?.node val elseKeyword = ifPsi.elseKeyword val elseNode = ifPsi.`else`?.node - val indent = node - .prevSibling { it.elementType == WHITE_SPACE } - ?.text - ?.lines() - ?.last() - ?.count { it == ' ' } ?: 0 + val indent = node.findIndentBeforeNode() if (node.isSingleLineIfElse()) { return @@ -132,11 +128,8 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR if (loopBodyNode == null || loopBodyNode.elementType != BLOCK) { NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode, node.elementType.toString(), node.startOffset, node) { // fixme proper way to calculate indent? or get step size (instead of hardcoded 4) - val indent = node.prevSibling { it.elementType == WHITE_SPACE } - ?.text - ?.lines() - ?.last() - ?.count { it == ' ' } ?: 0 + val indent = node.findIndentBeforeNode() + loopBody?.run { replaceWithBlock(indent) } @@ -152,6 +145,20 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR } } + private fun ASTNode.findIndentBeforeNode(): Int { + val indentNode = if (treeParent?.treeParent?.treeParent?.elementType == LAMBDA_EXPRESSION) { + treeParent.prevSibling { it.elementType == WHITE_SPACE } + } else { + prevSibling { it.elementType == WHITE_SPACE } + } + + return indentNode!! + .text + .lines() + .last() + .count { it == ' ' } + } + @Suppress("UnsafeCallOnNullableType") private fun checkWhenBranches(node: ASTNode) { (node.psi as KtWhenExpression) diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt index 2c877f4d59..395c7b9725 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt @@ -22,6 +22,12 @@ class BracesRuleFixTest : FixTestBase("test/paragraph3/braces", ::BracesInCondit fixAndCompare("LoopsBracesExpected.kt", "LoopsBracesTest.kt") } + @Test + @Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS) + fun `should add braces to if-else statements inside scope functions`() { + fixAndCompare("IfElseBracesInsideScopeFunctionsExpected.kt", "IfElseBracesInsideScopeFunctionsTest.kt") + } + @Test @Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS) fun `should add braces to loops inside scope functions`() { diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt new file mode 100644 index 0000000000..ca25fdef63 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt @@ -0,0 +1,51 @@ +package test.paragraph3.braces + +fun foo1() { + str.apply { + if (x > 0) { + foo() + } else { + bar() + } + } +} + +fun foo2() { + str.let { if (x > 0) { foo() } + else { + bar() + } + } +} + +fun foo3() { + str.run { + while (x > 0) { + if (x > 0) { + foo() + } else { + bar() + } + } + } +} + +fun foo4() { + str.with { while (x > 0) { + if (x > 0) { + foo() + } else { bar() } + } + } +} + +fun foo5() { + str.also { + while (x > 0) { if (x > 0) { + foo() + } else { + bar() + } + } + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt new file mode 100644 index 0000000000..a6481677af --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt @@ -0,0 +1,39 @@ +package test.paragraph3.braces + +fun foo1() { + str.apply { + if (x > 0) foo() + else bar() + } +} + +fun foo2() { + str.let { if (x > 0) { foo() } + else bar() + } +} + +fun foo3() { + str.run { + while (x > 0) { + if (x > 0) foo() + else bar() + } + } +} + +fun foo4() { + str.with { while (x > 0) { + if (x > 0) foo() + else { bar() } + } + } +} + +fun foo5() { + str.also { + while (x > 0) { if (x > 0) foo() + else bar() + } + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt index 48768a6830..3841323460 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsExpected.kt @@ -3,39 +3,42 @@ package test.paragraph3.braces fun foo1() { str.apply { for (i in 1..100) { - println(i) -} + println(i) + } } } fun foo2() { - str.let { - for (i in 1..100) { - println(i) -} + str.let { while (x > 0) { + println(i) + } } } fun foo3() { str.run { - for (i in 1..100) { - println(i) -} + do { + println(i) + } + while (x > 0) } } fun foo4() { - str.with { - for (i in 1..100) { - println(i) -} + str.with { do { + println(i) + } + while (x > 0) } } fun foo5() { str.also { for (i in 1..100) { - println(i) -} + while (x > 0) + { + println(i) + } + } } } diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt index 4fac8a7899..2713e89440 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/LoopsBracesInsideScopeFunctionsTest.kt @@ -7,25 +7,28 @@ fun foo1() { } fun foo2() { - str.let { - for (i in 1..100) println(i) + str.let { while (x > 0) println(i) } } fun foo3() { str.run { - for (i in 1..100) println(i) + do println(i) + while (x > 0) } } fun foo4() { - str.with { - for (i in 1..100) println(i) + str.with { do println(i) + while (x > 0) } } fun foo5() { str.also { - for (i in 1..100) println(i) + for (i in 1..100) { + while (x > 0) + println(i) + } } } From 8f4686f87123573fc60ed18f39002c907f2bb568 Mon Sep 17 00:00:00 2001 From: DrAlexD Date: Mon, 25 Sep 2023 11:25:38 +0300 Subject: [PATCH 3/3] * added `with` to scope functions list * fixed indent handling by using safe call for case when `if` comes after bracket * fixed indent handling for `else if` case * added test for `else if` case --- .../BracesInConditionalsAndLoopsRule.kt | 22 +++++++++++-------- ...fElseBracesInsideScopeFunctionsExpected.kt | 12 ++++++++++ .../IfElseBracesInsideScopeFunctionsTest.kt | 8 +++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt index a449fbcf94..34a5a7666c 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt @@ -12,6 +12,7 @@ import com.saveourtool.diktat.ruleset.utils.prevSibling import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.KtNodeTypes.BLOCK import org.jetbrains.kotlin.KtNodeTypes.CALL_EXPRESSION +import org.jetbrains.kotlin.KtNodeTypes.ELSE import org.jetbrains.kotlin.KtNodeTypes.IF import org.jetbrains.kotlin.KtNodeTypes.LAMBDA_EXPRESSION import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION @@ -146,17 +147,20 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR } private fun ASTNode.findIndentBeforeNode(): Int { - val indentNode = if (treeParent?.treeParent?.treeParent?.elementType == LAMBDA_EXPRESSION) { - treeParent.prevSibling { it.elementType == WHITE_SPACE } + val isElseIfStatement = treeParent.elementType == ELSE + val primaryIfNode = if (isElseIfStatement) treeParent.treeParent else this + + val indentNode = if (primaryIfNode.treeParent?.treeParent?.treeParent?.elementType == LAMBDA_EXPRESSION) { + primaryIfNode.treeParent.prevSibling { it.elementType == WHITE_SPACE } } else { - prevSibling { it.elementType == WHITE_SPACE } + primaryIfNode.prevSibling { it.elementType == WHITE_SPACE } } - return indentNode!! - .text - .lines() - .last() - .count { it == ' ' } + return indentNode + ?.text + ?.lines() + ?.last() + ?.count { it == ' ' } ?: 0 } @Suppress("UnsafeCallOnNullableType") @@ -201,6 +205,6 @@ class BracesInConditionalsAndLoopsRule(configRules: List) : DiktatR companion object { private const val INDENT_STEP = 4 const val NAME_ID = "races-rule" - private val scopeFunctions = listOf("let", "run", "apply", "also") + private val scopeFunctions = listOf("let", "run", "with", "apply", "also") } } diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt index ca25fdef63..84f48b95f5 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsExpected.kt @@ -49,3 +49,15 @@ fun foo5() { } } } + +fun foo6() { + str.apply { + if (x > 0) { + foo() + } else if (y > 0) { + abc() + } else { + bar() + } + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt index a6481677af..9bb93d0350 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBracesInsideScopeFunctionsTest.kt @@ -37,3 +37,11 @@ fun foo5() { } } } + +fun foo6() { + str.apply { + if (x > 0) foo() + else if (y > 0) abc() + else bar() + } +}