Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve semicolons in enums with members but no entries #434

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions core/src/main/java/com/facebook/ktfmt/format/EnumEntryList.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.facebook.ktfmt.format

import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtClassBody
import org.jetbrains.kotlin.psi.KtEnumEntry
import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespaceAndComments

/**
* PSI-like model of a list of enum entries.
*
* See https://youtrack.jetbrains.com/issue/KT-65157
*/
class EnumEntryList
private constructor(
val enumEntries: List<KtEnumEntry>,
val trailingComma: PsiElement?,
val terminatingSemicolon: PsiElement?,
) {
companion object {
fun extractParentList(enumEntry: KtEnumEntry): EnumEntryList {
return extractChildList(enumEntry.parent as KtClassBody)!!
}

fun extractChildList(classBody: KtClassBody): EnumEntryList? {
val clazz = classBody.parent
if (clazz !is KtClass || !clazz.isEnum()) return null

val enumEntries = classBody.children.filterIsInstance<KtEnumEntry>()

if (enumEntries.isEmpty()) {
var semicolon = classBody.firstChild
while (semicolon != null) {
if (semicolon.text == ";") break
semicolon = semicolon.nextSibling
}

return EnumEntryList(
enumEntries = enumEntries,
trailingComma = null,
terminatingSemicolon = semicolon,
)
}

var semicolon: PsiElement? = null
var comma: PsiElement? = null
val lastToken =
enumEntries
.last()
.lastChild
.getPrevSiblingIgnoringWhitespaceAndComments(withItself = true)!!
when (lastToken.text) {
"," -> {
comma = lastToken
}
";" -> {
semicolon = lastToken
val prevSibling = semicolon.getPrevSiblingIgnoringWhitespaceAndComments()
if (prevSibling?.text == ",") {
comma = prevSibling
}
}
}

return EnumEntryList(
enumEntries = enumEntries,
trailingComma = comma,
terminatingSemicolon = semicolon,
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import org.jetbrains.kotlin.psi.KtBreakExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtCatchClause
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtClassBody
import org.jetbrains.kotlin.psi.KtClassInitializer
import org.jetbrains.kotlin.psi.KtClassLiteralExpression
Expand Down Expand Up @@ -1941,12 +1942,13 @@ class KotlinInputAstVisitor(
override fun visitClassBody(body: KtClassBody) {
builder.sync(body)
emitBracedBlock(body) { children ->
val (enumEntries, nonEnumEntryMembers) = children.partition { it is KtEnumEntry }
val enumEntryList = EnumEntryList.extractChildList(body)
val members = children.filter { it !is KtEnumEntry }

if (enumEntries.isNotEmpty()) {
if (enumEntryList != null) {
builder.block(ZERO) {
builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
for (value in enumEntries) {
for (value in enumEntryList.enumEntries) {
visit(value)
if (builder.peekToken() == Optional.of(",")) {
builder.token(",")
Expand All @@ -1956,14 +1958,14 @@ class KotlinInputAstVisitor(
}
builder.guessToken(";")

if (nonEnumEntryMembers.isNotEmpty()) {
if (members.isNotEmpty()) {
builder.forcedBreak()
builder.blankLineWanted(OpsBuilder.BlankLineWanted.YES)
}
}

var prev: PsiElement? = null
for (curr in nonEnumEntryMembers) {
for (curr in members) {
val blankLineBetweenMembers =
when {
prev == null -> OpsBuilder.BlankLineWanted.PRESERVE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package com.facebook.ktfmt.format

import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtClassBody
import org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtEnumEntry
Expand Down Expand Up @@ -51,9 +53,16 @@ internal class RedundantSemicolonDetector {
if (parent is KtStringTemplateExpression || parent is KtStringTemplateEntry) {
return false
}
if (parent is KtEnumEntry &&
parent.siblings(forward = true, withItself = false).any { it is KtDeclaration }) {
return false

if (parent is KtEnumEntry) {
val classBody = parent.parent as KtClassBody
// Terminating semicolon with no other class members.
return classBody.children.last() == parent
}
if (parent is KtClassBody) {
val enumEntryList = EnumEntryList.extractChildList(parent) ?: return true
// Is not terminating semicolon or is terminating with no members.
return element != enumEntryList.terminatingSemicolon || parent.children.isEmpty()
}

val prevLeaf = element.prevLeaf(false)
Expand Down
62 changes: 62 additions & 0 deletions core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4649,6 +4649,68 @@ class FormatterTest {
|"""
.trimMargin())

@Test
fun `semicolon is removed from empty enum`() {
val code =
"""
|enum class SingleSemi {
| ;
|}
|
|enum class MultSemi {
| // a
| ;
| // b
| ;
| // c
| ;
|}
|"""
.trimMargin()
val expected =
"""
|enum class SingleSemi {}
|
|enum class MultSemi {
| // a
|
| // b
|
| // c
|
|}
|"""
.trimMargin()
assertThatFormatting(code).isEqualTo(expected)
}

@Test
fun `semicolon management in enum with no entries but other members`() {
val code =
"""
|enum class Empty {
| ;
|
| fun f() {}
| ;
| fun g() {}
|}
|"""
.trimMargin()
val expected =
"""
|enum class Empty {
| ;
|
| fun f() {}
|
| fun g() {}
|}
|"""
.trimMargin()
assertThatFormatting(code).isEqualTo(expected)
}

@Test
fun `handle varargs and spread operator`() =
assertFormatted(
Expand Down
Loading