Skip to content

Commit

Permalink
Automatically manage trailing commas when running with --google-style
Browse files Browse the repository at this point in the history
Commas are added/removed according to the following rules:
  - Lambda param lists => remove
  - Single element lists => remove
  - Lists that fit on one line => remove
  - All other lists (multiline lists) => add
  • Loading branch information
nreid260 committed Oct 7, 2023
1 parent 102c89d commit 5aee7c8
Show file tree
Hide file tree
Showing 7 changed files with 468 additions and 84 deletions.
23 changes: 14 additions & 9 deletions core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ package com.facebook.ktfmt.format
import com.facebook.ktfmt.debughelpers.printOps
import com.facebook.ktfmt.format.FormattingOptions.Style.DROPBOX
import com.facebook.ktfmt.format.FormattingOptions.Style.GOOGLE
import com.facebook.ktfmt.format.RedundantElementRemover.dropRedundantElements
import com.facebook.ktfmt.format.RedundantElementManager.dropRedundantElements
import com.facebook.ktfmt.format.RedundantElementManager.addRedundantElements
import com.facebook.ktfmt.format.WhitespaceTombstones.indexOfWhitespaceTombstone
import com.facebook.ktfmt.kdoc.Escaping
import com.facebook.ktfmt.kdoc.KDocCommentsHelper
Expand All @@ -32,7 +33,7 @@ import com.google.googlejavaformat.OpsBuilder
import com.google.googlejavaformat.java.FormatterException
import com.google.googlejavaformat.java.JavaOutput
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtil
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtilRt
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtilRt.convertLineSeparators
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiElementVisitor
Expand All @@ -44,7 +45,7 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset
object Formatter {

@JvmField
val GOOGLE_FORMAT = FormattingOptions(style = GOOGLE, blockIndent = 2, continuationIndent = 2)
val GOOGLE_FORMAT = FormattingOptions(style = GOOGLE, blockIndent = 2, continuationIndent = 2, manageTrailingCommas = true)

/** A format that attempts to reflect https://kotlinlang.org/docs/coding-conventions.html. */
@JvmField
Expand Down Expand Up @@ -86,12 +87,16 @@ object Formatter {
}
checkEscapeSequences(kotlinCode)

val lfCode = StringUtilRt.convertLineSeparators(kotlinCode)
val sortedImports = sortedAndDistinctImports(lfCode)
val noRedundantElements = dropRedundantElements(sortedImports, options)
val prettyCode =
prettyPrint(noRedundantElements, options, Newlines.guessLineSeparator(kotlinCode)!!)
return if (shebang.isNotEmpty()) shebang + "\n" + prettyCode else prettyCode
val formatted =
kotlinCode
.let { convertLineSeparators(it) }
.let { sortedAndDistinctImports(it) }
.let { dropRedundantElements(it, options) }
.let { prettyPrint(it, options, "\n") }
.let { addRedundantElements(it, options) }
.let { convertLineSeparators(it, Newlines.guessLineSeparator(kotlinCode)!!) }

return if (shebang.isNotEmpty()) { shebang + "\n" + formatted } else { formatted }
}

/** prettyPrint reflows 'code' using google-java-format's engine. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ data class FormattingOptions(
* Print the Ops generated by KotlinInputAstVisitor to help reason about formatting (i.e.,
* newline) decisions
*/
val debuggingPrintOpsAfterFormatting: Boolean = false
val debuggingPrintOpsAfterFormatting: Boolean = false,

