Skip to content

Commit 3c4e688

Browse files
DrAlexDDrAlexD
authored
Fixed BracesInConditionalsAndLoopsRule (#1750)
### What's done: - Reworked `insertEmptyBlock()` function for adding empty braces blocks to conditions and `do-while` loop with empty bodies. - Added check for braces block existence to avoid duplicate braces blocks. - Added tests for conditions including tests for cases of `else if` and cases of empty `if` and `else` bodies. It's part of #1737
1 parent 17d4bcc commit 3c4e688

File tree

4 files changed

+194
-26
lines changed

4 files changed

+194
-26
lines changed

‎diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt

+65-22
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ import com.saveourtool.diktat.ruleset.utils.isSingleLineIfElse
99
import com.saveourtool.diktat.ruleset.utils.loopType
1010
import com.saveourtool.diktat.ruleset.utils.prevSibling
1111

12-
import org.jetbrains.kotlin.KtNodeTypes
1312
import org.jetbrains.kotlin.KtNodeTypes.BLOCK
13+
import org.jetbrains.kotlin.KtNodeTypes.BLOCK_CODE_FRAGMENT
14+
import org.jetbrains.kotlin.KtNodeTypes.BODY
1415
import org.jetbrains.kotlin.KtNodeTypes.CALL_EXPRESSION
1516
import org.jetbrains.kotlin.KtNodeTypes.ELSE
1617
import org.jetbrains.kotlin.KtNodeTypes.IF
1718
import org.jetbrains.kotlin.KtNodeTypes.LAMBDA_EXPRESSION
1819
import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION
1920
import org.jetbrains.kotlin.KtNodeTypes.SAFE_ACCESS_EXPRESSION
21+
import org.jetbrains.kotlin.KtNodeTypes.THEN
2022
import org.jetbrains.kotlin.KtNodeTypes.WHEN
2123
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
2224
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
@@ -84,8 +86,7 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
8486
}
8587
}
8688
?: run {
87-
val nodeAfterCondition = ifPsi.rightParenthesis!!.node.treeNext
88-
node.insertEmptyBlockBetweenChildren(nodeAfterCondition, nodeAfterCondition, indent)
89+
node.insertEmptyBlockInsideThenNode(indent)
8990
}
9091
}
9192
}
@@ -116,18 +117,53 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
116117
}
117118
?: run {
118119
// `else` can have empty body e.g. when there is a semicolon after: `else ;`
119-
node.insertEmptyBlockBetweenChildren(elseKeyword.node.treeNext, null, indent)
120+
node.insertEmptyBlockInsideElseNode(indent)
120121
}
121122
}
122123
}
123124
}
124125

126+
private fun ASTNode.insertEmptyBlockInsideThenNode(indent: Int) {
127+
val ifPsi = psi as KtIfExpression
128+
val elseKeyword = ifPsi.elseKeyword
129+
val emptyThenNode = findChildByType(THEN)
130+
131+
emptyThenNode?.findChildByType(BLOCK_CODE_FRAGMENT) ?: run {
132+
val whiteSpacesAfterCondition = ifPsi.rightParenthesis?.node?.treeNext
133+
134+
whiteSpacesAfterCondition?.let {
135+
replaceChild(it, PsiWhiteSpaceImpl(" "))
136+
}
137+
emptyThenNode?.insertEmptyBlock(indent)
138+
elseKeyword?.let {
139+
addChild(PsiWhiteSpaceImpl(" "), elseKeyword.node)
140+
}
141+
}
142+
}
143+
144+
private fun ASTNode.insertEmptyBlockInsideElseNode(indent: Int) {
145+
val ifPsi = psi as KtIfExpression
146+
val elseKeyword = ifPsi.elseKeyword
147+
val emptyElseNode = findChildByType(ELSE)
148+
149+
emptyElseNode?.findChildByType(BLOCK_CODE_FRAGMENT) ?: run {
150+
val whiteSpacesAfterElseKeyword = elseKeyword?.node?.treeNext
151+
152+
whiteSpacesAfterElseKeyword?.let {
153+
replaceChild(it, PsiWhiteSpaceImpl(" "))
154+
}
155+
emptyElseNode?.insertEmptyBlock(indent)
156+
}
157+
}
158+
125159
@Suppress("UnsafeCallOnNullableType")
126160
private fun checkLoop(node: ASTNode) {
127161
val loopBody = (node.psi as KtLoopExpression).body
128162
val loopBodyNode = loopBody?.node
163+
129164
if (loopBodyNode == null || loopBodyNode.elementType != BLOCK) {
130-
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode, node.elementType.toString(), node.startOffset, node) {
165+
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode,
166+
node.elementType.toString(), node.startOffset, node) {
131167
// fixme proper way to calculate indent? or get step size (instead of hardcoded 4)
132168
val indent = node.findIndentBeforeNode()
133169

@@ -136,16 +172,29 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
136172
}
137173
?: run {
138174
// this corresponds to do-while with empty body
139-
node.insertEmptyBlockBetweenChildren(
140-
node.findChildByType(DO_KEYWORD)!!.treeNext,
141-
node.findChildByType(WHILE_KEYWORD)!!.treePrev,
142-
indent
143-
)
175+
node.insertEmptyBlockInsideDoWhileNode(indent)
144176
}
145177
}
146178
}
147179
}
148180

