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.

Sign up for GitHub

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

Jump to bottom

Fixed BracesInConditionalsAndLoopsRule and added tests #1750

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Fixed BracesInConditionalsAndLoopsRule and added tests #1750

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e09f81
Fixed BracesInConditionalsAndLoopsRule
DrAlexD Sep 25, 2023
bc3f1d4
- extracted functions for inserting empty blocks inside `if`,`else` a…
DrAlexD Sep 25, 2023
3fdde5f
- changed to null safe calls
DrAlexD Sep 25, 2023
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
81 changes: 59 additions & 22 deletions .../kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/BracesInConditionalsAndLoopsRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@
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,7 @@
}
}
?: run {
val nodeAfterCondition = ifPsi.rightParenthesis!!.node.treeNext
node.insertEmptyBlockBetweenChildren(nodeAfterCondition, nodeAfterCondition, indent)
node.insertEmptyBlockInsideThenNode(indent)
}
}
}
Expand Down Expand Up @@ -116,18 +117,49 @@
}
?: run {
// `else` can have empty body e.g. when there is a semicolon after: `else ;`
node.insertEmptyBlockBetweenChildren(elseKeyword.node.treeNext, null, indent)
node.insertEmptyBlockInsideElseNode(indent)
}
}
}
}

private fun ASTNode.insertEmptyBlockInsideThenNode(indent: Int) {
val ifPsi = psi as KtIfExpression
val elseKeyword = ifPsi.elseKeyword
val emptyThenNode = findChildByType(THEN)!!
Fixed Show fixed Hide fixed

emptyThenNode.findChildByType(BLOCK_CODE_FRAGMENT) ?: run {
val whiteSpacesAfterCondition = ifPsi.rightParenthesis!!.node.treeNext
Fixed Show fixed Hide fixed

replaceChild(whiteSpacesAfterCondition, PsiWhiteSpaceImpl(" "))
emptyThenNode.insertEmptyBlock(indent)
elseKeyword?.let {
addChild(PsiWhiteSpaceImpl(" "), elseKeyword.node)
}
}
}

private fun ASTNode.insertEmptyBlockInsideElseNode(indent: Int) {
val ifPsi = psi as KtIfExpression
val elseKeyword = ifPsi.elseKeyword
val emptyElseNode = findChildByType(ELSE)!!
Fixed Show fixed Hide fixed

emptyElseNode.findChildByType(BLOCK_CODE_FRAGMENT) ?: run {
val whiteSpacesAfterElseKeyword = elseKeyword!!.node.treeNext
Fixed Show fixed Hide fixed

replaceChild(whiteSpacesAfterElseKeyword, PsiWhiteSpaceImpl(" "))
emptyElseNode.insertEmptyBlock(indent)
}
}

@Suppress("UnsafeCallOnNullableType")
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) {
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.findIndentBeforeNode()

Expand All @@ -136,16 +168,27 @@
}
?: run {
// this corresponds to do-while with empty body
node.insertEmptyBlockBetweenChildren(
node.findChildByType(DO_KEYWORD)!!.treeNext,
node.findChildByType(WHILE_KEYWORD)!!.treePrev,
indent
)
node.insertEmptyBlockInsideDoWhileNode(indent)
}
}
}
}

private fun ASTNode.insertEmptyBlockInsideDoWhileNode(indent: Int) {
findChildByType(BODY) ?: run {
val doKeyword = findChildByType(DO_KEYWORD)!!
Fixed Show fixed Hide fixed
val whileKeyword = findChildByType(WHILE_KEYWORD)!!
Fixed Show fixed Hide fixed
val whiteSpacesAfterDoKeyword = doKeyword.treeNext

addChild(CompositeElement(BODY), whileKeyword)
val emptyWhenNode = findChildByType(BODY)!!
Fixed Show fixed Hide fixed

replaceChild(whiteSpacesAfterDoKeyword, PsiWhiteSpaceImpl(" "))
emptyWhenNode.insertEmptyBlock(indent)
addChild(PsiWhiteSpaceImpl(" "), whileKeyword)
}
}

private fun ASTNode.findIndentBeforeNode(): Int {
val isElseIfStatement = treeParent.elementType == ELSE
val primaryIfNode = if (isElseIfStatement) treeParent.treeParent else this
Expand Down Expand Up @@ -175,7 +218,8 @@
block.findChildrenMatching { it.isPartOfComment() }.isEmpty()
}
.forEach {
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode, "WHEN", it.node.startOffset, it.node) {
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode,
"WHEN", it.node.startOffset, it.node) {
it.astReplace(it.firstStatement!!.node.psi)
}
}
Expand All @@ -187,21 +231,14 @@
))
}

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
3 changes: 0 additions & 3 deletions diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/BracesRuleFixTest.kt
View file
Open in desktop
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
80 changes: 79 additions & 1 deletion diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBraces1Expected.kt
View file
Open in desktop
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
50 changes: 50 additions & 0 deletions diktat-rules/src/test/resources/test/paragraph3/braces/IfElseBraces1Test.kt
View file
Open in desktop
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
Loading