Merge pull request #1058 from t-kameyama/issue_1057

Fix formatting with comments (`colon-spacing`)
pinterest · Jan 16, 2021 · aa03a6b · aa03a6b
2 parents 12d7591 + 75f3bc7
commit aa03a6b
Show file tree

Hide file tree

Showing 3 changed files with 199 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Fix internal error in `no-unused-imports` ([#1040](https://github.com/pinterest/ktlint/issues/1040))
 - Fix false positives when declaration has tail comments (`spacing-between-declarations-with-comments`) ([#1053](https://github.com/pinterest/ktlint/issues/1053))
 - Fix false positive after `else` keyword (`argument-list-wrapping`) ([#1047](https://github.com/pinterest/ktlint/issues/1047))
+- Fix formatting with comments (`colon-spacing`) ([#1057](https://github.com/pinterest/ktlint/issues/1057))
 
 ### Changed
 - Update Gradle shadow plugin to `6.1.0` version

diff --git a/...-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt b/...-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt
@@ -3,20 +3,28 @@ package com.pinterest.ktlint.ruleset.standard
 import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION
 import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY
+import com.pinterest.ktlint.core.ast.ElementType.EQ
 import com.pinterest.ktlint.core.ast.isPartOf
 import com.pinterest.ktlint.core.ast.isPartOfComment
 import com.pinterest.ktlint.core.ast.isPartOfString
+import com.pinterest.ktlint.core.ast.isWhiteSpace
 import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline
+import com.pinterest.ktlint.core.ast.nextLeaf
 import com.pinterest.ktlint.core.ast.prevLeaf
 import com.pinterest.ktlint.core.ast.upsertWhitespaceAfterMe
 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.psi.KtBlockExpression
 import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.jetbrains.kotlin.psi.KtConstructor
+import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.psi.KtProperty
 import org.jetbrains.kotlin.psi.KtTypeConstraint
 import org.jetbrains.kotlin.psi.KtTypeParameterList
+import org.jetbrains.kotlin.psi.psiUtil.siblings
+import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
 
 class SpacingAroundColonRule : Rule("colon-spacing") {
 
@@ -36,24 +44,51 @@ class SpacingAroundColonRule : Rule("colon-spacing") {
                     node.parent !is KtTypeConstraint && // where T : S
                     node.parent?.parent !is KtTypeParameterList
             val prevLeaf = node.prevLeaf()
-            if (prevLeaf != null &&
-                prevLeaf.isWhiteSpaceWithNewline() &&
-                // FIXME: relocate : so that it would be in front of comment
-                // (see SpacingAroundColonRuleTest.testFormatEOF & ChainWrappingRule)
-                prevLeaf.prevLeaf()?.isPartOfComment() != true
-            ) {
+            if (prevLeaf != null && prevLeaf.isWhiteSpaceWithNewline()) {
                 emit(prevLeaf.startOffset, "Unexpected newline before \":\"", true)
-                val text = prevLeaf.text
                 if (autoCorrect) {
-                    if (removeSpacingBefore) {
-                        prevLeaf.treeParent.removeChild(prevLeaf)
-                    } else {
-                        (prevLeaf as LeafPsiElement).rawReplaceWithText(" ")
+                    val parent = node.parent
+                    val prevNonCodeElements = node.siblings(forward = false, withItself = false)
+                        .takeWhile { it.node.isWhiteSpace() || it.node.isPartOfComment() }.toList()
+                    when {
+                        parent is KtProperty || parent is KtNamedFunction -> {
+                            val equalsSignElement = node.siblings(forward = true, withItself = false)
+                                .firstOrNull { it.node.elementType == EQ }
+                            if (equalsSignElement != null) {
+                                equalsSignElement.nextSibling?.takeIf { it.node.isWhiteSpace() }?.delete()
+                                prevNonCodeElements.forEach { parent.addAfter(it, equalsSignElement) }
+                            }
+                            val blockElement = node.siblings(forward = true, withItself = false)
+                                .firstIsInstanceOrNull<KtBlockExpression>()
+                            if (blockElement != null) {
+                                prevNonCodeElements
+                                    .let { if (it.first().node.isWhiteSpace()) it.drop(1) else it }
+                                    .forEach { blockElement.addAfter(it, blockElement.lBrace) }
+                            }
+                            parent.deleteChildRange(prevNonCodeElements.last(), prevNonCodeElements.first())
+                        }
+                        prevLeaf.prevLeaf()?.isPartOfComment() == true -> {
+                            val nextLeaf = node.nextLeaf()
+                            prevNonCodeElements.reversed().forEach {
+                                node.treeParent.addChild(it.node, nextLeaf)
+                            }
+                            if (nextLeaf != null && nextLeaf.isWhiteSpace()) {
+                                node.treeParent.removeChild(nextLeaf)
+                            }
+                        }
+                        else -> {
+                            val text = prevLeaf.text
+                            if (removeSpacingBefore) {
+                                prevLeaf.treeParent.removeChild(prevLeaf)
+                            } else {
+                                (prevLeaf as LeafPsiElement).rawReplaceWithText(" ")
+                            }
+                            node.upsertWhitespaceAfterMe(text)
+                        }
                     }
-                    node.upsertWhitespaceAfterMe(text)
                 }
             }
-            if (node.prevSibling is PsiWhiteSpace && removeSpacingBefore) {
+            if (node.prevSibling is PsiWhiteSpace && removeSpacingBefore && !prevLeaf.isWhiteSpaceWithNewline()) {
                 emit(node.startOffset, "Unexpected spacing before \":\"", true)
                 if (autoCorrect) {
                     node.prevSibling.node.treeParent.removeChild(node.prevSibling.node)

diff --git a/...ndard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt b/...ndard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt
@@ -248,14 +248,158 @@ class SpacingAroundColonRuleTest {
             class X :
                 Y,
                 Z
-            class A // comment
-                : B
-            class A /*
+            class A : // comment
+                B
+            class A : /*
 
             */
-                : B
-            val xmlFormatter:
-                String = ""
+                B
+            val xmlFormatter: String =
+                ""
+            """.trimIndent()
+        )
+    }
+
+    // https://github.com/pinterest/ktlint/issues/1057
+    @Test
+    fun testLintNewLineBeforeColon() {
+        assertThat(
+            SpacingAroundColonRule().lint(
+                """
+                fun test() {
+                    val v1
+                        : Int = 1
+
+                    val v2 // comment
+                        : Int = 1
+
+                    val v3
+                        // comment
+                        : Int = 1
+
+                    fun f1()
+                        : Int = 1
+
+                    fun f2() // comment
+                        : Int = 1
+
+                    fun f3()
+                        // comment
+                        : Int = 1
+
+                    fun g1()
+                        : Int {
+                        return 1
+                    }
+
+                    fun g2() // comment
+                        : Int {
+                        return 1
+                    }
+
+                    fun g3()
+                        // comment
+                        : Int {
+                        return 1
+                    }
+                }
+                """.trimIndent()
+            )
+        ).isEqualTo(
+            listOf(
+                LintError(2, 11, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(5, 22, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(9, 19, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(12, 13, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(15, 24, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(19, 19, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(22, 13, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(27, 24, "colon-spacing", "Unexpected newline before \":\""),
+                LintError(33, 19, "colon-spacing", "Unexpected newline before \":\""),
+            )
+        )
+    }
+
+    @Test
+    fun testFormatNewLineBeforeColon() {
+        assertThat(
+            SpacingAroundColonRule().format(
+                """
+                fun test() {
+                    val v1
+                        : Int = 1
+
+                    val v2 // comment
+                        : Int = 1
+
+                    val v3
+                        // comment
+                        : Int = 1
+
+                    fun f1()
+                        : Int = 1
+
+                    fun f2() // comment
+                        : Int = 1
+
+                    fun f3()
+                        // comment
+                        : Int = 1
+
+                    fun g1()
+                        : Int {
+                        return 1
+                    }
+
+                    fun g2() // comment
+                        : Int {
+                        return 1
+                    }
+
+                    fun g3()
+                        // comment
+                        : Int {
+                        return 1
+                    }
+                }
+                """.trimIndent()
+            )
+        ).isEqualTo(
+            """
+            fun test() {
+                val v1: Int =
+                    1
+
+                val v2: Int = // comment
+                    1
+
+                val v3: Int =
+                    // comment
+                    1
+
+                fun f1(): Int =
+                    1
+
+                fun f2(): Int = // comment
+                    1
+
+                fun f3(): Int =
+                    // comment
+                    1
+
+                fun g1(): Int {
+                    return 1
+                }
+
+                fun g2(): Int { // comment
+                    return 1
+                }
+
+                fun g3(): Int {
+                    // comment
+                    return 1
+                }
+            }
             """.trimIndent()
         )
     }