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

Add missing whitespace when else is on same line as true condition #1565

Merged
merged 4 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ The callback function provided as parameter to the format function is now called
* When a glob is specified then ensure that it matches files in the current directory and not only in subdirectories of the current directory ([#1533](https://github.com/pinterest/ktlint/issue/1533)).
* 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))
* Prevent class cast exception on ".editorconfig" property `ktlint_code_style` ([#1559](https://github.com/pinterest/ktlint/issues/1559))
* Handle trailing comma in enums `trailing-comma` ([#1542](https://github.com/pinterest/ktlint/pull/1542))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ internal object SuppressionLocatorBuilder {
val hintsList = collect(rootNode)
return if (hintsList.isEmpty()) {
noSuppression
} else { offset, ruleId, isRoot ->
} else {
toSuppressedRegionsLocator(hintsList)
}
}

private fun toSuppressedRegionsLocator(hintsList: List<SuppressionHint>): SuppressionLocator =
{ offset, ruleId, isRoot ->
hintsList
.filter { offset in it.range }
.any { hint -> hint.disabledRules.isEmpty() || hint.disabledRules.contains(ruleId) }
}
}

/**
* @param range zero-based range of lines where lint errors should be suppressed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ class AnnotationSpacingRule : Rule("annotation-spacing") {
val s = it.text
// Ensure at least one occurrence of two line breaks
s.indexOf("\n") != s.lastIndexOf("\n")
} else it.isPartOfComment() && !it.isCommentOnSameLineAsPrevLeaf()
} else {
it.isPartOfComment() && !it.isCommentOnSameLineAsPrevLeaf()
}
}
)
if (next != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
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
import com.pinterest.ktlint.core.ast.ElementType.LBRACE
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.isPartOfComment
import com.pinterest.ktlint.core.ast.isWhiteSpace
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithoutNewline
import com.pinterest.ktlint.core.ast.prevLeaf
import com.pinterest.ktlint.core.ast.lineIndent
import com.pinterest.ktlint.core.ast.nextSibling
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 @@ -20,50 +29,100 @@ 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)
}
}
}

private fun autocorrect(node: ASTNode) {
val prevLeaves =
node.leaves(forward = false).takeWhile { it.elementType !in listOf(RPAR, ELSE_KEYWORD) }.toList().reversed()
node
.leaves(forward = false)
.takeWhile { it.elementType !in listOf(RPAR, ELSE_KEYWORD) }
.toList()
.reversed()
val nextLeaves =
node.leaves(forward = true).takeWhile { it.isWhiteSpaceWithoutNewline() || it.isPartOfComment() }.toList()
val rightBraceIndent = node.treeParent
.prevLeaf { it is PsiWhiteSpace && it.textContains('\n') }?.text.orEmpty()
.let { "\n${it.substringAfterLast("\n")}" }
node
.leaves(forward = true)
.takeWhile { it.isWhiteSpaceWithoutNewline() || it.isPartOfComment() }
.toList()
.dropLastWhile { it.isWhiteSpaceWithoutNewline() }

(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, "}"))
}

// Make sure else starts on same line as newly inserted right brace
if (node.elementType == THEN && node.treeNext?.treeNext?.elementType == ELSE_KEYWORD) {
node.treeParent.replaceChild(node.treeNext, PsiWhiteSpaceImpl(" "))
if (node.elementType == THEN) {
node
.nextSibling { !it.isPartOfComment() }
?.let { nextSibling ->
if (nextSibling.elementType == ELSE_KEYWORD) {
(nextSibling as LeafPsiElement).upsertWhitespaceBeforeMe(" ")
}
if (nextSibling.elementType == WHITE_SPACE && nextSibling.text != " ") {
(nextSibling as LeafPsiElement).rawReplaceWithText(" ")
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -480,16 +480,93 @@ 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 { ... }")
.hasLintViolations(
LintViolation(5, 18, "Missing { ... }"),
LintViolation(6, 13, "Missing { ... }")
).isFormattedAs(formattedCode)
}

@Test
fun `Issue 1560 - Given an if statement with else keyword on same line as true branch`() {
val code =
"""
fun foo() = if (bar())
"a" else
"b"
""".trimIndent()
val formattedCode =
"""
fun foo() = if (bar()) {
"a"
} else {
"b"
}
""".trimIndent()
multiLineIfElseRuleAssertThat(code)
.hasLintViolations(
LintViolation(2, 5, "Missing { ... }"),
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)
}
}