Enforce a newline between a trailing comma and the arrow of a when en…

…try when trailing-comma on declaration site is enabled Closes pinterest#1519
paul-dingemans · Aug 7, 2022 · 507e483 · 507e483
1 parent aa1a664
commit 507e483
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -149,6 +149,7 @@ Although, Ktlint 0.47.0 falls back on property `disabled_rules` whenever `ktlint
 * Handle trailing comma in enums `trailing-comma` ([#1542](https://github.com/pinterest/ktlint/pull/1542))
 * Split rule `trailing-comma` into `trailing-comma-on-call-site` and `trailing-comma-on-declaration-site` ([#1555](https://github.com/pinterest/ktlint/pull/1555))
 * Fix indent of when entry with a dot qualified expression instead of simple value when trailing comma is required ([#1519](https://github.com/pinterest/ktlint/pull/1519))
+* Fix whitespace between trailing comma and arrow in when entry when trailing comma is required ([#1519](https://github.com/pinterest/ktlint/pull/1519))
 
 ### Changed
 

diff --git a/...c/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaOnDeclarationSiteRule.kt b/...c/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaOnDeclarationSiteRule.kt
@@ -4,18 +4,23 @@ import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.api.EditorConfigProperties
 import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
 import com.pinterest.ktlint.core.ast.ElementType
+import com.pinterest.ktlint.core.ast.ElementType.ARROW
+import com.pinterest.ktlint.core.ast.ElementType.WHEN_ENTRY
+import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
 import com.pinterest.ktlint.core.ast.children
 import com.pinterest.ktlint.core.ast.containsLineBreakInRange
 import com.pinterest.ktlint.core.ast.lineIndent
 import com.pinterest.ktlint.core.ast.lineNumber
 import com.pinterest.ktlint.core.ast.prevCodeLeaf
 import com.pinterest.ktlint.core.ast.prevLeaf
+import com.pinterest.ktlint.core.ast.upsertWhitespaceAfterMe
 import kotlin.properties.Delegates
 import org.ec4j.core.model.PropertyType
 import org.ec4j.core.model.PropertyType.PropertyValueParser
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.PsiElement
 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.tree.TokenSet
 import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
@@ -239,62 +244,87 @@ public class TrailingCommaOnDeclarationSiteRule :
             else -> if (trailingCommaNode != null) TrailingCommaState.REDUNDANT else TrailingCommaState.NOT_EXISTS
         }
         when (trailingCommaState) {
-            TrailingCommaState.EXISTS -> if (!isTrailingCommaAllowed) {
-                emit(
-                    trailingCommaNode!!.startOffset,
-                    "Unnecessary trailing comma before \"${inspectNode.text}\"",
-                    true
-                )
-                if (autoCorrect) {
-                    this.removeChild(trailingCommaNode)
-                }
-            }
-            TrailingCommaState.MISSING -> if (isTrailingCommaAllowed) {
-                val addNewLineBeforeArrowInWhenEntry = addNewLineBeforeArrowInWhen()
-                val prevNode = inspectNode.prevCodeLeaf()!!
-                if (addNewLineBeforeArrowInWhenEntry) {
-                    emit(
-                        prevNode.startOffset + prevNode.textLength,
-                        "Missing trailing comma and newline before \"${inspectNode.text}\"",
-                        true
-                    )
+            TrailingCommaState.EXISTS ->
+                if (isTrailingCommaAllowed) {
+                    inspectNode
+                        .treeParent
+                        .takeIf { it.elementType == WHEN_ENTRY }
+                        ?.findChildByType(ARROW)
+                        ?.prevLeaf()
+                        ?.let { lastNodeBeforeArrow ->
+                            if (lastNodeBeforeArrow.elementType != WHITE_SPACE || !lastNodeBeforeArrow.textContains('\n')) {
+                                emit(
+                                    trailingCommaNode!!.startOffset,
+                                    "Expected a newline between the trailing comma and  \"${inspectNode.text}\"",
+                                    true
+                                )
+                                if (autoCorrect) {
+                                    val parentIndent = "\n" + inspectNode.getWhenEntryIndent()
+                                    if (lastNodeBeforeArrow.elementType == WHITE_SPACE) {
+                                        (lastNodeBeforeArrow as LeafPsiElement).rawReplaceWithText(parentIndent)
+                                    } else {
+                                        (lastNodeBeforeArrow as LeafPsiElement).upsertWhitespaceAfterMe(parentIndent)
+                                    }
+                                }
+                            }
+                        }
                 } else {
                     emit(
-                        prevNode.startOffset + prevNode.textLength,
-                        "Missing trailing comma before \"${inspectNode.text}\"",
+                        trailingCommaNode!!.startOffset,
+                        "Unnecessary trailing comma before \"${inspectNode.text}\"",
                         true
                     )
+                    if (autoCorrect) {
+                        this.removeChild(trailingCommaNode)
+                    }
                 }
-                if (autoCorrect) {
+            TrailingCommaState.MISSING ->
+                if (isTrailingCommaAllowed) {
+                    val addNewLineBeforeArrowInWhenEntry = addNewLineBeforeArrowInWhen()
+                    val prevNode = inspectNode.prevCodeLeaf()!!
                     if (addNewLineBeforeArrowInWhenEntry) {
-                        val parentIndent = "\n" + prevNode.getWhenEntryIndent()
-                        val leafBeforeArrow = (psi as KtWhenEntry).arrow?.prevLeaf()
-                        if (leafBeforeArrow != null && leafBeforeArrow is PsiWhiteSpace) {
-                            val newLine = KtPsiFactory(prevNode.psi).createWhiteSpace(parentIndent)
-                            leafBeforeArrow.replace(newLine)
-                        } else {
-                            val newLine = KtPsiFactory(prevNode.psi).createWhiteSpace(parentIndent)
-                            prevNode.psi.parent.addAfter(newLine, prevNode.psi)
-                        }
+                        emit(
+                            prevNode.startOffset + prevNode.textLength,
+                            "Missing trailing comma and newline before \"${inspectNode.text}\"",
+                            true
+                        )
+                    } else {
+                        emit(
+                            prevNode.startOffset + prevNode.textLength,
+                            "Missing trailing comma before \"${inspectNode.text}\"",
+                            true
+                        )
                     }
+                    if (autoCorrect) {
+                        if (addNewLineBeforeArrowInWhenEntry) {
+                            val parentIndent = "\n" + prevNode.getWhenEntryIndent()
+                            val leafBeforeArrow = (psi as KtWhenEntry).arrow?.prevLeaf()
+                            if (leafBeforeArrow != null && leafBeforeArrow is PsiWhiteSpace) {
+                                val newLine = KtPsiFactory(prevNode.psi).createWhiteSpace(parentIndent)
+                                leafBeforeArrow.replace(newLine)
+                            } else {
+                                val newLine = KtPsiFactory(prevNode.psi).createWhiteSpace(parentIndent)
+                                prevNode.psi.parent.addAfter(newLine, prevNode.psi)
+                            }
+                        }
 
-                    if (inspectNode.treeParent.elementType == ElementType.ENUM_ENTRY) {
-                        with(KtPsiFactory(prevNode.psi)) {
-                            val parentIndent = (prevNode.psi.parent.prevLeaf() as? PsiWhiteSpace)?.text ?: "\n"
-                            val newline = createWhiteSpace(parentIndent)
-                            val enumEntry = inspectNode.treeParent.psi
-                            enumEntry.apply {
-                                add(newline)
-                                removeChild(inspectNode)
-                                parent.addAfter(createSemicolon(), this)
+                        if (inspectNode.treeParent.elementType == ElementType.ENUM_ENTRY) {
+                            with(KtPsiFactory(prevNode.psi)) {
+                                val parentIndent = (prevNode.psi.parent.prevLeaf() as? PsiWhiteSpace)?.text ?: "\n"
+                                val newline = createWhiteSpace(parentIndent)
+                                val enumEntry = inspectNode.treeParent.psi
+                                enumEntry.apply {
+                                    add(newline)
+                                    removeChild(inspectNode)
+                                    parent.addAfter(createSemicolon(), this)
+                                }
                             }
                         }
-                    }
 
-                    val comma = KtPsiFactory(prevNode.psi).createComma()
-                    prevNode.psi.parent.addAfter(comma, prevNode.psi)
+                        val comma = KtPsiFactory(prevNode.psi).createComma()
+                        prevNode.psi.parent.addAfter(comma, prevNode.psi)
+                    }
                 }
-            }
             TrailingCommaState.REDUNDANT -> {
                 emit(
                     trailingCommaNode!!.startOffset,

diff --git a/...st/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaOnDeclarationSiteRuleTest.kt b/...st/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaOnDeclarationSiteRuleTest.kt
@@ -307,6 +307,60 @@ class TrailingCommaOnDeclarationSiteRuleTest {
             ).isFormattedAs(formattedCode)
     }
 
+    @Test
+    fun `Given that the trailing comma is required on declaration site and that the trailing comma exists but is not followed by a newline then add the newline before the arrow`() {
+        val code =
+            """
+            fun foo(bar: Any): String = when(bar) {
+                1,
+                2,-> {
+                    "a"
+                }
+                3,
+                4,-> {
+                    "b"
+                }
+                5,
+                6 /* some comment */,-> {
+                    "c"
+                }
+                else -> {
+                    "d"
+                }
+            }
+            """.trimIndent()
+        val formattedCode =
+            """
+            fun foo(bar: Any): String = when(bar) {
+                1,
+                2,
+                -> {
+                    "a"
+                }
+                3,
+                4,
+                -> {
+                    "b"
+                }
+                5,
+                6 /* some comment */,
+                -> {
+                    "c"
+                }
+                else -> {
+                    "d"
+                }
+            }
+            """.trimIndent()
+        ruleAssertThat(code)
+            .withEditorConfigOverride(allowTrailingCommaProperty to true)
+            .hasLintViolations(
+                LintViolation(3, 6, "Expected a newline between the trailing comma and  \"->\""),
+                LintViolation(7, 6, "Expected a newline between the trailing comma and  \"->\""),
+                LintViolation(11, 25, "Expected a newline between the trailing comma and  \"->\"")
+            ).isFormattedAs(formattedCode)
+    }
+
     @Test
     fun `Given that the trailing comma is not allowed on declaration site then remove it from the destructuring declaration when present`() {
         val code =