Skip to content

Commit

Permalink
Fix multiline if statement
Browse files Browse the repository at this point in the history 
Closes pinterest#828
  • Loading branch information
@paul-dingemans
paul-dingemans committed Aug 2, 2022
1 parent bed5f8f commit 36a9f44
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -127,6 +127,7 @@ The callback function provided as parameter to the format function is now called
* Execute `ktlint` cli on default kotlin extensions only when an (existing) path to a directory is given. ([#917](https://github.com/pinterest/ktlint/issue/917)).
* Invoke callback on `format` function for all errors including errors that are autocorrected ([#1491](https://github.com/pinterest/ktlint/issues/1491))
* Add missing whitespace when else is on same line as true condition `multiline-if-else` ([#1560](https://github.com/pinterest/ktlint/issues/1560))
* Fix multiline if-statements `multiline-if-else` ([#828](https://github.com/pinterest/ktlint/issues/828))


### Changed
Expand Down
67 changes: 52 additions & 15 deletions ...set-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/MultiLineIfElseRule.kt
View file
@@ -1,6 +1,11 @@
package com.pinterest.ktlint.ruleset.standard

import com.pinterest.ktlint.core.IndentConfig
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.api.DefaultEditorConfigProperties
import com.pinterest.ktlint.core.api.EditorConfigProperties
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.ELSE
import com.pinterest.ktlint.core.ast.ElementType.ELSE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.IF
Expand All @@ -9,14 +14,13 @@ import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.RPAR
import com.pinterest.ktlint.core.ast.ElementType.THEN
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isPartOf
import com.pinterest.ktlint.core.ast.isPartOfComment
import com.pinterest.ktlint.core.ast.isWhiteSpace
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithoutNewline
import com.pinterest.ktlint.core.ast.lineIndent
import com.pinterest.ktlint.core.ast.nextSibling
import com.pinterest.ktlint.core.ast.prevLeaf
import com.pinterest.ktlint.core.ast.upsertWhitespaceBeforeMe
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.psi.KtBlockExpression
Expand All @@ -25,23 +29,51 @@ import org.jetbrains.kotlin.psi.psiUtil.leaves
/**
* https://kotlinlang.org/docs/reference/coding-conventions.html#formatting-control-flow-statements
*/
class MultiLineIfElseRule : Rule("multiline-if-else") {
class MultiLineIfElseRule :
Rule("multiline-if-else"),
UsesEditorConfigProperties {
override val editorConfigProperties: List<UsesEditorConfigProperties.EditorConfigProperty<*>> =
listOf(
DefaultEditorConfigProperties.indentSizeProperty,
DefaultEditorConfigProperties.indentStyleProperty,
)
private var indentConfig = IndentConfig.DEFAULT_INDENT_CONFIG

override fun beforeFirstNode(editorConfigProperties: EditorConfigProperties) {
indentConfig = IndentConfig(
indentStyle = editorConfigProperties.getEditorConfigValue(DefaultEditorConfigProperties.indentStyleProperty),
tabWidth = editorConfigProperties.getEditorConfigValue(DefaultEditorConfigProperties.indentSizeProperty)
)
}

override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.elementType == THEN || node.elementType == ELSE) {
if (!node.treePrev.textContains('\n')) { // if (...) <statement>
if (node.firstChildNode?.elementType == BLOCK) {
return
}

if (node.firstChildNode?.firstChildNode?.elementType != LBRACE) {
emit(node.firstChildNode.startOffset, "Missing { ... }", true)
if (autoCorrect) {
autocorrect(node)
if (!node.treePrev.textContains('\n')) {
if (node.firstChildNode.elementType == IF) {
// Allow single line for:
// else if (...)
return
}
if (!node.treeParent.textContains('\n')) {
// Allow single line if statements as long as they are really simple (e.g. do not contain newlines)
// if (...) <statement> // no else statement
// if (...) <statement> else <statement>
return
}
}

emit(node.firstChildNode.startOffset, "Missing { ... }", true)
if (autoCorrect) {
autocorrect(node)
}
}
}

Expand All @@ -58,19 +90,24 @@ class MultiLineIfElseRule : Rule("multiline-if-else") {
.takeWhile { it.isWhiteSpaceWithoutNewline() || it.isPartOfComment() }
.toList()
.dropLastWhile { it.isWhiteSpaceWithoutNewline() }
val rightBraceIndent = node.treeParent
.prevLeaf { it is PsiWhiteSpace && it.textContains('\n') }?.text.orEmpty()
.let { "\n${it.substringAfterLast("\n")}" }

(node.treePrev as LeafPsiElement).rawReplaceWithText(" ")
prevLeaves
.firstOrNull()
.takeIf { it.isWhiteSpace() }
?.let {
(it as LeafPsiElement).rawReplaceWithText(" ")
}
KtBlockExpression(null).apply {
val previousChild = node.firstChildNode
node.replaceChild(node.firstChildNode, this)
addChild(LeafPsiElement(LBRACE, "{"))
prevLeaves.forEach(::addChild)
addChild(PsiWhiteSpaceImpl("\n" + node.lineIndent() + indentConfig.indent))
prevLeaves
.dropWhile { it.isWhiteSpace() }
.forEach(::addChild)
addChild(previousChild)
nextLeaves.forEach(::addChild)
addChild(PsiWhiteSpaceImpl(rightBraceIndent))
addChild(PsiWhiteSpaceImpl("\n" + node.lineIndent()))
addChild(LeafPsiElement(RBRACE, "}"))
}

Expand Down
62 changes: 58 additions & 4 deletions ...standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/MultiLineIfElseRuleTest.kt
View file
Expand Up @@ -480,17 +480,20 @@ class MultiLineIfElseRuleTest {
fun test(a: Int, b: Int, c: Int, d: Int, bar: Boolean) {
foo(
a,
if (bar) b else {
if (bar) {
b
} else {
c
},
d
)
}
""".trimIndent()
multiLineIfElseRuleAssertThat(code)
// TODO: It is not consistent that argument "b" is not wrapped in a block while argument "c" is wrapped
.hasLintViolation(6, 13, "Missing { ... }")
.isFormattedAs(formattedCode)
.hasLintViolations(
LintViolation(5, 18, "Missing { ... }"),
LintViolation(6, 13, "Missing { ... }")
).isFormattedAs(formattedCode)
}

@Test
Expand All @@ -515,4 +518,55 @@ class MultiLineIfElseRuleTest {
LintViolation(3, 5, "Missing { ... }")
).isFormattedAs(formattedCode)
}

@Test
fun `Issue 828 - Given an if statement with multiline statement starting on same line as if`() {
val code =
"""
fun foo() {
if (true) 50
.toString()
}
""".trimIndent()
val formattedCode =
"""
fun foo() {
if (true) {
50
.toString()
}
}
""".trimIndent()
multiLineIfElseRuleAssertThat(code)
.addAdditionalRuleProvider { IndentationRule() }
.hasLintViolation(2, 15, "Missing { ... }")
.isFormattedAs(formattedCode)
}

@Test
fun `Issue 828 - Given an if statement with simple branches but the else branch is on a separate line`() {
val code =
"""
fun foo() {
if (true) 50
else 55
}
""".trimIndent()
val formattedCode =
"""
fun foo() {
if (true) {
50
} else {
55
}
}
""".trimIndent()
multiLineIfElseRuleAssertThat(code)
.addAdditionalRuleProvider { IndentationRule() }
.hasLintViolations(
LintViolation(2, 15, "Missing { ... }"),
LintViolation(3, 10, "Missing { ... }")
).isFormattedAs(formattedCode)
}
}

0 comments on commit 36a9f44

Please sign in to comment.