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

Fixed BracesInConditionalsAndLoopsRule and added tests #1750

Merged
merged 3 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ import com.saveourtool.diktat.ruleset.utils.isSingleLineIfElse
import com.saveourtool.diktat.ruleset.utils.loopType
import com.saveourtool.diktat.ruleset.utils.prevSibling

import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.KtNodeTypes.BLOCK
import org.jetbrains.kotlin.KtNodeTypes.BLOCK_CODE_FRAGMENT
import org.jetbrains.kotlin.KtNodeTypes.BODY
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
import org.jetbrains.kotlin.KtNodeTypes.SAFE_ACCESS_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.THEN
import org.jetbrains.kotlin.KtNodeTypes.WHEN
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
Expand Down Expand Up @@ -84,8 +86,17 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
}
}
?: run {
val nodeAfterCondition = ifPsi.rightParenthesis!!.node.treeNext
node.insertEmptyBlockBetweenChildren(nodeAfterCondition, nodeAfterCondition, indent)
val emptyThenNode = node.findChildByType(THEN)!!

if (emptyThenNode.findChildByType(BLOCK_CODE_FRAGMENT) == null) {
val whiteSpacesAfterCondition = ifPsi.rightParenthesis!!.node.treeNext

node.replaceChild(whiteSpacesAfterCondition, PsiWhiteSpaceImpl(" "))
emptyThenNode.insertEmptyBlock(indent)
if (elseKeyword != null) {
node.addChild(PsiWhiteSpaceImpl(" "), elseKeyword.node)
}
}
}
}
}
Expand Down Expand Up @@ -116,7 +127,14 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
}
?: run {
// `else` can have empty body e.g. when there is a semicolon after: `else ;`
node.insertEmptyBlockBetweenChildren(elseKeyword.node.treeNext, null, indent)
val emptyElseNode = node.findChildByType(ELSE)!!

if (emptyElseNode.findChildByType(BLOCK_CODE_FRAGMENT) == null) {
val whiteSpacesAfterElseKeyword = elseKeyword.node.treeNext

node.replaceChild(whiteSpacesAfterElseKeyword, PsiWhiteSpaceImpl(" "))
emptyElseNode.insertEmptyBlock(indent)
}
}
}
}
Expand All @@ -126,6 +144,7 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
private fun checkLoop(node: ASTNode) {
val loopBody = (node.psi as KtLoopExpression).body
val loopBodyNode = loopBody?.node

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)
Expand All @@ -136,11 +155,19 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
}
?: run {
// this corresponds to do-while with empty body
node.insertEmptyBlockBetweenChildren(
node.findChildByType(DO_KEYWORD)!!.treeNext,
node.findChildByType(WHILE_KEYWORD)!!.treePrev,
indent
)

if (node.findChildByType(BODY) == null) {
val doKeyword = node.findChildByType(DO_KEYWORD)!!
val whileKeyword = node.findChildByType(WHILE_KEYWORD)!!
val whiteSpacesAfterDoKeyword = doKeyword.treeNext

node.addChild(CompositeElement(BODY), whileKeyword)
val emptyWhenNode = node.findChildByType(BODY)!!

node.replaceChild(whiteSpacesAfterDoKeyword, PsiWhiteSpaceImpl(" "))
emptyWhenNode.insertEmptyBlock(indent)
node.addChild(PsiWhiteSpaceImpl(" "), whileKeyword)
}
}
}
}
Expand Down Expand Up @@ -187,21 +214,14 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
))
}

private fun ASTNode.insertEmptyBlockBetweenChildren(
firstChild: ASTNode,
secondChild: ASTNode?,
indent: Int
) {
val emptyBlock = CompositeElement(KtNodeTypes.BLOCK_CODE_FRAGMENT)
addChild(emptyBlock, firstChild)
addChild(PsiWhiteSpaceImpl(" "), emptyBlock)
private fun ASTNode.insertEmptyBlock(indent: Int) {
val emptyBlock = CompositeElement(BLOCK_CODE_FRAGMENT)
addChild(emptyBlock, null)
emptyBlock.addChild(LeafPsiElement(LBRACE, "{"))
emptyBlock.addChild(PsiWhiteSpaceImpl("\n${" ".repeat(indent)}"))
emptyBlock.addChild(LeafPsiElement(RBRACE, "}"))
secondChild?.let {
replaceChild(it, PsiWhiteSpaceImpl(" "))
}
}

companion object {
private const val INDENT_STEP = 4
const val NAME_ID = "races-rule"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.BracesInConditionalsAndLoop
import com.saveourtool.diktat.util.FixTestBase

import generated.WarningNames
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

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

@Test
@Tag(WarningNames.NO_BRACES_IN_CONDITIONALS_AND_LOOPS)
@Disabled("https://github.com/saveourtool/diktat/issues/1737")
fun `should add braces to do-while loops with empty body`() {
fixAndCompare("DoWhileBracesExpected.kt", "DoWhileBracesTest.kt")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,85 @@ fun foo3() {
fun foo4() {
if (x > 0) {
} else {
} ;
};
}

fun foo5() {
if (x > 0)
{
foo()
} else
{
bar()
}
}

fun foo6() {
if (x > 0) {
foo()
} else if (y > 0) {
abc()
} else {
bar()
}
}

fun foo7() {
if (x > 0)
{
foo()
} else if (y > 0)
{
abc()
} else
{
bar()
}
}

fun foo8() {
if (x > 0) {
if (y > 0) foo() else abc()
} else {
bar()
}
}

fun foo9() {
if (x > 0) {
foo()
} else if (y > 0) {
abc()
} else {
bar()
}
}

fun foo10() {
if (x > 0) {
foo()
} else if (z > 0) {
if (y > 0) abc() else qwe()
} else {
bar()
}
}

fun foo11() {
if (x > 0) else bar()
}

fun foo12() {
if (x > 0) {
foo()
} else {
};
}

fun foo13() {
if (x > 0) {
} else {
};
}

fun foo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,56 @@ fun foo4() {
else ;
}

fun foo5() {
if (x > 0)
foo()
else
bar()
}

fun foo6() {
if (x > 0) foo()
else if (y > 0) abc()
else bar()
}

fun foo7() {
if (x > 0)
foo()
else if (y > 0)
abc()
else
bar()
}

fun foo8() {
if (x > 0) if (y > 0) foo() else abc()
else bar()
}

fun foo9() {
if (x > 0) foo()
else if (y > 0) abc() else bar()
}

fun foo10() {
if (x > 0) foo()
else if (z > 0) if (y > 0) abc() else qwe()
else bar()
}

fun foo11() {
if (x > 0) else bar()
}

fun foo12() {
if (x > 0) foo() else ;
}

fun foo13() {
if (x > 0) else ;
}

fun foo() {
if (a) {
bar()
Expand Down