181+
private fun ASTNode.insertEmptyBlockInsideDoWhileNode(indent: Int) {
182+
findChildByType(BODY) ?: run {
183+
val doKeyword = findChildByType(DO_KEYWORD)
184+
val whileKeyword = findChildByType(WHILE_KEYWORD)
185+
val whiteSpacesAfterDoKeyword = doKeyword?.treeNext
186+
187+
addChild(CompositeElement(BODY), whileKeyword)
188+
val emptyWhenNode = findChildByType(BODY)
189+
190+
whiteSpacesAfterDoKeyword?.let {
191+
replaceChild(it, PsiWhiteSpaceImpl(" "))
192+
}
193+
emptyWhenNode?.insertEmptyBlock(indent)
194+
addChild(PsiWhiteSpaceImpl(" "), whileKeyword)
195+
}
196+
}
197+
149198
private fun ASTNode.findIndentBeforeNode(): Int {
150199
val isElseIfStatement = treeParent.elementType == ELSE
151200
val primaryIfNode = if (isElseIfStatement) treeParent.treeParent else this
@@ -175,7 +224,8 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
175224
block.findChildrenMatching { it.isPartOfComment() }.isEmpty()
176225
}
177226
.forEach {
178-
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode, "WHEN", it.node.startOffset, it.node) {
227+
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode,
228+
"WHEN", it.node.startOffset, it.node) {
179229
it.astReplace(it.firstStatement!!.node.psi)
180230
}
181231
}
@@ -187,21 +237,14 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
187237
))
188238
}
189239