/** Remove and insert trialing commas based on how the code would format without them. */
val manageTrailingCommas: Boolean = false,
) {

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ class KotlinInputAstVisitor(
visitEachCommaSeparated(
typeArgumentList.arguments,
typeArgumentList.trailingComma != null,
wrapInBlock = !isGoogleStyle,
prefix = "<",
postfix = ">",
)
Expand Down Expand Up @@ -1091,7 +1092,7 @@ class KotlinInputAstVisitor(
}

val indent = if (leadingBreak) ZERO else expressionBreakNegativeIndent
builder.block(indent, isEnabled = wrapInBlock) {
builder.block(indent, isEnabled = wrapInBlock) {
if (leadingBreak) {
builder.breakOp(breakType, "", ZERO)
}
Expand Down Expand Up @@ -2374,7 +2375,7 @@ class KotlinInputAstVisitor(
expression.trailingComma != null,
prefix = "[",
postfix = "]",
wrapInBlock = true)
wrapInBlock = !isGoogleStyle)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,39 @@ package com.facebook.ktfmt.format
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.kdoc.psi.impl.KDocImpl
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtImportList
import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.psi.KtReferenceExpression
import org.jetbrains.kotlin.psi.KtTreeVisitorVoid
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.startOffset

/** Removes elements that are not needed in the code, such as semicolons and unused imports. */
object RedundantElementRemover {
/**
* Adds and removes elements that are not strictly needed in the code, such as semicolons and
* unused imports.
*/
object RedundantElementManager {
/** Remove extra semicolons and unused imports, if enabled in the [options] */
fun dropRedundantElements(code: String, options: FormattingOptions): String {
val file = Parser.parse(code)
val redundantImportDetector = RedundantImportDetector(enabled = options.removeUnusedImports)
val redundantSemicolonDetector = RedundantSemicolonDetector()
val trailingCommaDetector = TrailingCommas.Detector()

file.accept(
object : KtTreeVisitorVoid() {
override fun visitElement(element: PsiElement) {
if (element is KDocImpl) {
redundantImportDetector.takeKdoc(element)
} else {
redundantSemicolonDetector.takeElement(element) { super.visitElement(element) }
return
}

redundantSemicolonDetector.takeElement(element)
if (options.manageTrailingCommas) {
trailingCommaDetector.takeElement(element)
}
super.visitElement(element)
}

override fun visitPackageDirective(directive: KtPackageDirective) {
Expand All @@ -63,16 +73,49 @@ object RedundantElementRemover {
val result = StringBuilder(code)
val elementsToRemove =
redundantSemicolonDetector.getRedundantSemicolonElements() +
redundantImportDetector.getRedundantImportElements()
redundantImportDetector.getRedundantImportElements() +
trailingCommaDetector.getTrailingCommaElements()

for (element in elementsToRemove.sortedByDescending(PsiElement::endOffset)) {
// Don't insert extra newlines when the semicolon is already a line terminator
val replacement = if (element.nextSibling.containsNewline()) "" else "\n"
val replacement =
if (element.text == ";" && !element.nextSibling.containsNewline()) {
"\n"
} else {
""
}
result.replace(element.startOffset, element.endOffset, replacement)
}

return result.toString()
}

fun addRedundantElements(code: String, options: FormattingOptions): String {
// TODO(nickreid): Generalize this for addtional sugested elements.
if (!options.manageTrailingCommas) {
return code
}

val file = Parser.parse(code)
val trailingCommaSuggestor = TrailingCommas.Suggestor()

file.accept(
object : KtTreeVisitorVoid() {
override fun visitKtElement(element: KtElement) {
trailingCommaSuggestor.takeElement(element)
super.visitElement(element)
}
})

val result = StringBuilder(code)
val suggestionElements = trailingCommaSuggestor.getTrailingCommaSuggestions()

for (element in suggestionElements.sortedByDescending(PsiElement::endOffset)) {
result.insert(element.endOffset, ',')
}

return result.toString()
}

private fun PsiElement?.containsNewline(): Boolean {
if (this !is PsiWhiteSpace) return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,13 @@ internal class RedundantSemicolonDetector {

fun getRedundantSemicolonElements(): List<PsiElement> = extraSemicolons

/** returns **true** if this element was an extra comma, **false** otherwise. */
fun takeElement(element: PsiElement, superBlock: () -> Unit) {
fun takeElement(element: PsiElement) {
if (isExtraSemicolon(element)) {
extraSemicolons += element
} else {
superBlock.invoke()
}
}

/** returns **true** if this element was an extra comma, **false** otherwise. */
private fun isExtraSemicolon(element: PsiElement): Boolean {
if (element.text != ";") {
return false
Expand Down
103 changes: 103 additions & 0 deletions core/src/main/java/com/facebook/ktfmt/format/TrailingCommas.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package com.facebook.ktfmt.format

import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtFunctionLiteral
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.KtParameterList
import org.jetbrains.kotlin.psi.KtTypeArgumentList
import org.jetbrains.kotlin.psi.KtTypeParameterList
import org.jetbrains.kotlin.psi.KtValueArgumentList
import org.jetbrains.kotlin.psi.KtWhenEntry
import org.jetbrains.kotlin.psi.psiUtil.allChildren

/** Detects trailing commas or elements that should have trailing commas. */
object TrailingCommas {

class Detector {
private val trailingCommas = mutableListOf<PsiElement>()

fun getTrailingCommaElements(): List<PsiElement> = trailingCommas

/** returns **true** if this element was a traling comma, **false** otherwise. */
fun takeElement(element: PsiElement) {
if (isTrailingComma(element)) {
trailingCommas += element
}
}

private fun isTrailingComma(element: PsiElement): Boolean {
if (element.text != ",") {
return false
}

return extractManagedList(element.parent)?.trailingComma == element
}
}

class Suggestor {
private val suggestionElements = mutableListOf<PsiElement>()

fun getTrailingCommaSuggestions(): List<PsiElement> = suggestionElements

fun takeElement(element: KtElement) {
if (!element.text.contains("\n")) {
return // Only suggest trailing commas where there is already a line break
}

when (element) {
is KtWhenEntry -> return
is KtParameterList -> {
if (element.parent is KtFunctionLiteral && element.parent.parent is KtLambdaExpression) {
return // Never add trailing commas to lambda param lists
}
}
}

val list = extractManagedList(element) ?: return
if (list.items.size <= 1) {
return // Never insert commas to single-element lists
}
if (list.trailingComma != null) {
return // Never insert a comma if there already is one somehow
}

suggestionElements.add(list.items.last().leftLeafIgnoringCommentsAndWhitespace())
}
}

private class ManagedList(val items: List<KtElement>, val trailingComma: PsiElement?)

private fun extractManagedList(element: PsiElement): ManagedList? {
// TODO(nickreid): What about:
// - function-type param lists
// - destructuring declarations
return when (element) {
is KtValueArgumentList -> ManagedList(element.arguments, element.trailingComma)
is KtParameterList -> ManagedList(element.parameters, element.trailingComma)
is KtTypeArgumentList -> ManagedList(element.arguments, element.trailingComma)
is KtTypeParameterList -> ManagedList(element.parameters, element.trailingComma)
is KtCollectionLiteralExpression -> {
ManagedList(element.innerExpressions, element.trailingComma)
}
is KtWhenEntry -> ManagedList(element.conditions.toList(), element.trailingComma)
else -> null
}
}

private fun PsiElement.leftLeafIgnoringCommentsAndWhitespace(): PsiElement {
var child = this.lastChild
while (child != null) {
if (child is PsiWhiteSpace || child is PsiComment) {
child = child.prevSibling
} else {
return child.leftLeafIgnoringCommentsAndWhitespace()
}
}
return this
}
}
Loading

0 comments on commit 5aee7c8

Please sign in to comment.