From e9118ad45effe77b68ea9d28ead73ea236d7a994 Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Sat, 26 Dec 2020 10:13:37 +0100 Subject: [PATCH] Remove trailing comma's from other types (#709) --- .../com/pinterest/ktlint/core/ast/package.kt | 26 ++ .../experimental/NoTrailingCommaRule.kt | 152 +++++++- .../experimental/NoTrailingCommaRuleTest.kt | 338 +++++++++++++++++- 3 files changed, 499 insertions(+), 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 index 853b912efa..0cddfa464e 100644 --- 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/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRule.kt index 9f83acce2e..49602ec470 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRule.kt +++ b/ktlint-ruleset-experimental/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 } diff --git a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRuleTest.kt b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRuleTest.kt index 13b13df77c..b00705f3fc 100644 --- a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRuleTest.kt +++ b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/NoTrailingCommaRuleTest.kt @@ -16,6 +16,10 @@ class NoTrailingCommaRuleTest { "a", "b", // The comma before the comment should be removed without removing the comment itself ) + val list3 = listOf( + "a", + "b", /* The comma before the comment should be removed without removing the comment itself */ + ) """.trimIndent() val autoCorrectedCode = """ @@ -24,12 +28,17 @@ class NoTrailingCommaRuleTest { "a", "b" // The comma before the comment should be removed without removing the comment itself ) + val list3 = listOf( + "a", + "b" /* The comma before the comment should be removed without removing the comment itself */ + ) """.trimIndent() assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( listOf( - LintError(line = 1, col = 28, ruleId = "no-trailing-comma", detail = "Trailing command in argument list is redundant"), - LintError(line = 4, col = 8, ruleId = "no-trailing-comma", detail = "Trailing command in argument list is redundant"), + LintError(line = 1, col = 28, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 8, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), ) ) assertThat(NoTrailingCommaRule().format(code)) @@ -40,23 +49,336 @@ class NoTrailingCommaRuleTest { fun testFormatIsCorrectWithValueList() { val code = """ - data class Foo1( + data class Foo1(val bar: Int,) + data class Foo2( val bar: Int, // The comma before the comment should be removed without removing the comment itself ) - data class Foo2(val bar: Int,) + data class Foo3( + val bar: Int, /* The comma before the comment should be removed without removing the comment itself */ + ) """.trimIndent() val autoCorrectedCode = """ - data class Foo1( + data class Foo1(val bar: Int) + data class Foo2( val bar: Int // The comma before the comment should be removed without removing the comment itself ) - data class Foo2(val bar: Int) + data class Foo3( + val bar: Int /* The comma before the comment should be removed without removing the comment itself */ + ) + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 29, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 3, col = 16, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 6, col = 16, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithClassTypeParameters() { + val code = + """ + class Foo1 {} + class Foo2< + A, + B, // The comma before the comment should be removed without removing the comment itself + > {} + class Foo3< + A, + B, /* The comma before the comment should be removed without removing the comment itself */ + > {} + """.trimIndent() + val autoCorrectedCode = + """ + class Foo1 {} + class Foo2< + A, + B // The comma before the comment should be removed without removing the comment itself + > {} + class Foo3< + A, + B /* The comma before the comment should be removed without removing the comment itself */ + > {} + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 16, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 8, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithWhenEntry() { + val code = + """ + fun foo(bar: Int): String = when(bar) { + 1, 2, -> "a" + 3, 4, // The comma before the comment should be removed without removing the comment itself + -> "a" + 5, 6, /* The comma before the comment should be removed without removing the comment itself */ -> "a" + else -> "b" + } + """.trimIndent() + val autoCorrectedCode = + """ + fun foo(bar: Int): String = when(bar) { + 1, 2 -> "a" + 3, 4 // The comma before the comment should be removed without removing the comment itself + -> "a" + 5, 6 /* The comma before the comment should be removed without removing the comment itself */ -> "a" + else -> "b" + } + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 2, col = 9, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 3, col = 9, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 5, col = 9, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithDestructuringDeclaration() { + val code = + """ + fun foo() { + fun bar(): Pair = Pair(1, 2) + + val (x, y,) = bar() + val ( + x, + y, // The comma before the comment should be removed without removing the comment itself + ) = bar() + val ( + x, + y, /* The comma before the comment should be removed without removing the comment itself */ + ) = bar() + } + """.trimIndent() + val autoCorrectedCode = + """ + fun foo() { + fun bar(): Pair = Pair(1, 2) + + val (x, y) = bar() + val ( + x, + y // The comma before the comment should be removed without removing the comment itself + ) = bar() + val ( + x, + y /* The comma before the comment should be removed without removing the comment itself */ + ) = bar() + } + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 4, col = 14, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 7, col = 10, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 11, col = 10, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithFunctionParameters() { + val code = + """ + val foo1 = Pair(1, 2,) + val foo2 = Pair( + 1, + 2, // The comma before the comment should be removed without removing the comment itself + ) + val foo3 = Pair( + 1, + 2, /* The comma before the comment should be removed without removing the comment itself */ + ) + """.trimIndent() + val autoCorrectedCode = + """ + val foo1 = Pair(1, 2) + val foo2 = Pair( + 1, + 2 // The comma before the comment should be removed without removing the comment itself + ) + val foo3 = Pair( + 1, + 2 /* The comma before the comment should be removed without removing the comment itself */ + ) + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 21, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 8, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithFunctionType() { + val code = + """ + val fooBar1: (Int, Int,) -> Int = 42 + val fooBar2: ( + Int, + Int, // The comma before the comment should be removed without removing the comment itself + ) -> Int = 42 + val fooBar3: ( + Int, + Int, /* The comma before the comment should be removed without removing the comment itself */ + ) -> Int = 42 + """.trimIndent() + val autoCorrectedCode = + """ + val fooBar1: (Int, Int) -> Int = 42 + val fooBar2: ( + Int, + Int // The comma before the comment should be removed without removing the comment itself + ) -> Int = 42 + val fooBar3: ( + Int, + Int /* The comma before the comment should be removed without removing the comment itself */ + ) -> Int = 42 + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 23, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 8, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithFunctionLiteral() { + val code = + """ + val fooBar1: (Int, Int) -> Int = { foo, bar, -> foo * bar } + val fooBar2: (Int, Int) -> Int = { + foo, + bar, // The comma before the comment should be removed without removing the comment itself + -> foo * bar + } + val fooBar3: (Int, Int) -> Int = { + foo, + bar, /* The comma before the comment should be removed without removing the comment itself */ + -> foo * bar + } + """.trimIndent() + val autoCorrectedCode = + """ + val fooBar1: (Int, Int) -> Int = { foo, bar -> foo * bar } + val fooBar2: (Int, Int) -> Int = { + foo, + bar // The comma before the comment should be removed without removing the comment itself + -> foo * bar + } + val fooBar3: (Int, Int) -> Int = { + foo, + bar /* The comma before the comment should be removed without removing the comment itself */ + -> foo * bar + } + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 44, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 9, col = 8, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithTypeArgumentList() { + val code = + """ + val list1: List = emptyList() + val list2: List< + String, // The comma before the comment should be removed without removing the comment itself + > = emptyList() + val list3: List< + String, /* The comma before the comment should be removed without removing the comment itself */ + > = emptyList() + """.trimIndent() + val autoCorrectedCode = + """ + val list1: List = emptyList() + val list2: List< + String // The comma before the comment should be removed without removing the comment itself + > = emptyList() + val list3: List< + String /* The comma before the comment should be removed without removing the comment itself */ + > = emptyList() + """.trimIndent() + + assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( + listOf( + LintError(line = 1, col = 23, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 3, col = 11, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 6, col = 11, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + ) + ) + assertThat(NoTrailingCommaRule().format(code)) + .isEqualTo(autoCorrectedCode) + } + + @Test + fun testFormatIsCorrectWithArrayIndexExpression() { + val code = + """ + val foo = Array(2) { 42 } + val bar1 = foo[1,] + val bar2 = foo[ + 1, // The comma before the comment should be removed without removing the comment itself + ] + val bar3 = foo[ + 1, /* The comma before the comment should be removed without removing the comment itself */ + ] + """.trimIndent() + val autoCorrectedCode = + """ + val foo = Array(2) { 42 } + val bar1 = foo[1] + val bar2 = foo[ + 1 // The comma before the comment should be removed without removing the comment itself + ] + val bar3 = foo[ + 1 /* The comma before the comment should be removed without removing the comment itself */ + ] """.trimIndent() assertThat(NoTrailingCommaRule().lint(code)).isEqualTo( listOf( - LintError(line = 2, col = 16, ruleId = "no-trailing-comma", detail = "Trailing command in argument list is redundant"), - LintError(line = 4, col = 29, ruleId = "no-trailing-comma", detail = "Trailing command in argument list is redundant"), + LintError(line = 2, col = 17, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 4, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), + LintError(line = 7, col = 6, ruleId = "no-trailing-comma", detail = "Trailing comma is redundant"), ) ) assertThat(NoTrailingCommaRule().format(code))