diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRule.kt index a417d3ca80..615ab62cd6 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRule.kt @@ -3,9 +3,12 @@ package com.pinterest.ktlint.ruleset.experimental.trailingcomma import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.api.UsesEditorConfigProperties import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY +import com.pinterest.ktlint.core.ast.ElementType.SEMICOLON import com.pinterest.ktlint.core.ast.children import com.pinterest.ktlint.core.ast.containsLineBreakInRange import com.pinterest.ktlint.core.ast.isRoot +import com.pinterest.ktlint.core.ast.lineNumber import com.pinterest.ktlint.core.ast.prevCodeLeaf import com.pinterest.ktlint.core.ast.prevLeaf import kotlin.properties.Delegates @@ -25,6 +28,7 @@ import org.jetbrains.kotlin.psi.KtPsiFactory import org.jetbrains.kotlin.psi.KtValueArgumentList import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.KtWhenExpression +import org.jetbrains.kotlin.psi.psiUtil.allChildren import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType import org.jetbrains.kotlin.psi.psiUtil.nextLeaf @@ -202,23 +206,46 @@ public class TrailingCommaRule : require(psi is KtClass) if (psi.isEnum()) { - val classBody = node.children().singleOrNull { it.psi is KtClassBody } - ?: return // Nothing to do for empty enum class + val classBody = node + .children() + .singleOrNull { it.psi is KtClassBody } + ?: return - val lastRelevantChild = classBody.psi.children.toList().asReversed() - .first() + if (classBody.lastTwoEnumEntriesAreOnSameLine()) return // Do nothing when last two entries are on same line + val inspectNode = classBody.determineNodeToInspectForEnum() ?: return + node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect) + } + } - val fullAST = psi - val fullPsi = classBody.psi + /** + * Determines what [ASTNode] to inspect when dealing with Enums. + * + * If an Enum is terminated by a semicolon, that will be inspected, otherwise the element following the last + * enum entry (which should be the `}` terminating the enum class) + */ + private fun ASTNode.determineNodeToInspectForEnum(): ASTNode? { + val lastEnumEntry = psi + .children + .last { it is KtEnumEntry } - if (lastRelevantChild !is KtEnumEntry) { - // Aborting processing since Enum contains more than just constants. - return - } + val semicolon = lastEnumEntry + .node + .children() + .singleOrNull { it.elementType == SEMICOLON } - val inspectNode = classBody.lastChildNode - node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect) - } + // Inspect the semicolon if it is semicolon terminated, otherwise next non-ignorable node in the tree + return semicolon + ?: lastEnumEntry.nextLeaf { !it.node.isIgnorable() }!!.node + } + + private fun ASTNode.lastTwoEnumEntriesAreOnSameLine(): Boolean { + val enumEntries = psi + .children + .filterIsInstance() + .reversed() // We are interested in the last two ones. + + if (enumEntries.count() < 2) return false + return enumEntries[0].node.lineNumber() == enumEntries[1].node.lineNumber() } private fun ASTNode.reportAndCorrectTrailingCommaNodeBefore( @@ -264,6 +291,7 @@ public class TrailingCommaRule : true ) } + if (autoCorrect) { if (addNewLineBeforeArrowInWhenEntry) { val parentIndent = (prevNode.psi.parent.prevLeaf() as? PsiWhiteSpace)?.text ?: "\n" @@ -276,6 +304,20 @@ public class TrailingCommaRule : prevNode.psi.parent.addAfter(newLine, prevNode.psi) } } + + if (inspectNode.treeParent.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) } @@ -302,6 +344,13 @@ public class TrailingCommaRule : false } + private fun ASTNode?.correctTrailingCommaInSemicolonTerminatedEnum() = + when { + this == null -> false + psi is KtEnumEntry -> psi.allChildren.last?.node?.elementType == SEMICOLON + else -> false + } + private fun ASTNode?.findPreviousTrailingCommaNodeOrNull(): ASTNode? { var node = this while (node?.isIgnorable() == true) { @@ -365,11 +414,6 @@ public class TrailingCommaRule : ElementType.VALUE_ARGUMENT_LIST ) - private val ENUM_IGNORED_TYPES = TokenSet.create( - ElementType.WHITE_SPACE, - ElementType.RBRACE - ) - internal const val ALLOW_TRAILING_COMMA_NAME = "ij_kotlin_allow_trailing_comma" private const val ALLOW_TRAILING_COMMA_DESCRIPTION = "Defines whether a trailing comma (or no trailing comma)" + "should be enforced on the defining side," + diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRuleTest.kt index 575129252b..6238d549e5 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/TrailingCommaRuleTest.kt @@ -5,6 +5,7 @@ import com.pinterest.ktlint.ruleset.experimental.trailingcomma.TrailingCommaRule import com.pinterest.ktlint.ruleset.experimental.trailingcomma.TrailingCommaRule.Companion.allowTrailingCommaProperty import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThat import com.pinterest.ktlint.test.LintViolation +import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test class TrailingCommaRuleTest { @@ -1023,60 +1024,65 @@ class TrailingCommaRuleTest { } @Test - fun `Given that a trailing comma is required then add trailing comma after last enum member`() { + fun `Given that a trailing comma is is not allowed then remove comma after last enum member`() { val code = """ enum class Shape { SQUARE, - TRIANGLE + TRIANGLE, } """.trimIndent() val formattedCode = """ enum class Shape { SQUARE, - TRIANGLE, + TRIANGLE } """.trimIndent() trailingCommaRuleAssertThat(code) - .withEditorConfigOverride(allowTrailingCommaProperty to true) - .hasLintViolation(3, 13, "Missing trailing comma before \"}\"") + .withEditorConfigOverride(allowTrailingCommaProperty to false) + .hasLintViolation(3, 13, "Unnecessary trailing comma before \"}\"") .isFormattedAs(formattedCode) } @Test - fun `Given that a trailing comma is is not allowed then remove comma after last enum member`() { + fun `Given that a trailing comma is not allowed then it is removed for enums terminated with semicolon`() { val code = """ enum class Shape { SQUARE, TRIANGLE, + ; + + fun print() = name() } """.trimIndent() + + // TODO: Should the semicolon be moved up automatically? val formattedCode = """ enum class Shape { SQUARE, TRIANGLE + ; + + fun print() = name() } """.trimIndent() trailingCommaRuleAssertThat(code) .withEditorConfigOverride(allowTrailingCommaProperty to false) - .hasLintViolation(3, 13, "Unnecessary trailing comma before \"}\"") + .hasLintViolation(3, 13, "Unnecessary trailing comma before \";\"") .isFormattedAs(formattedCode) } @Test - fun `Given that a trailing comma is not allowed then nothing happens for enums terminated with semicolon`() { + fun `Given that a trailing comma is not allowed then it is not removed for enums where last two entries are on same line`() { val code = """ enum class Shape { - SQUARE, - TRIANGLE; - - fun print() = name() + SQUARE, TRIANGLE, } """.trimIndent() @@ -1085,93 +1091,191 @@ class TrailingCommaRuleTest { .hasNoLintViolations() } - @Test - fun `Given that a trailing comma is allowed then nothing happens for enums terminated with semicolon`() { - val code = - """ - enum class Shape { - SQUARE, - TRIANGLE; + @Nested + inner class MissingRequiredTrailingComma { - fun print() = name() - } - """.trimIndent() + @Test + fun `Given that last two enumeration entries are on same line, do not add a trailing comma`() { + val code = + """ + enum class Shape { + SQUARE, TRIANGLE + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasNoLintViolations() + } + + @Test + fun `Given an enum is terminated by a semicolon and EOL comment without a trailing comma, then it is added `() { + val code = + """ + enum class Shape { + SQUARE, + TRIANGLE; // EOL Comment should be kept + } + """.trimIndent() + + val formattedCode = + """ + enum class Shape { + SQUARE, + TRIANGLE, // EOL Comment should be kept + ; + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(3, 13, "Missing trailing comma before \";\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given an enum is terminated by a semicolon and block comment, then it is added `() { + val code = + """ + enum class Shape { + SQUARE, + TRIANGLE; /* block comment should be kept */ + } + """.trimIndent() + + val formattedCode = + """ + enum class Shape { + SQUARE, + TRIANGLE, /* block comment should be kept */ + ; + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(3, 13, "Missing trailing comma before \";\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given an enum terminated by semicolon without a trailing comma then it is added`() { + val code = + """ + enum class Shape { + SQUARE, + TRIANGLE; + } + """.trimIndent() + + val formattedCode = + """ + enum class Shape { + SQUARE, + TRIANGLE, + ; + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(3, 13, "Missing trailing comma before \";\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given an enum without trailing-comma with other declarations following the enum entries then it is added`() { + val code = + """ + enum class Shape { + SQUARE, + TRIANGLE; + + fun print() = name() + } + """.trimIndent() - trailingCommaRuleAssertThat(code) - .withEditorConfigOverride(allowTrailingCommaProperty to true) - .hasNoLintViolations() - } + val formattedCode = + """ + enum class Shape { + SQUARE, + TRIANGLE, + ; - @Test - fun `Given that a trailing comma is allowed then it is added for complicated enums`() { - val code = - """ - interface Printable { - fun print(): String - } + fun print() = name() + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(3, 13, "Missing trailing comma before \";\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given that a trailing comma is required then it is added for complicated enums`() { + val code = + """ + interface Printable { + fun print(): String + } - enum class Shape : Printable { - /* - * Block comment is OK! - */ - Square { - override fun print() = "■" // EOL Comment should be OK - }, + enum class Shape : Printable { + Square { + override fun print() = "■" + }, - Triangle { - override fun print() = "▲" + Triangle { + override fun print() = "▲" + } } - } - """.trimIndent() - - val formattedCode = - """ - interface Printable { - fun print(): String - } + """.trimIndent() - enum class Shape : Printable { - /* - * Block comment is OK! - */ - Square { - override fun print() = "■" // EOL Comment should be OK - }, - - Triangle { - override fun print() = "▲" - }, - } - """.trimIndent() + val formattedCode = + """ + interface Printable { + fun print(): String + } - trailingCommaRuleAssertThat(code) - .withEditorConfigOverride(allowTrailingCommaProperty to true) - .hasLintViolation(15, 6, "Missing trailing comma before \"}\"") - .isFormattedAs(formattedCode) - } + enum class Shape : Printable { + Square { + override fun print() = "■" + }, - @Test - fun `Given that a trailing comma is allowed then it is added if enum is already terminated with semi-colon`() { - val code = - """ - enum class Shape { - SQUARE, - TRIANGLE; - } - """.trimIndent() + Triangle { + override fun print() = "▲" + }, + } + """.trimIndent() + + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(12, 6, "Missing trailing comma before \"}\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given that a trailing comma is required then add trailing comma after last enum member`() { + val code = + """ + enum class Shape { + SQUARE, + TRIANGLE + } + """.trimIndent() - val formattedCode = - """ - enum class Shape { - SQUARE, - TRIANGLE, - ; - } - """.trimIndent() + val formattedCode = + """ + enum class Shape { + SQUARE, + TRIANGLE, + } + """.trimIndent() - trailingCommaRuleAssertThat(code) - .withEditorConfigOverride(allowTrailingCommaProperty to true) - .hasLintViolation(3, 14, "Missing trailing comma before \"}\"") - .isFormattedAs(formattedCode) + trailingCommaRuleAssertThat(code) + .withEditorConfigOverride(allowTrailingCommaProperty to true) + .hasLintViolation(3, 13, "Missing trailing comma before \"}\"") + .isFormattedAs(formattedCode) + } } }