From cec9b501428d5236c306ce2b50adc4cbe0338ffe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= <nbondanca@meta.com>
Date: Tue, 18 Jun 2024 06:01:21 -0700
Subject: [PATCH] Managing trailing commas is turned on by default now

Summary:
As requested on
https://github.com/facebook/ktfmt/issues/216
https://github.com/facebook/ktfmt/issues/442

Reviewed By: davidtorosyan

Differential Revision: D58585873

fbshipit-source-id: 61662ce4b21ad56b5554eb2275cd2e9341ccb1a4
---
 CHANGELOG.md                                  |  1 +
 .../com/facebook/ktfmt/format/Formatter.kt    |  3 -
 .../ktfmt/format/FormattingOptions.kt         |  2 +-
 .../facebook/ktfmt/format/FormatterTest.kt    | 18 ++++-
 .../format/GoogleStyleFormatterKtTest.kt      | 77 +++++--------------
 .../com/facebook/ktfmt/testutil/KtfmtTruth.kt |  6 +-
 6 files changed, 41 insertions(+), 66 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9a5ee8ea..b084b82c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 ## [Unreleased]
 
 ### Changed
+- All styles managing trailing commas now (https://github.com/facebook/ktfmt/issues/216, https://github.com/facebook/ktfmt/issues/442)
 
 
 ## [0.51]
diff --git a/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt b/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
index 951f1bc5..2238832a 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
@@ -47,7 +47,6 @@ object Formatter {
       FormattingOptions(
           blockIndent = 2,
           continuationIndent = 4,
-          manageTrailingCommas = false,
       )
 
   @JvmField
@@ -55,7 +54,6 @@ object Formatter {
       FormattingOptions(
           blockIndent = 2,
           continuationIndent = 2,
-          manageTrailingCommas = true,
       )
 
   /** A format that attempts to reflect https://kotlinlang.org/docs/coding-conventions.html. */
@@ -64,7 +62,6 @@ object Formatter {
       FormattingOptions(
           blockIndent = 4,
           continuationIndent = 4,
-          manageTrailingCommas = false,
       )
 
   private val MINIMUM_KOTLIN_VERSION = KotlinVersion(1, 4)
diff --git a/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt b/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt
index aa246d7b..e8db51d3 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt
@@ -51,7 +51,7 @@ data class FormattingOptions(
      * 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,
+    val manageTrailingCommas: Boolean = true,
 
     /** Whether ktfmt should remove imports that are not used. */
     val removeUnusedImports: Boolean = true,
diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
index f8546bfe..3c8b4a34 100644
--- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
+++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
@@ -19,8 +19,10 @@ package com.facebook.ktfmt.format
 import com.facebook.ktfmt.format.Formatter.META_FORMAT
 import com.facebook.ktfmt.testutil.assertFormatted
 import com.facebook.ktfmt.testutil.assertThatFormatting
+import com.facebook.ktfmt.testutil.defaultTestFormattingOptions
 import com.google.common.truth.Truth.assertThat
 import org.junit.Assert.fail
+import org.junit.BeforeClass
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
@@ -1472,7 +1474,7 @@ class FormatterTest {
             .trimMargin()
 
     assertThatFormatting(code)
-        .withOptions(META_FORMAT.copy(removeUnusedImports = false))
+        .withOptions(defaultTestFormattingOptions.copy(removeUnusedImports = false))
         .isEqualTo(code)
   }
 
@@ -4288,7 +4290,9 @@ class FormatterTest {
       |}
       |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(META_FORMAT.copy(maxWidth = 22)).isEqualTo(expected)
+    assertThatFormatting(code)
+        .withOptions(defaultTestFormattingOptions.copy(maxWidth = 22))
+        .isEqualTo(expected)
   }
 
   @Test
@@ -5373,7 +5377,9 @@ class FormatterTest {
       |class MyClass {}
       |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(META_FORMAT.copy(maxWidth = 33)).isEqualTo(expected)
+    assertThatFormatting(code)
+        .withOptions(defaultTestFormattingOptions.copy(maxWidth = 33))
+        .isEqualTo(expected)
   }
 
   @Test
@@ -7494,5 +7500,11 @@ class FormatterTest {
   companion object {
     /** Triple quotes, useful to use within triple-quoted strings. */
     private const val TQ = "\"\"\""
+
+    @JvmStatic
+    @BeforeClass
+    fun setUp(): Unit {
+      defaultTestFormattingOptions = META_FORMAT.copy(manageTrailingCommas = false)
+    }
   }
 }
diff --git a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt
index f5440bd4..2e52c52b 100644
--- a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt
+++ b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt
@@ -18,6 +18,8 @@ package com.facebook.ktfmt.format
 
 import com.facebook.ktfmt.testutil.assertFormatted
 import com.facebook.ktfmt.testutil.assertThatFormatting
+import com.facebook.ktfmt.testutil.defaultTestFormattingOptions
+import org.junit.BeforeClass
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
@@ -100,7 +102,7 @@ class GoogleStyleFormatterKtTest {
         |"""
             .trimMargin()
 
-    assertThatFormatting(code).withOptions(Formatter.GOOGLE_FORMAT).isEqualTo(expected)
+    assertThatFormatting(code).isEqualTo(expected)
     // Don't add more tests here
   }
 
@@ -144,7 +146,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -196,7 +197,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -245,7 +245,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -270,7 +269,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -292,7 +290,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -310,7 +307,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -335,7 +331,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -361,7 +356,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -373,7 +367,6 @@ class GoogleStyleFormatterKtTest {
       |  .format(expression)
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -401,7 +394,6 @@ class GoogleStyleFormatterKtTest {
       |    .secondLink()
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -420,7 +412,6 @@ class GoogleStyleFormatterKtTest {
       |  DUMMY
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -480,7 +471,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -498,7 +488,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `line breaks inside when expressions and conditions`() =
@@ -529,7 +519,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
       )
 
   @Test
@@ -545,7 +534,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
       )
 
   @Test
@@ -565,7 +553,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
       )
 
   @Test
@@ -581,7 +568,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
       )
 
   @Test
@@ -599,7 +585,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `Arguments are blocks`() =
@@ -628,7 +614,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -649,7 +634,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -668,7 +652,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `named arguments indent their value expression`() =
@@ -685,7 +669,7 @@ class GoogleStyleFormatterKtTest {
       |  )
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `breaking long binary operations`() =
@@ -709,7 +693,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -761,7 +744,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -793,7 +775,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   // TODO: there's a bug here - the last case shouldn't break after 'foo'.
@@ -823,7 +804,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -849,7 +829,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -866,7 +845,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -910,7 +888,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -947,7 +924,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(Formatter.GOOGLE_FORMAT).isEqualTo(expected)
+    assertThatFormatting(code).isEqualTo(expected)
   }
 
   @Test
@@ -1034,7 +1011,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(Formatter.GOOGLE_FORMAT).isEqualTo(expected)
+    assertThatFormatting(code).isEqualTo(expected)
   }
 
   @Test
@@ -1072,7 +1049,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(Formatter.GOOGLE_FORMAT).isEqualTo(expected)
+    assertThatFormatting(code).isEqualTo(expected)
   }
 
   @Test
@@ -1105,7 +1082,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
             .trimMargin(),
-        formattingOptions = Formatter.GOOGLE_FORMAT,
         deduceMaxWidth = false)
   }
 
@@ -1138,7 +1114,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
             .trimMargin(),
-        formattingOptions = Formatter.GOOGLE_FORMAT,
         deduceMaxWidth = false)
   }
 
@@ -1195,7 +1170,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1219,7 +1193,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `chained calls that don't fit in one line`() =
@@ -1238,7 +1212,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1289,7 +1262,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1307,7 +1279,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1336,7 +1307,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1353,7 +1323,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1384,7 +1353,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1402,7 +1370,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1431,7 +1398,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1448,7 +1414,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1471,7 +1436,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1490,7 +1454,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1505,7 +1468,6 @@ class GoogleStyleFormatterKtTest {
       |)
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1522,7 +1484,6 @@ class GoogleStyleFormatterKtTest {
       |  ) -> Unit
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1545,7 +1506,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1566,7 +1526,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `array-literal in annotation`() =
@@ -1609,7 +1569,6 @@ class GoogleStyleFormatterKtTest {
       |class Host
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1673,7 +1632,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1711,7 +1669,6 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
   @Test
@@ -1731,7 +1688,7 @@ class GoogleStyleFormatterKtTest {
       |}
       |"""
               .trimMargin(),
-          formattingOptions = Formatter.GOOGLE_FORMAT)
+      )
 
   @Test
   fun `trailing commas in enums`() {
@@ -1829,11 +1786,17 @@ class GoogleStyleFormatterKtTest {
         |}
         |"""
             .trimMargin()
-    assertThatFormatting(code).withOptions(Formatter.GOOGLE_FORMAT).isEqualTo(expected)
+    assertThatFormatting(code).isEqualTo(expected)
   }
 
   companion object {
     /** Triple quotes, useful to use within triple-quoted strings. */
     private const val TQ = "\"\"\""
+
+    @JvmStatic
+    @BeforeClass
+    fun setUp(): Unit {
+      defaultTestFormattingOptions = Formatter.GOOGLE_FORMAT
+    }
   }
 }
diff --git a/core/src/test/java/com/facebook/ktfmt/testutil/KtfmtTruth.kt b/core/src/test/java/com/facebook/ktfmt/testutil/KtfmtTruth.kt
index ba80d89c..f8435161 100644
--- a/core/src/test/java/com/facebook/ktfmt/testutil/KtfmtTruth.kt
+++ b/core/src/test/java/com/facebook/ktfmt/testutil/KtfmtTruth.kt
@@ -26,6 +26,8 @@ import com.google.common.truth.Truth
 import org.intellij.lang.annotations.Language
 import org.junit.Assert
 
+var defaultTestFormattingOptions: FormattingOptions = Formatter.META_FORMAT
+
 /**
  * Verifies the given code passes through formatting, and stays the same at the end
  *
@@ -44,7 +46,7 @@ import org.junit.Assert
  */
 fun assertFormatted(
     @Language("kts") code: String,
-    formattingOptions: FormattingOptions = Formatter.META_FORMAT,
+    formattingOptions: FormattingOptions = defaultTestFormattingOptions,
     deduceMaxWidth: Boolean = false,
 ) {
   val first = code.lines().first()
@@ -81,7 +83,7 @@ fun assertThatFormatting(@Language("kts") code: String): FormattedCodeSubject {
 
 class FormattedCodeSubject(metadata: FailureMetadata, private val code: String) :
     Subject(metadata, code) {
-  private var options: FormattingOptions = Formatter.META_FORMAT
+  private var options: FormattingOptions = defaultTestFormattingOptions
   private var allowTrailingWhitespace = false
 
   fun withOptions(options: FormattingOptions): FormattedCodeSubject {