RedundantBraces: improve check around infix

scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala

@@ -533,20 +533,31 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
   )(implicit ft: FT, style: ScalafmtConfig): Boolean = p match {
     case _: Member.Infix =>
       /* for infix, we will preserve the block unless the closing brace
+       * is not followed by an operator of a further infix expression, or
        * follows a non-whitespace character on the same line as we don't
        * break lines around infix expressions.
        * we shouldn't join with the previous line (which might also end
        * in a comment), and if we keep the break before the right brace
        * we are removing, that will likely invalidate the expression. */
-      def checkOpen = {
-        val nft = ftoks.next(ft)
-        nft.noBreak || style.formatInfix(p) && !nft.right.is[T.Comment]
+      val rft = ftoks.matchingRight(ft)
+      def checkAfterRight(wasNonComment: => Boolean) = {
+        val nrft = ftoks.nextNonComment(rft)
+        !nrft.right.is[T.Ident] || wasNonComment && style.formatInfix(p) ||
+        findTreeWithParent(p) { // check if infix is in parens
+          case pp: Member.ArgClause => ftoks.getClosingIfWithinParensOrBraces(pp)
+              .map(_.isRight)
+          case _: Member.Infix => None
+          case _: Tree.Block => Some(false)
+          case pp => Some(ftoks.isEnclosedWithinParens(pp))
+        }.isDefined
       }
-      def checkClose = {
-        val nft = ftoks.prev(ftoks.matchingRight(ft))
-        nft.noBreak || style.formatInfix(p) && !nft.left.is[T.Comment]
+      val prft = ftoks.prev(rft)
+      prft.left match {
+        case _: T.Comma => RewriteTrailingCommas.enabled &&
+          checkAfterRight(!ftoks.prev(prft).left.is[T.Comment])
+        case _: T.Comment => prft.noBreak || checkAfterRight(false)
+        case _ => prft.noBreak || checkAfterRight(true)
       }
-      checkOpen && checkClose
     case _ => true
   }
 

scalafmt-tests-community/intellij/src/test/scala/org/scalafmt/community/intellij/CommunityIntellijScalaSuite.scala

@@ -13,7 +13,7 @@ abstract class CommunityIntellijScalaSuite(name: String)
 class CommunityIntellijScala_2024_2_Suite
     extends CommunityIntellijScalaSuite("intellij-scala-2024.2") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(59238466)
+  override protected def totalStatesVisited: Option[Int] = Some(59238459)
 
   override protected def builds = Seq(getBuild(
     "2024.2.28",
@@ -52,7 +52,7 @@ class CommunityIntellijScala_2024_2_Suite
 class CommunityIntellijScala_2024_3_Suite
     extends CommunityIntellijScalaSuite("intellij-scala-2024.3") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(59452507)
+  override protected def totalStatesVisited: Option[Int] = Some(59452500)
 
   override protected def builds = Seq(getBuild(
     "2024.3.4",

scalafmt-tests-community/scala2/src/test/scala/org/scalafmt/community/scala2/CommunityScala2Suite.scala

@@ -9,7 +9,7 @@ abstract class CommunityScala2Suite(name: String)
 
 class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(42554888)
+  override protected def totalStatesVisited: Option[Int] = Some(42554770)
 
   override protected def builds =
     Seq(getBuild("v2.12.20", dialects.Scala212, 1277))
@@ -18,7 +18,7 @@ class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {
 
 class CommunityScala2_13Suite extends CommunityScala2Suite("scala-2.13") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(53285090)
+  override protected def totalStatesVisited: Option[Int] = Some(53284940)
 
   override protected def builds =
     Seq(getBuild("v2.13.14", dialects.Scala213, 1287))

scalafmt-tests-community/scala3/src/test/scala/org/scalafmt/community/scala3/CommunityScala3Suite.scala

@@ -9,15 +9,15 @@ abstract class CommunityScala3Suite(name: String)
 
 class CommunityScala3_2Suite extends CommunityScala3Suite("scala-3.2") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(39175052)
+  override protected def totalStatesVisited: Option[Int] = Some(39174973)
 
   override protected def builds = Seq(getBuild("3.2.2", dialects.Scala32, 791))
 
 }
 
 class CommunityScala3_3Suite extends CommunityScala3Suite("scala-3.3") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(42300471)
+  override protected def totalStatesVisited: Option[Int] = Some(42300385)
 
   override protected def builds = Seq(getBuild("3.3.3", dialects.Scala33, 861))
 

scalafmt-tests-community/spark/src/test/scala/org/scalafmt/community/spark/CommunitySparkSuite.scala

@@ -9,15 +9,15 @@ abstract class CommunitySparkSuite(name: String)
 
 class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(86105939)
+  override protected def totalStatesVisited: Option[Int] = Some(86105465)
 
   override protected def builds = Seq(getBuild("v3.4.1", dialects.Scala213, 2585))
 
 }
 
 class CommunitySpark3_5Suite extends CommunitySparkSuite("spark-3.5") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(91105919)
+  override protected def totalStatesVisited: Option[Int] = Some(91105553)
 
   override protected def builds = Seq(getBuild("v3.5.3", dialects.Scala213, 2756))
 

scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat

@@ -10687,11 +10687,10 @@ def isNeverSubType(tp1: Type, tp2: Type): Boolean = /*logResult(s"isNeverSubType
   tp2.dealias
 ) match {
   case (TypeRef(_, sym1, args1), TypeRef(_, sym2, args2)) =>
-    isNeverSubClass(sym1, sym2) || {
-      (sym1 isSubClass sym2) && {
-        val tp1seen = tp1 baseType sym2
-        isNeverSubArgs(tp1seen.typeArgs, args2, sym2.typeParams)
-      }
+    isNeverSubClass(sym1, sym2) ||
+    (sym1 isSubClass sym2) && {
+      val tp1seen = tp1 baseType sym2
+      isNeverSubArgs(tp1seen.typeArgs, args2, sym2.typeParams)
     }
   case _ => false
 }
@@ -10718,11 +10717,10 @@ override def isHidden(dia: Diagnostic)(using Context): Boolean =
   }
 >>>
 override def isHidden(dia: Diagnostic)(using Context): Boolean =
-  super.isHidden(dia) || {
+  super.isHidden(dia) ||
     dia.msg.isNonSensical &&
     hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
     !ctx.settings.YshowSuppressedErrors.value
-  }
 <<< #4133 redundant braces around inner infix
 maxColumn = 78
 rewrite.rules = [RedundantBraces, RedundantParens]
@@ -10738,10 +10736,9 @@ lazy val defaultSettings: Seq[Setting[_]] = Def.settings(
 >>>
 lazy val defaultSettings: Seq[Setting[_]] = Def.settings(
   Dependencies.Versions,
-  Compile / javacOptions ++= {
+  Compile / javacOptions ++=
     DefaultJavacOptions ++
-      JdkOptions.targetJdkJavacOptions(targetSystemJdk.value)
-  },
+      JdkOptions.targetJdkJavacOptions(targetSystemJdk.value),
   resolverSettings
 )
 <<< #4133 redundant braces with overflow

scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat

@@ -10440,11 +10440,10 @@ def isNeverSubType(tp1: Type, tp2: Type): Boolean = /*logResult(s"isNeverSubType
 def isNeverSubType(tp1: Type, tp2: Type): Boolean = /*logResult(s"isNeverSubType($tp1, $tp2)")*/
   (tp1.dealias, tp2.dealias) match {
     case (TypeRef(_, sym1, args1), TypeRef(_, sym2, args2)) =>
-      isNeverSubClass(sym1, sym2) || {
-        (sym1 isSubClass sym2) && {
-          val tp1seen = tp1 baseType sym2
-          isNeverSubArgs(tp1seen.typeArgs, args2, sym2.typeParams)
-        }
+      isNeverSubClass(sym1, sym2) ||
+      (sym1 isSubClass sym2) && {
+        val tp1seen = tp1 baseType sym2
+        isNeverSubArgs(tp1seen.typeArgs, args2, sym2.typeParams)
       }
     case _ => false
   }
@@ -10471,11 +10470,10 @@ override def isHidden(dia: Diagnostic)(using Context): Boolean =
   }
 >>>
 override def isHidden(dia: Diagnostic)(using Context): Boolean =
-  super.isHidden(dia) || {
+  super.isHidden(dia) ||
     dia.msg.isNonSensical &&
     hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
     !ctx.settings.YshowSuppressedErrors.value
-  }
 <<< #4133 redundant braces around inner infix
 maxColumn = 78
 rewrite.rules = [RedundantBraces, RedundantParens]
@@ -10491,10 +10489,9 @@ lazy val defaultSettings: Seq[Setting[_]] = Def.settings(
 >>>
 lazy val defaultSettings: Seq[Setting[_]] = Def.settings(
   Dependencies.Versions,
-  Compile / javacOptions ++= {
+  Compile / javacOptions ++=
     DefaultJavacOptions ++
-      JdkOptions.targetJdkJavacOptions(targetSystemJdk.value)
-  },
+      JdkOptions.targetJdkJavacOptions(targetSystemJdk.value),
   resolverSettings
 )
 <<< #4133 redundant braces with overflow

scalafmt-tests/shared/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat

@@ -279,15 +279,14 @@ object a {
   lowClass.isJavaDefined && highClass.isJavaDefined && { // skip if both are java-defined, and
     lowClass.isNonBottomSubClass(
       highClass
-    ) || { //  - low <:< high, which means they are overrides in Java and javac is doing the check; or
-      base.info.parents.tail.forall {
-        p => //  - every mixin parent is unrelated to (not a subclass of) low and high, i.e.,
-          val psym =
-            p.typeSymbol //    we're not mixing in high or low, both are coming from the superclass
-          !psym.isNonBottomSubClass(lowClass) && !psym.isNonBottomSubClass(
-            highClass
-          )
-      }
+    ) || //  - low <:< high, which means they are overrides in Java and javac is doing the check; or
+    base.info.parents.tail.forall {
+      p => //  - every mixin parent is unrelated to (not a subclass of) low and high, i.e.,
+        val psym =
+          p.typeSymbol //    we're not mixing in high or low, both are coming from the superclass
+        !psym.isNonBottomSubClass(lowClass) && !psym.isNonBottomSubClass(
+          highClass
+        )
     }
   }
 }
@@ -307,13 +306,12 @@ object a {
 >>>
 object a {
   lowClass.isJavaDefined && highClass.isJavaDefined && {
-    lowClass.isNonBottomSubClass(highClass) || {
-      base.info.parents.tail.forall { p =>
-        val psym = p.typeSymbol
-        !psym.isNonBottomSubClass(lowClass) && !psym.isNonBottomSubClass(
-          highClass
-        )
-      }
+    lowClass.isNonBottomSubClass(highClass) ||
+    base.info.parents.tail.forall { p =>
+      val psym = p.typeSymbol
+      !psym.isNonBottomSubClass(lowClass) && !psym.isNonBottomSubClass(
+        highClass
+      )
     }
   }
 }

scalafmt-tests/shared/src/test/resources/rewrite/RedundantBraces2.stat

@@ -448,3 +448,62 @@ def foo = {
   // this path intentionally survives a `clean`
   (LocalRootProject / baseDirectory).value / "test-suite/target/test-suite-stability.js",
 }
+<<< braces in infix, with a trailing comma
+Def.settings(
+  saveForStabilityTest / artifactPath := {
+    // this path intentionally survives a `clean`
+    (LocalRootProject / baseDirectory).value / "test-suite/target/test-suite-stability.js" /* c */,
+  },
+  saveForStabilityTest := foo
+)
+>>>
+Def.settings(
+  saveForStabilityTest / artifactPath :=
+    // this path intentionally survives a `clean`
+    (LocalRootProject / baseDirectory).value / "test-suite/target/test-suite-stability.js" /* c */
+  ,
+  saveForStabilityTest := foo
+)
+<<< braces at end of infix expressions
+newlines.source = fold
+newlines.afterInfix = keep
+===
+Switch(typeof(instance), List(
+    str("string") -> {
+      Return(constantClassResult(BoxedStringClass))
+    },
+    str("boolean") -> {
+      Return(constantClassResult(BoxedBooleanClass))
+    },
+    str("undefined") -> {
+      Return(constantClassResult(BoxedUnitClass))
+    } + 1
+))
+>>>
+Switch(
+  typeof(instance),
+  List(
+    str("string") ->
+      Return(constantClassResult(BoxedStringClass)),
+    str("boolean") ->
+      Return(constantClassResult(BoxedBooleanClass)),
+    str("undefined") ->
+      Return(constantClassResult(BoxedUnitClass)) + 1
+  )
+)
+<<< closing brace between comment and comma
+newlines.source = fold
+newlines.afterInfix = keep
+===
+dataFrameCachingWrapper(
+  (args: FnArgsWithId) => {
+    fn(args.df, args.batchId) // dfId is not used, see hack comment above.
+  },
+  sessionHolder)
+>>>
+dataFrameCachingWrapper(
+  (args: FnArgsWithId) =>
+    fn(args.df, args.batchId) // dfId is not used, see hack comment above.
+  ,
+  sessionHolder
+)

scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala

@@ -144,7 +144,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
     val explored = Debug.explored.get()
     logger.debug(s"Total explored: $explored")
     if (!onlyUnit && !onlyManual)
-      assertEquals(explored, 1204679, "total explored")
+      assertEquals(explored, 1204920, "total explored")
     val results = debugResults.result()
     // TODO(olafur) don't block printing out test results.
     // I don't want to deal with scalaz's Tasks :'(