Remove trailing comma's from other types (pinterest#709)

paul-dingemans · Aug 8, 2021 · e9118ad · e9118ad
1 parent 8dad976
commit e9118ad
Show file tree

Hide file tree

Showing 3 changed files with 499 additions and 17 deletions.
diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/ast/package.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/ast/package.kt
@@ -260,3 +260,29 @@ fun ASTNode.lineIndent(): String {
     }
     return ""
 }
+
+/**
+ *  Print content of a node and the element type of the node, its parent and its direct children. Utility is meant to
+ *  be used during development only. Please do not remove.
+ */
+@Suppress("unused")
+fun ASTNode.logStructure(): ASTNode =
+    also {
+        println("Processing ${text.replace("\n", "\\n")} : Type $elementType with parent ${treeParent?.elementType} ")
+        children()
+            .toList()
+            .map {
+                println("  ${it.text.replace("\n", "\\n")} : Type ${it.elementType}")
+            }
+    }
+
+/**
+ *  Assert the element type of the node
+ */
+@Suppress("unused")
+fun ASTNode.assertElementType(vararg elementTypes: IElementType): ASTNode =
+    also {
+        assert(elementTypes.contains(this.elementType)) {
+            "Expected element of types $elementType but received ${this.elementType}"
+        }
+    }
diff --git a/...rimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRule.kt b/...rimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRule.kt
@@ -2,7 +2,9 @@ package com.pinterest.ktlint.ruleset.experimental
 
 import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType
+import com.pinterest.ktlint.core.ast.assertElementType
 import com.pinterest.ktlint.core.ast.children
+import com.pinterest.ktlint.core.ast.prevLeaf
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.psi.psiUtil.endOffset
 
@@ -13,19 +15,151 @@ class NoTrailingCommaRule : Rule("no-trailing-comma") {
         autoCorrect: Boolean,
         emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
     ) {
-        if (node.elementType == ElementType.VALUE_ARGUMENT_LIST || node.elementType == ElementType.VALUE_PARAMETER_LIST) {
-            val lastNode = node
+        when (node.elementType) {
+            ElementType.COLLECTION_LITERAL_EXPRESSION ->
+                // TODO: According to https://github.com/JetBrains/intellij-kotlin/blob/master/formatter/src/org/jetbrains/kotlin/idea/formatter/trailingComma/util.kt
+                //  it is possible to have a trailing comma with this type of element. Need an example before it can
+                //  be implemented
+                Unit
+            ElementType.DESTRUCTURING_DECLARATION -> visitDestructuringDeclaration(node, emit, autoCorrect)
+            ElementType.FUNCTION_LITERAL -> visitFunctionLiteral(node, emit, autoCorrect)
+            ElementType.FUNCTION_TYPE -> visitFunctionType(node, emit, autoCorrect)
+            ElementType.INDICES -> visitIndices(node, emit, autoCorrect)
+            ElementType.TYPE_ARGUMENT_LIST -> visitTypeList(node, emit, autoCorrect)
+            ElementType.TYPE_PARAMETER_LIST -> visitTypeList(node, emit, autoCorrect)
+            ElementType.VALUE_ARGUMENT_LIST -> visitValueList(node, emit, autoCorrect)
+            ElementType.VALUE_PARAMETER_LIST -> visitValueList(node, emit, autoCorrect)
+            ElementType.WHEN_ENTRY -> visitWhenEntry(node, emit, autoCorrect)
+            else -> Unit
+        }
+    }
+
+    private fun visitDestructuringDeclaration(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.DESTRUCTURING_DECLARATION)
+            .children()
+            .last { it.elementType == ElementType.RPAR }
+            .prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun visitFunctionLiteral(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.FUNCTION_LITERAL)
+            .children()
+            .lastOrNull() { it.elementType == ElementType.ARROW }
+            ?.prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun visitFunctionType(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.FUNCTION_TYPE)
+            .firstChildNode
+            .assertElementType(ElementType.VALUE_PARAMETER_LIST)
+            .children()
+            .last()
+            .prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun visitIndices(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.INDICES)
+            .children()
+            .last { it.elementType == ElementType.RBRACKET }
+            .prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun visitValueList(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        if (node.treeParent.elementType != ElementType.FUNCTION_LITERAL &&
+            node.treeParent.elementType != ElementType.FUNCTION_TYPE
+        ) {
+            val inspectNode = node
+                .assertElementType(ElementType.VALUE_ARGUMENT_LIST, ElementType.VALUE_PARAMETER_LIST)
                 .children()
-                .filter { it.elementType != ElementType.WHITE_SPACE }
-                .filter { it.elementType != ElementType.EOL_COMMENT }
-                .filter { it.elementType != ElementType.RPAR }
-                .last()
-            if (lastNode.elementType == ElementType.COMMA) {
-                emit(lastNode.psi.endOffset - 1, "Trailing command in argument list is redundant", true)
+                .last { it.elementType == ElementType.RPAR }
+                .prevLeaf()
+            node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+        }
+    }
+
+    private fun visitTypeList(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.TYPE_ARGUMENT_LIST, ElementType.TYPE_PARAMETER_LIST)
+            .children()
+            .first { it.elementType == ElementType.GT }
+            .prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun visitWhenEntry(
+        node: ASTNode,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        val inspectNode = node
+            .assertElementType(ElementType.WHEN_ENTRY)
+            .children()
+            .first { it.elementType == ElementType.ARROW }
+            .prevLeaf()
+        node.reportAndOrCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
+    }
+
+    private fun ASTNode.reportAndOrCorrectTrailingCommaNodeBefore(
+        inspectNode: ASTNode?,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+        autoCorrect: Boolean
+    ) {
+        inspectNode
+            .findPreviousTrailingCommaNodeOrNull()
+            ?.let { trailingCommaNode ->
+                emit(trailingCommaNode.psi.endOffset - 1, "Trailing comma is redundant", true)
                 if (autoCorrect) {
-                    node.removeChild(lastNode)
+                    this.removeChild(trailingCommaNode)
                 }
             }
+    }
+
+    private fun ASTNode?.findPreviousTrailingCommaNodeOrNull(): ASTNode? {
+        var node = this
+        while (node != null && node.isIgnorable()) {
+            node = node.prevLeaf()
+        }
+        return if (node != null && node.elementType == ElementType.COMMA) {
+            node
+        } else {
+            null
         }
     }
+
+    private fun ASTNode.isIgnorable(): Boolean =
+        elementType == ElementType.WHITE_SPACE ||
+            elementType == ElementType.EOL_COMMENT ||
+            elementType == ElementType.BLOCK_COMMENT
 }