From dc8757fc3773d4ac69f4af4644d1197b94c74aba Mon Sep 17 00:00:00 2001
From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com>
Date: Mon, 17 Jun 2024 05:46:46 -0700
Subject: [PATCH 1/2] Test non-idempotent optional-braces trailing comma

---
 .../trailing-commas/trailingCommasAlways.stat | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
index d09926e9b6..9761efca0e 100644
--- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
+++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
@@ -442,3 +442,38 @@ val x = (
 val x = (
     x => x + 1,
 )
+<<< #4058 1
+maxColumn = 16
+runner.dialect = scala3
+===
+f(
+  if true then 0
+  else
+    1234567890
+  ,
+)
+>>>
+Idempotency violated
+=> Diff (- obtained, + expected)
+   else
+-    1234567890
+-  ,
++    1234567890,
+ )
+<<< #4058 2
+maxColumn = 16
+runner.dialect = scala3
+===
+f(
+  if true then 0
+  else
+    1234567890,
+)
+>>>
+Idempotency violated
+=> Diff (- obtained, + expected)
+   else
+-    1234567890,
++    1234567890
++  ,
+ )

From c5f44f254335accca4acf7bb55e12b570b1ba1d6 Mon Sep 17 00:00:00 2001
From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com>
Date: Mon, 17 Jun 2024 06:00:53 -0700
Subject: [PATCH 2/2] RewriteTrailingCommas: handle optional braces case

Don't check equality of owners of comma and closing delimiter, it's no
longer guaranteed. Also, inconsistent with the check in FormatWriter so
that could lead to further non-idempotency.
---
 .../rewrite/RewriteTrailingCommas.scala       | 17 ++++++-------
 .../trailing-commas/trailingCommasAlways.stat | 24 ++++++++-----------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RewriteTrailingCommas.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RewriteTrailingCommas.scala
index 0cc37c1d38..4b03948e9c 100644
--- a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RewriteTrailingCommas.scala
+++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RewriteTrailingCommas.scala
@@ -55,25 +55,26 @@ private class RewriteTrailingCommas(implicit val ftoks: FormatTokens)
   private[rewrite] def shouldRemove(
       ft: FormatToken,
   )(implicit session: Session): Boolean = ft.right.is[Token.Comma] && {
-    val rightOwner = ft.meta.rightOwner
     val nft = ftoks.nextNonCommentAfter(ft)
+    def delimOwner = nft.meta.rightOwner
 
-    // comma and paren/bracket/brace need to have the same owner
-    (rightOwner eq nft.meta.rightOwner) &&
-    (nft.right match {
-      case rp: Token.RightParen => rightOwner
+    // comma and paren/bracket/brace should generally have the same owner
+    // however, with optional-braces comma could be before outdent
+    // and hence owned by the previous expression
+    nft.right match {
+      case rp: Token.RightParen => delimOwner
           .isAny[Member.SyntaxValuesClause, Member.Tuple] ||
         ftoks.matchingOpt(rp).exists { lp =>
           val claimant = session.claimedRule(ftoks.justBefore(lp))
           claimant.forall(_.rule.isInstanceOf[RedundantParens])
         }
 
-      case _: Token.RightBracket => rightOwner.is[Member.SyntaxValuesClause]
+      case _: Token.RightBracket => delimOwner.is[Member.SyntaxValuesClause]
 
-      case _: Token.RightBrace => rightOwner.is[Importer]
+      case _: Token.RightBrace => delimOwner.is[Importer]
 
       case _ => false
-    })
+    }
   }
 
   override def onRight(lt: Replacement, hasFormatOff: Boolean)(implicit
diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
index 9761efca0e..794e662cbc 100644
--- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
+++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat
@@ -453,13 +453,11 @@ f(
   ,
 )
 >>>
-Idempotency violated
-=> Diff (- obtained, + expected)
-   else
--    1234567890
--  ,
-+    1234567890,
- )
+f(
+  if true then 0
+  else
+    1234567890,
+)
 <<< #4058 2
 maxColumn = 16
 runner.dialect = scala3
@@ -470,10 +468,8 @@ f(
     1234567890,
 )
 >>>
-Idempotency violated
-=> Diff (- obtained, + expected)
-   else
--    1234567890,
-+    1234567890
-+  ,
- )
+f(
+  if true then 0
+  else
+    1234567890,
+)