Skip to content

Commit fa78077

Browse files
nreid260facebook-github-bot
nreid260
authored and
facebook-github-bot
committed
Automatically manage trailing commas when running with --google-style (#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
1 parent 102c89d commit fa78077

7 files changed

Lines changed: 493 additions & 83 deletions

File tree

‎core/src/main/java/com/facebook/ktfmt/format/Formatter.kt‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ package com.facebook.ktfmt.format
1919
import com.facebook.ktfmt.debughelpers.printOps
2020
import com.facebook.ktfmt.format.FormattingOptions.Style.DROPBOX
2121
import com.facebook.ktfmt.format.FormattingOptions.Style.GOOGLE
22-
import com.facebook.ktfmt.format.RedundantElementRemover.dropRedundantElements
22+
import com.facebook.ktfmt.format.RedundantElementManager.dropRedundantElements
23+
import com.facebook.ktfmt.format.RedundantElementManager.addRedundantElements
2324
import com.facebook.ktfmt.format.WhitespaceTombstones.indexOfWhitespaceTombstone
2425
import com.facebook.ktfmt.kdoc.Escaping
2526
import com.facebook.ktfmt.kdoc.KDocCommentsHelper
@@ -32,7 +33,7 @@ import com.google.googlejavaformat.OpsBuilder
3233
import com.google.googlejavaformat.java.FormatterException
3334
import com.google.googlejavaformat.java.JavaOutput
3435
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtil
35-
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtilRt
36+
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtilRt.convertLineSeparators
3637
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
3738
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
3839
import org.jetbrains.kotlin.com.intellij.psi.PsiElementVisitor
@@ -44,7 +45,7 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset
4445
object Formatter {
4546

4647
@JvmField
47-
val GOOGLE_FORMAT = FormattingOptions(style = GOOGLE, blockIndent = 2, continuationIndent = 2)
48+
val GOOGLE_FORMAT = FormattingOptions(style = GOOGLE, blockIndent = 2, continuationIndent = 2, manageTrailingCommas = true)
4849

4950
/** A format that attempts to reflect https://kotlinlang.org/docs/coding-conventions.html. */
5051
@JvmField
@@ -86,12 +87,14 @@ object Formatter {
8687
}
8788
checkEscapeSequences(kotlinCode)
8889

89-
val lfCode = StringUtilRt.convertLineSeparators(kotlinCode)
90-
val sortedImports = sortedAndDistinctImports(lfCode)
91-
val noRedundantElements = dropRedundantElements(sortedImports, options)
92-
val prettyCode =
93-
prettyPrint(noRedundantElements, options, Newlines.guessLineSeparator(kotlinCode)!!)
94-
return if (shebang.isNotEmpty()) shebang + "\n" + prettyCode else prettyCode
90+
return kotlinCode
91+
.let { convertLineSeparators(it) }
92+
.let { sortedAndDistinctImports(it) }
93+
.let { dropRedundantElements(it, options) }
94+
.let { prettyPrint(it, options, "\n") }
95+
.let { addRedundantElements(it, options) }
96+
.let { convertLineSeparators(it, Newlines.guessLineSeparator(kotlinCode)!!) }
97+
.let { if (shebang.isEmpty()) it else shebang + "\n" + it }
9598
}
9699

97100
/** prettyPrint reflows 'code' using google-java-format's engine. */

‎core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,16 @@ data class FormattingOptions(
5353
* Print the Ops generated by KotlinInputAstVisitor to help reason about formatting (i.e.,
5454
* newline) decisions
5555
*/
56-
val debuggingPrintOpsAfterFormatting: Boolean = false
56+
val debuggingPrintOpsAfterFormatting: Boolean = false,
57+
58+
/**
59+
* Automatically remove and insert trialing commas.
60+
*
61+
* Lists that cannot fit on one line will have trailing commas inserted. Lists that span
62+
* multiple lines will have them removed. Manually inserted trailing commas cannot be used as a
63+
* hint to force breaking lists to multiple lines.
64+
*/
65+
val manageTrailingCommas: Boolean = false,
5766
) {
5867

5968
companion object {

‎core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ class KotlinInputAstVisitor(
258258
visitEachCommaSeparated(
259259
typeArgumentList.arguments,
260260
typeArgumentList.trailingComma != null,
261+
wrapInBlock = !isGoogleStyle,
261262
prefix = "<",
262263
postfix = ">",
263264
)
@@ -2374,7 +2375,7 @@ class KotlinInputAstVisitor(
23742375
expression.trailingComma != null,
23752376
prefix = "[",
23762377
postfix = "]",
2377-
wrapInBlock = true)
2378+
wrapInBlock = !isGoogleStyle)
23782379
}
23792380
}
23802381

core/src/main/java/com/facebook/ktfmt/format/RedundantElementRemover.kt renamed to core/src/main/java/com/facebook/ktfmt/format/RedundantElementManager.kt

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,29 +19,39 @@ package com.facebook.ktfmt.format
1919
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
2020
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
2121
import org.jetbrains.kotlin.kdoc.psi.impl.KDocImpl
22+
import org.jetbrains.kotlin.psi.KtElement
2223
import org.jetbrains.kotlin.psi.KtImportList
2324
import org.jetbrains.kotlin.psi.KtPackageDirective
2425
import org.jetbrains.kotlin.psi.KtReferenceExpression
2526
import org.jetbrains.kotlin.psi.KtTreeVisitorVoid
2627
import org.jetbrains.kotlin.psi.psiUtil.endOffset
2728
import org.jetbrains.kotlin.psi.psiUtil.startOffset
2829

29-
/** Removes elements that are not needed in the code, such as semicolons and unused imports. */
30-
object RedundantElementRemover {
30+
/**
31+
* Adds and removes elements that are not strictly needed in the code, such as semicolons and
32+
* unused imports.
33+
*/
34+
object RedundantElementManager {
3135
/** Remove extra semicolons and unused imports, if enabled in the [options] */
3236
fun dropRedundantElements(code: String, options: FormattingOptions): String {
3337
val file = Parser.parse(code)
3438
val redundantImportDetector = RedundantImportDetector(enabled = options.removeUnusedImports)
3539
val redundantSemicolonDetector = RedundantSemicolonDetector()
40+
val trailingCommaDetector = TrailingCommas.Detector()
3641

3742
file.accept(
3843
object : KtTreeVisitorVoid() {
3944
override fun visitElement(element: PsiElement) {
4045
if (element is KDocImpl) {
4146
redundantImportDetector.takeKdoc(element)
42-
} else {
43-
redundantSemicolonDetector.takeElement(element) { super.visitElement(element) }
47+
return
4448
}
49+
50+
redundantSemicolonDetector.takeElement(element)
51+
if (options.manageTrailingCommas) {
52+
trailingCommaDetector.takeElement(element)
53+
}
54+
super.visitElement(element)
4555
}
4656

4757
override fun visitPackageDirective(directive: KtPackageDirective) {
@@ -63,17 +73,49 @@ object RedundantElementRemover {
6373
val result = StringBuilder(code)
6474
val elementsToRemove =
6575
redundantSemicolonDetector.getRedundantSemicolonElements() +
66-
redundantImportDetector.getRedundantImportElements()
76+
redundantImportDetector.getRedundantImportElements() +
77+
trailingCommaDetector.getTrailingCommaElements()
6778

6879
for (element in elementsToRemove.sortedByDescending(PsiElement::endOffset)) {
6980
// Don't insert extra newlines when the semicolon is already a line terminator
70-
val replacement = if (element.nextSibling.containsNewline()) "" else "\n"
81+
val replacement =
82+
if (element.text == ";" && !element.nextSibling.containsNewline()) {
83+
"\n"
84+
} else {
85+
""
86+
}
7187
result.replace(element.startOffset, element.endOffset, replacement)
7288
}
7389

7490
return result.toString()
7591
}
7692

93+
fun addRedundantElements(code: String, options: FormattingOptions): String {
94+
if (!options.manageTrailingCommas) {
95+
return code
96+
}
97+
98+
val file = Parser.parse(code)
99+
val trailingCommaSuggestor = TrailingCommas.Suggestor()
100+
101+
file.accept(
102+
object : KtTreeVisitorVoid() {
103+
override fun visitKtElement(element: KtElement) {
104+
trailingCommaSuggestor.takeElement(element)
105+
super.visitElement(element)
106+
}
107+
})
108+
109+
val result = StringBuilder(code)
110+
val suggestionElements = trailingCommaSuggestor.getTrailingCommaSuggestions()
111+
112+
for (element in suggestionElements.sortedByDescending(PsiElement::endOffset)) {
113+
result.insert(element.endOffset, ',')
114+
}
115+
116+
return result.toString()
117+
}
118+
77119
private fun PsiElement?.containsNewline(): Boolean {
78120
if (this !is PsiWhiteSpace) return false
79121
return this.text.contains('\n')

‎core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,13 @@ internal class RedundantSemicolonDetector {
3535

3636
fun getRedundantSemicolonElements(): List<PsiElement> = extraSemicolons
3737

38-
/** returns **true** if this element was an extra comma, **false** otherwise. */
39-
fun takeElement(element: PsiElement, superBlock: () -> Unit) {
38+
fun takeElement(element: PsiElement) {
4039
if (isExtraSemicolon(element)) {
4140
extraSemicolons += element
42-
} else {
43-
superBlock.invoke()
4441
}
4542
}
4643

44+
/** returns **true** if this element was an extra comma, **false** otherwise. */
4745
private fun isExtraSemicolon(element: PsiElement): Boolean {
4846
if (element.text != ";") {
4947
return false

‎core/src/main/java/com/facebook/ktfmt/format/TrailingCommas.kt‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package com.facebook.ktfmt.format
2+
3+
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
4+
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
5+
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
6+
import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
7+
import org.jetbrains.kotlin.psi.KtElement
8+
import org.jetbrains.kotlin.psi.KtFunctionLiteral
9+
import org.jetbrains.kotlin.psi.KtLambdaExpression
10+
import org.jetbrains.kotlin.psi.KtParameterList
11+
import org.jetbrains.kotlin.psi.KtTypeArgumentList
12+
import org.jetbrains.kotlin.psi.KtTypeParameterList
13+
import org.jetbrains.kotlin.psi.KtValueArgumentList
14+
import org.jetbrains.kotlin.psi.KtWhenEntry
15+
16+
/** Detects trailing commas or elements that should have trailing commas. */
17+
object TrailingCommas {
18+
19+
class Detector {
20+
private val trailingCommas = mutableListOf<PsiElement>()
21+
22+
fun getTrailingCommaElements(): List<PsiElement> = trailingCommas
23+
24+
/** returns **true** if this element was a traling comma, **false** otherwise. */
25+
fun takeElement(element: PsiElement) {
26+
if (isTrailingComma(element)) {
27+
trailingCommas += element
28+
}
29+
}
30+
31+
private fun isTrailingComma(element: PsiElement): Boolean {
32+
if (element.text != ",") {
33+
return false
34+
}
35+
36+
return extractManagedList(element.parent)?.trailingComma == element
37+
}
38+
}
39+
40+
class Suggestor {
41+
private val suggestionElements = mutableListOf<PsiElement>()
42+
43+
fun getTrailingCommaSuggestions(): List<PsiElement> = suggestionElements
44+
45+
/**
46+
* Record elements which should have trailing commas inserted.
47+
*
48+
* This function determines which element type which may need trailing commas, as well as logic
49+
* for when they shold be inserted.
50+
*
51+
* Example:
52+
* ```
53+
* fun foo(
54+
* x: VeryLongName,
55+
* y: MoreThanLineLimit // Record this list
56+
* ) { }
57+
*
58+
* fun bar(x: ShortName, y: FitsOnLine) { } // Ignore this list
59+
* ```
60+
*/
61+
fun takeElement(element: KtElement) {
62+
if (!element.text.contains("\n")) {
63+
return // Only suggest trailing commas where there is already a line break
64+
}
65+
66+
when (element) {
67+
is KtWhenEntry -> return
68+
is KtParameterList -> {
69+
if (element.parent is KtFunctionLiteral && element.parent.parent is KtLambdaExpression) {
70+
return // Never add trailing commas to lambda param lists
71+
}
72+
}
73+
}
74+
75+
val list = extractManagedList(element) ?: return
76+
if (list.items.size <= 1) {
77+
return // Never insert commas to single-element lists
78+
}
79+
if (list.trailingComma != null) {
80+
return // Never insert a comma if there already is one somehow
81+
}
82+
83+
suggestionElements.add(list.items.last().leftLeafIgnoringCommentsAndWhitespace())
84+
}
85+
}
86+
87+
private class ManagedList(val items: List<KtElement>, val trailingComma: PsiElement?)
88+
89+
private fun extractManagedList(element: PsiElement): ManagedList? {
90+
return when (element) {
91+
is KtValueArgumentList -> ManagedList(element.arguments, element.trailingComma)
92+
is KtParameterList -> ManagedList(element.parameters, element.trailingComma)
93+
is KtTypeArgumentList -> ManagedList(element.arguments, element.trailingComma)
94+
is KtTypeParameterList -> ManagedList(element.parameters, element.trailingComma)
95+
is KtCollectionLiteralExpression -> {
96+
ManagedList(element.getInnerExpressions(), element.trailingComma)
97+
}
98+
is KtWhenEntry -> ManagedList(element.conditions.toList(), element.trailingComma)
99+
else -> null
100+
}
101+
}
102+
103+
/**
104+
* Return the element ahead of the where a comma would be appropriate for a list item.
105+
*
106+
* Example:
107+
* ```
108+
* fun foo(
109+
* x: VeryLongName,
110+
* y: MoreThanLineLimit /# Comment #/ = { it } /# Comment #/
111+
* ^^^^^^ // After this element
112+
* ) { }
113+
* ```
114+
*/
115+
private fun PsiElement.leftLeafIgnoringCommentsAndWhitespace(): PsiElement {
116+
var child = this.lastChild
117+
while (child != null) {
118+
if (child is PsiWhiteSpace || child is PsiComment) {
119+
child = child.prevSibling
120+
} else {
121+
return child.leftLeafIgnoringCommentsAndWhitespace()
122+
}
123+
}
124+
return this
125+
}
126+
}

0 commit comments

Comments
 (0)