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
…#427)

Summary:
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

Pull Request resolved: #427

Reviewed By: davidtorosyan

Differential Revision: D51115906

Pulled By: strulovich

fbshipit-source-id: 2579591911a613efdb495c34a9c70270979af8a9
  • Loading branch information
nreid260 authored and facebook-github-bot committed Nov 11, 2023
1 parent 102c89d commit fa78077
Show file tree
Hide file tree
Showing 7 changed files with 493 additions and 83 deletions.
21 changes: 12 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,14 @@ 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
return 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)!!) }
.let { if (shebang.isEmpty()) it else shebang + "\n" + it }
}

/** 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,16 @@ 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,

/**
* Automatically remove and insert trialing commas.
*
* Lists that cannot fit on one line will have trailing commas inserted. Lists that span
* multiple lines will have them removed. Manually inserted trailing commas cannot be used as a
* hint to force breaking lists to multiple lines.
*/
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 @@ -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,17 +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 {
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
return this.text.contains('\n')
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
126 changes: 126 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,126 @@
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.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

/** 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

/**
* Record elements which should have trailing commas inserted.
*
* This function determines which element type which may need trailing commas, as well as logic
* for when they shold be inserted.
*
* Example:
* ```
* fun foo(
* x: VeryLongName,
* y: MoreThanLineLimit // Record this list
* ) { }
*
* fun bar(x: ShortName, y: FitsOnLine) { } // Ignore this list
* ```
*/
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? {
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.getInnerExpressions(), element.trailingComma)
}
is KtWhenEntry -> ManagedList(element.conditions.toList(), element.trailingComma)
else -> null
}
}

/**
* Return the element ahead of the where a comma would be appropriate for a list item.
*
* Example:
* ```
* fun foo(
* x: VeryLongName,
* y: MoreThanLineLimit /# Comment #/ = { it } /# Comment #/
* ^^^^^^ // After this element
* ) { }
* ```
*/
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 fa78077

Please sign in to comment.