FormatOps: use optimal slb end in binpacked (...

scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala

@@ -260,27 +260,27 @@ class FormatOps(
 
   // invoked on closing paren, part of ParamClause
   @tailrec
-  final def defnSiteLastToken(close: FormatToken, t: Tree): T = {
-    def alt = getLastToken(t)
+  final def defnSiteLastToken(close: FormatToken, t: Tree): FormatToken = {
+    def alt = getLast(t)
     t match {
       case _: Term.ParamClause | _: Type.FuncParamClause |
           _: Member.ParamClauseGroup => t.parent match {
           case Some(p) => defnSiteLastToken(close, p)
           case _ => alt
         }
       case t: Defn.Def => getHeadOpt(t.body).fold(alt) { ft =>
-          if (ft.left.is[T.LeftBrace] && t.body.is[Term.Block]) ft.left
-          else prevNonCommentBefore(ft).left
+          if (ft.left.is[T.LeftBrace] && t.body.is[Term.Block]) ft
+          else prevNonCommentBefore(ft)
         }
       case _: Ctor.Primary => close match {
           // This is a terrible terrible hack. Please consider removing this.
           // The RightParen() LeftBrace() pair is presumably a ") {" combination
           // at a class definition
-          case FormatToken(_: T.RightParen, b: T.LeftBrace, _) => b
-          case _ => close.left
+          case FormatToken(_: T.RightParen, _: T.LeftBrace, _) => next(close)
+          case _ => close
         }
       case t => t.tokens.find(x => x.is[T.Equals] && owners(x) == t)
-          .getOrElse(alt)
+          .fold(alt)(before)
     }
   }
 
@@ -345,14 +345,14 @@ class FormatOps(
 
   def templateDerivesOrCurlyOrLastNonTrivial(template: Template)(implicit
       ft: FormatToken,
-  ): T = findTemplateGroupOnRight(_.getExpireToken)(template)
-    .getOrElse(templateCurlyOrLastNonTrivial(template).left)
+  ): FormatToken = findTemplateGroupOnRight(_.getExpireToken)(template)
+    .getOrElse(templateCurlyOrLastNonTrivial(template))
 
   private def findTreeInGroup[A](
       trees: Seq[Tree],
       func: TemplateSupertypeGroup => A,
-  )(expireFunc: Seq[Tree] => T)(implicit ft: FormatToken): Option[A] = trees
-    .find(_.pos.end >= ft.right.end).map { x =>
+  )(expireFunc: Seq[Tree] => FormatToken)(implicit ft: FormatToken): Option[A] =
+    trees.find(_.pos.end >= ft.right.end).map { x =>
       func(TemplateSupertypeGroup(x, trees, expireFunc))
     }
 
@@ -364,11 +364,11 @@ class FormatOps(
       if (isSeqSingle(groups))
         // for the last group, return '{' or ':'
         findTreeInGroup(groups.head, func) { x =>
-          getHeadOpt(template.body).getOrElse(getLastNonTrivial(x.last)).left
+          getHeadOpt(template.body).getOrElse(getLastNonTrivial(x.last))
         }
       else {
         // for intermediate groups, return its last token
-        val res = findTreeInGroup(groups.head, func)(x => tokenAfter(x).left)
+        val res = findTreeInGroup(groups.head, func)(tokenAfter)
         if (res.isDefined) res else iter(groups.tail)
       }
     getTemplateGroups(template).flatMap(iter)
@@ -971,7 +971,7 @@ class FormatOps(
       // this method is called on a `with` or comma; hence, it can
       // only refer to second or subsequent init/derive in a group
       // we'll indent only the second, but not any subsequent ones
-      val expire = x.getExpireToken
+      val expire = x.getExpireToken.left
       val indent =
         if (!x.isSecond) Indent.Empty
         else Indent(Num(indentIfSecond), expire, ExpiresOn.After)
@@ -1013,22 +1013,23 @@ class FormatOps(
       isFirstCtor: Boolean,
       owners: => Set[Tree],
       rhs: => Option[Tree],
-      lastToken: T,
+      lastFt: FormatToken,
       indentLen: Int,
       extendsThenWith: => Boolean = false,
   )(implicit
       fileLine: FileLine,
       ft: FormatToken,
       style: ScalafmtConfig,
   ): Seq[Split] = {
+    val lastToken = lastFt.left
     val nlMod = NewlineT(alt = Some(Space))
     val indent =
       if (!isFirstCtor) Indent.Empty
       else Indent(Num(indentLen), lastToken, ExpiresOn.After)
     if (style.binPack.keepParentConstructors)
       if (ft.hasBreak) Seq(Split(nlMod, 0).withIndent(indent))
       else {
-        val slbEnd = rhs.fold(lastToken)(getLastToken)
+        val slbEnd = endOfSingleLineBlock(rhs.fold(lastFt)(getLast))
         Seq(
           Split(Space, 0).withIndent(indent).withSingleLine(
             slbEnd,
@@ -1056,9 +1057,10 @@ class FormatOps(
       }
       val noSyntaxNL = extendsThenWith
       val pnlPolicy = PenalizeAllNewlines(lastToken, 1, noSyntaxNL = noSyntaxNL)
+      val slbEnd = endOfSingleLineBlock(lastFt)
       Seq(
         Split(Space, 0)
-          .withSingleLine(lastToken, exclude = exclude, noSyntaxNL = noSyntaxNL)
+          .withSingleLine(slbEnd, exclude = exclude, noSyntaxNL = noSyntaxNL)
           .orPolicy(pnlPolicy).withIndent(indent),
         Split(nlMod, 0).onlyIf(nlOnelineTag != Right(false))
           .preActivateFor(nlOnelineTag.left.toOption)
@@ -3017,7 +3019,7 @@ object FormatOps {
   case class TemplateSupertypeGroup(
       superType: Tree,
       superTypeGroup: Seq[Tree],
-      expireTokenFunc: Seq[Tree] => T,
+      expireTokenFunc: Seq[Tree] => FormatToken,
   ) {
     def getExpireToken = expireTokenFunc(superTypeGroup)
     def isSecond = superTypeGroup.drop(1).headOption.contains(superType)

scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala

@@ -764,7 +764,7 @@ class Router(formatOps: FormatOps) {
           ) =>
         val policy = Policy ?
           (style.binPack.keepParentConstructors || template.pos.isEmpty) || {
-            val expire = templateDerivesOrCurlyOrLastNonTrivial(template)
+            val expire = templateDerivesOrCurlyOrLastNonTrivial(template).left
             val forceNewlineBeforeExtends = Policy.before(expire, "NLPCTOR") {
               case Decision(FormatToken(_, soft.ExtendsOrDerives(), m), s)
                   if m.rightOwner eq template =>
@@ -903,10 +903,11 @@ class Router(formatOps: FormatOps) {
         val isBeforeOpenParen =
           if (defnSite) style.newlines.isBeforeOpenParenDefnSite
           else style.newlines.isBeforeOpenParenCallSite
-        val optimal: T =
-          if (isBeforeOpenParen) close
-          else if (!defnSite || isBracket) endOfSingleLineBlock(afterClose)
+        val optimalFt: FormatToken =
+          if (isBeforeOpenParen) afterClose
+          else if (!defnSite || isBracket) getSlbEndOnLeft(afterClose)
           else defnSiteLastToken(afterClose, leftOwner)
+        val optimal = optimalFt.left
 
         val wouldDangle = onlyConfigStyle || mustDangleForTrailingCommas ||
           dangleCloseDelim || closeBreak && beforeClose.left.is[T.Comment]
@@ -1892,7 +1893,7 @@ class Router(formatOps: FormatOps) {
           true,
           Set(rightOwner),
           enumCase.inits.headOption,
-          getLastToken(rightOwner),
+          getLast(rightOwner),
           style.indent.extendSite,
           enumCase.inits.lengthCompare(1) > 0,
         )
@@ -1929,7 +1930,7 @@ class Router(formatOps: FormatOps) {
               isFirstCtor,
               Set(template),
               findTemplateGroupOnRight(_.superType)(template),
-              templateCurlyOrLastNonTrivial(template).left,
+              templateCurlyOrLastNonTrivial(template),
               style.indent.main,
               extendsThenWith,
             )
@@ -1940,7 +1941,7 @@ class Router(formatOps: FormatOps) {
               !t.lhs.is[Type.With],
               withChain(top).toSet,
               Some(t.rhs),
-              top.tokens.last,
+              getLast(top),
               style.indent.main,
             )
           case enumCase: Defn.EnumCase =>

scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces.stat

@@ -7331,3 +7331,14 @@ object a:
      else
         val found = findRefRecur(previous, prevPrec, prevCtx)
    end checkImportAlternatives
+<<< #4133 binpack with enum cases and trailing comment
+binPack.preset = always
+===
+object Parsers:
+  enum Location(inArgs: Boolean):
+    case InPatternArgs extends Location(false, true, true) // InParens not true, since it might be an alternative
+>>>
+object Parsers:
+   enum Location(inArgs: Boolean):
+      case InPatternArgs extends Location(false, true,
+            true) // InParens not true, since it might be an alternative

scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat

@@ -7051,3 +7051,15 @@ object a:
      else
         val found = findRefRecur(previous, prevPrec, prevCtx)
    end checkImportAlternatives
+<<< #4133 binpack with enum cases and trailing comment
+binPack.preset = always
+===
+object Parsers:
+  enum Location(inArgs: Boolean):
+    case InPatternArgs extends Location(false, true, true) // InParens not true, since it might be an alternative
+>>>
+object Parsers:
+   enum Location(inArgs: Boolean):
+      case InPatternArgs extends Location(
+            false, true, true
+          ) // InParens not true, since it might be an alternative

scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_keep.stat

@@ -7362,3 +7362,14 @@ object a:
      else
         val found = findRefRecur(previous, prevPrec, prevCtx)
    end checkImportAlternatives
+<<< #4133 binpack with enum cases and trailing comment
+binPack.preset = always
+===
+object Parsers:
+  enum Location(inArgs: Boolean):
+    case InPatternArgs extends Location(false, true, true) // InParens not true, since it might be an alternative
+>>>
+object Parsers:
+   enum Location(inArgs: Boolean):
+      case InPatternArgs extends Location(false, true,
+            true) // InParens not true, since it might be an alternative

scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_unfold.stat

@@ -7622,3 +7622,15 @@ object a:
      else
         val found = findRefRecur(previous, prevPrec, prevCtx)
    end checkImportAlternatives
+<<< #4133 binpack with enum cases and trailing comment
+binPack.preset = always
+===
+object Parsers:
+  enum Location(inArgs: Boolean):
+    case InPatternArgs extends Location(false, true, true) // InParens not true, since it might be an alternative
+>>>
+object Parsers:
+   enum Location(inArgs: Boolean):
+      case InPatternArgs extends Location(
+            false, true, true
+          ) // InParens not true, since it might be an alternative

scalafmt-tests/shared/src/test/resources/unit/Comment.stat

@@ -219,7 +219,8 @@ object literal extends scala.Dynamic { // scalastyle:ignore
     ???
 }
 >>>
-object literal extends scala.Dynamic { // scalastyle:ignore
+object literal
+    extends scala.Dynamic { // scalastyle:ignore
   ???
 }
 <<< #1234 1: single-line