190-
private fun ASTNode.insertEmptyBlockBetweenChildren(
191-
firstChild: ASTNode,
192-
secondChild: ASTNode?,
193-
indent: Int
194-
) {
195-
val emptyBlock = CompositeElement(KtNodeTypes.BLOCK_CODE_FRAGMENT)
196-
addChild(emptyBlock, firstChild)
197-
addChild(PsiWhiteSpaceImpl(" "), emptyBlock)
240+
private fun ASTNode.insertEmptyBlock(indent: Int) {
241+
val emptyBlock = CompositeElement(BLOCK_CODE_FRAGMENT)
242+
addChild(emptyBlock, null)
198243
emptyBlock.addChild(LeafPsiElement(LBRACE, "{"))
199244
emptyBlock.addChild(PsiWhiteSpaceImpl("\n${" ".repeat(indent)}"))
200245
emptyBlock.addChild(LeafPsiElement(RBRACE, "}"))
201-
secondChild?.let {
202-
replaceChild(it, PsiWhiteSpaceImpl(" "))
203-
}
204246
}
247+
205248
companion object {
206249
private const val INDENT_STEP = 4
207250
const val NAME_ID = "races-rule"

‎diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt

-3
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.BracesInConditionalsAndLoop
44
import com.saveourtool.diktat.util.FixTestBase
55

66
import generated.WarningNames
7-
import org.junit.jupiter.api.Disabled
87
import org.junit.jupiter.api.Tag
98
import org.junit.jupiter.api.Test
109

1110
class BracesRuleFixTest : FixTestBase("test/paragraph3/braces", ::BracesInConditionalsAndLoopsRule) {
1211
@Test
1312
@Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS)
14-
@Disabled("https://github.com/saveourtool/diktat/issues/1737")
1513
fun `should add braces to if-else statements - 1`() {
1614
fixAndCompare("IfElseBraces1Expected.kt", "IfElseBraces1Test.kt")
1715
}
@@ -36,7 +34,6 @@ class BracesRuleFixTest : FixTestBase("test/paragraph3/braces", ::BracesInCondit
3634

3735
@Test
3836
@Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS)
39-
@Disabled("https://github.com/saveourtool/diktat/issues/1737")
4037
fun `should add braces to do-while loops with empty body`() {
4138
fixAndCompare("DoWhileBracesExpected.kt", "DoWhileBracesTest.kt")
4239
}

‎diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBraces1Expected.kt

+79-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,85 @@ fun foo3() {
2727
fun foo4() {
2828
if (x > 0) {
2929
} else {
30-
} ;
30+
};
31+
}
32+
33+
fun foo5() {
34+
if (x > 0)
35+
{
36+
foo()
37+
} else
38+
{
39+
bar()
40+
}
41+
}
42+
43+
fun foo6() {
44+
if (x > 0) {
45+
foo()
46+
} else if (y > 0) {
47+
abc()
48+
} else {
49+
bar()
50+
}
51+
}
52+
53+
fun foo7() {
54+
if (x > 0)
55+
{
56+
foo()
57+
} else if (y > 0)
58+
{
59+
abc()
60+
} else
61+
{
62+
bar()
63+
}
64+
}
65+
66+
fun foo8() {
67+
if (x > 0) {
68+
if (y > 0) foo() else abc()
69+
} else {
70+
bar()
71+
}
72+
}
73+
74+
fun foo9() {
75+
if (x > 0) {
76+
foo()
77+
} else if (y > 0) {
78+
abc()
79+
} else {
80+
bar()
81+
}
82+
}
83+
84+
fun foo10() {
85+
if (x > 0) {
86+
foo()
87+
} else if (z > 0) {
88+
if (y > 0) abc() else qwe()
89+
} else {
90+
bar()
91+
}
92+
}
93+
94+
fun foo11() {
95+
if (x > 0) else bar()
96+
}
97+
98+
fun foo12() {
99+
if (x > 0) {
100+
foo()
101+
} else {
102+
};
103+
}
104+
105+
fun foo13() {
106+
if (x > 0) {
107+
} else {
108+
};
31109
}
32110

33111
fun foo() {

‎diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBraces1Test.kt

+50
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,56 @@ fun foo4() {
2525
else ;
2626
}
2727

28+
fun foo5() {
29+
if (x > 0)
30+
foo()
31+
else
32+
bar()
33+
}
34+
35+
fun foo6() {
36+
if (x > 0) foo()
37+
else if (y > 0) abc()
38+
else bar()
39+
}
40+
41+
fun foo7() {
42+
if (x > 0)
43+
foo()
44+
else if (y > 0)
45+
abc()
46+
else
47+
bar()
48+
}
49+
50+
fun foo8() {
51+
if (x > 0) if (y > 0) foo() else abc()
52+
else bar()
53+
}
54+
55+
fun foo9() {
56+
if (x > 0) foo()
57+
else if (y > 0) abc() else bar()
58+
}
59+
60+
fun foo10() {
61+
if (x > 0) foo()
62+
else if (z > 0) if (y > 0) abc() else qwe()
63+
else bar()
64+
}
65+
66+
fun foo11() {
67+
if (x > 0) else bar()
68+
}
69+
70+
fun foo12() {
71+
if (x > 0) foo() else ;
72+
}
73+
74+
fun foo13() {
75+
if (x > 0) else ;
76+
}
77+
2878
fun foo() {
2979
if (a) {
3080
bar()

0 commit comments

Comments
 (0)