Router: implement binpack=onelineSjs conf option

scalameta · Jun 8, 2024 · cbfcd70 · cbfcd70
1 parent 4e8d721
commit cbfcd70
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 42 deletions.
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala
@@ -300,14 +300,19 @@ class FormatOps(
       getOneArgPerLineSplitsAfterComma(right, splits)
   }
 
+  def splitOneArgPerLineAfterCommaOnBreak(comma: T)(implicit
+      fileLine: FileLine,
+  ): Policy = splitOneArgPerLineAfterCommaOnBreak(TokenRanges.empty)(comma)
+
   def splitOneArgPerLineAfterCommaOnBreak(
-      comma: T,
-  )(implicit fileLine: FileLine): Policy = delayedBreakPolicyBefore(comma) {
-    Policy.after(comma, prefix = "NL->A[,]") {
+      exclude: TokenRanges,
+  )(comma: T)(implicit fileLine: FileLine): Policy = {
+    val policy = Policy.after(comma, prefix = "NL->A[,]") {
       case Decision(t @ FormatToken(`comma`, right, _), splits)
           if !right.is[T.Comment] || t.hasBreak =>
         getOneArgPerLineSplitsAfterComma(right, splits)
     }
+    delayedBreakPolicy(Policy.End < comma, exclude)(policy)
   }
 
   private def getOneArgPerLineSplitsAfterComma(r: T, s: Seq[Split]) =

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
@@ -1082,7 +1082,7 @@ class Router(formatOps: FormatOps) {
 
           val nextCommaOneline = argumentStarts.get(ft.meta.idx).flatMap { x =>
             val noNeed = isSeqSingle(getArgs(leftOwner)) ||
-              style.binPack.defnSiteFor(isBracket) != BinPack.Site.Oneline
+              !style.binPack.defnSiteFor(isBracket).isOneline
             if (noNeed) None else findFirstOnRight[T.Comma](getLast(x), close)
           }
 
@@ -1162,12 +1162,11 @@ class Router(formatOps: FormatOps) {
         def findComma(ft: FormatToken) = findFirstOnRight[T.Comma](ft, close)
           .map(_.right)
 
-        val oneline = style.binPack.callSiteFor(open) == BinPack.Site.Oneline
+        val binPack = style.binPack.callSiteFor(open)
+        val oneline = binPack.isOneline
         val nextCommaOneline =
           if (!oneline || isSingleArg) None
           else firstArg.map(getLast).flatMap(findComma)
-        val nextCommaOnelinePolicy = nextCommaOneline
-          .map(splitOneArgPerLineAfterCommaOnBreak)
 
         val noNextCommaOneline = oneline && nextCommaOneline.isEmpty
         val noNextCommaOnelineCurried = noNextCommaOneline &&
@@ -1179,6 +1178,11 @@ class Router(formatOps: FormatOps) {
         val exclude =
           if (!isBracket) insideBracesBlock(ft, close)
           else insideBlock[T.LeftBracket](ft, close)
+        val sjsOneline = !isBracket && binPack == BinPack.Site.OnelineSjs
+        val sjsExclude = exclude.getIf(sjsOneline)
+
+        val nextCommaOnelinePolicy = nextCommaOneline
+          .map(splitOneArgPerLineAfterCommaOnBreak(sjsExclude))
         val newlinesPenalty = 3 + indentLen * bracketPenalty
         val penalizeNewlinesPolicy =
           policyWithExclude(exclude, Policy.End.Before, Policy.End.On)(
@@ -1193,7 +1197,8 @@ class Router(formatOps: FormatOps) {
 
         val noSplit =
           if (nlOnly) Split.ignored
-          else if (slbOrNL) baseNoSplit.withSingleLine(close, noSyntaxNL = true)
+          else if (slbOrNL) baseNoSplit
+            .withSingleLine(close, sjsExclude, noSyntaxNL = true)
           else {
             def okSingleArgsIndents = singleArgAsInfix.isEmpty &&
               !noNextCommaOneline && style.binPack.indentCallSiteSingleArg &&
@@ -1221,13 +1226,14 @@ class Router(formatOps: FormatOps) {
                 if (noSplitIndents.exists(_.hasStateColumn)) p &
                   penalizeNewlinesPolicy
                 else {
-                  val slbEnd = nextCommaOneline.getOrElse(close)
-                  SingleLineBlock(slbEnd, noSyntaxNL = true) ==>
-                    penalizeNewlinesPolicy
+                  val end = nextCommaOneline.getOrElse(close)
+                  val slbPolicy =
+                    SingleLineBlock(end, noSyntaxNL = true, exclude = sjsExclude)
+                  slbPolicy ==> penalizeNewlinesPolicy
                 }
               }
             val indentPolicy = Policy ? noSplitIndents.isEmpty || {
-              def unindentPolicy = Policy ? isSingleArg &&
+              def unindentPolicy = Policy ? (isSingleArg || sjsOneline) &&
                 unindentAtExclude(exclude, Num(-indentLen))
               def indentOncePolicy =
                 Policy ? style.binPack.indentCallSiteOnce && {
@@ -1396,7 +1402,7 @@ class Router(formatOps: FormatOps) {
           else argumentStarts.get(ft.meta.idx).map { nextArg =>
             val lastFT = getLast(nextArg)
             val lastTok = lastFT.left
-            val oneline = binPack == BinPack.Site.Oneline
+            val oneline = binPack.isOneline
             val afterNextArg = nextNonComment(lastFT)
             val nextCommaOrParen = afterNextArg.right match {
               case _: T.Comma | _: T.RightParen | _: T.RightBracket =>
@@ -1421,14 +1427,17 @@ class Router(formatOps: FormatOps) {
                 else None
               }
 
+            val sjsExclude =
+              if (binPack ne BinPack.Site.OnelineSjs) TokenRanges.empty
+              else insideBracesBlock(ft, lastTok)
             val nlPolicy = Policy ? oneline && nextCommaOrParen.map {
               case FormatToken(_, t: T.Comma, _) =>
-                splitOneArgPerLineAfterCommaOnBreak(t)
+                splitOneArgPerLineAfterCommaOnBreak(sjsExclude)(t)
               case _ if callSite =>
                 def delayBreakBefore(token: T): Policy = {
                   // force break if multiline and if there's no other break
                   val lastEnd = lastTok.end
-                  delayedBreakPolicy(Policy.End == lastEnd)(
+                  delayedBreakPolicy(Policy.End == lastEnd, sjsExclude)(
                     Policy.RelayOnSplit { case (s, nextft) =>
                       s.isNL && nextft.right.end > lastEnd // don't need anymore
                     }(decideNewlinesOnlyBeforeToken(token), NoPolicy),
@@ -1467,7 +1476,8 @@ class Router(formatOps: FormatOps) {
                 val end = endOfSingleLineBlock(
                   nextCommaOrParen.flatMap(getEndOfResultType).getOrElse(lastFT),
                 )
-                Split(Space, 0).withSingleLine(end, noSyntaxNL = true)
+                Split(Space, 0)
+                  .withSingleLine(end, noSyntaxNL = true, exclude = sjsExclude)
               }
             Seq(noSplit, nlSplit)
           }

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/TokenRange.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/TokenRange.scala
@@ -10,6 +10,11 @@ class TokenRange private (val lt: Token, val rt: Token) {
 
 class TokenRanges private (val ranges: Seq[TokenRange]) extends AnyVal {
 
+  @inline
+  def isEmpty: Boolean = ranges.isEmpty
+  @inline
+  def getIf(flag: Boolean): TokenRanges = if (flag) this else TokenRanges.empty
+
   def append(range: TokenRange): TokenRanges = {
     ranges.headOption.foreach(range.validateAfter)
     new TokenRanges(range +: ranges)

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/PolicyOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/PolicyOps.scala
@@ -146,20 +146,22 @@ object PolicyOps {
       (lastPolicy <== endLt(range.lt)) ==> (endRt(range.rt) ==> policy)
     }
 
-  def delayedBreakPolicy(end: Policy.End.WithPos)(
-      onBreakPolicy: Policy,
-  )(implicit fileLine: FileLine): Policy = Policy ? onBreakPolicy.isEmpty ||
-    new Policy.Map(endPolicy = end, desc = onBreakPolicy.toString)({ s =>
-      if (s.isNL) s.orPolicy(onBreakPolicy) else s
-    })
-
-  def delayedBreakPolicyBefore(token: T)(onBreakPolicy: Policy)(implicit
-      fileLine: FileLine,
-  ): Policy = delayedBreakPolicy(Policy.End < token)(onBreakPolicy)
+  def delayedBreakPolicy(
+      end: => Policy.End.WithPos,
+      exclude: TokenRanges = TokenRanges.empty,
+  )(onBreakPolicy: Policy)(implicit fileLine: FileLine): Policy =
+    Policy ? onBreakPolicy.isEmpty ||
+      policyWithExclude(exclude, Policy.End.On, Policy.End.After)(
+        new Policy.Map(end, desc = onBreakPolicy.toString)({ s =>
+          if (s.isNL) s.orPolicy(onBreakPolicy) else s
+        }),
+      )
 
-  def delayedBreakPolicyFor(token: T)(f: T => Policy)(implicit
-      fileLine: FileLine,
-  ): Policy = delayedBreakPolicyBefore(token)(f(token))
+  def delayedBreakPolicyBefore(token: T)(onBreakPolicy: Policy): Policy =
+    delayedBreakPolicy(Policy.End < token)(onBreakPolicy)
+
+  def delayedBreakPolicyFor(token: T)(f: T => Policy): Policy =
+    delayedBreakPolicyBefore(token)(f(token))
 
   def decideNewlinesOnlyBeforeClose(close: T)(implicit
       fileLine: FileLine,

diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat
@@ -2889,10 +2889,9 @@ object a {
 }
 >>>
 object a {
-  assert(
-    fastOptFile.get(scalaJSSourceMap).exists {
-      _.getPath == fastOptFile.data.getPath + ".map"
-    }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
+  assert(fastOptFile.get(scalaJSSourceMap).exists {
+    _.getPath == fastOptFile.data.getPath + ".map"
+  }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
 }
 <<< binPack with named parameter values, configStyleArguments + danglingParentheses
 binPack.preset = true

diff --git a/scalafmt-tests/src/test/resources/newlines/source_fold.stat b/scalafmt-tests/src/test/resources/newlines/source_fold.stat
@@ -2737,8 +2737,8 @@ object a {
 >>>
 object a {
   assert(fastOptFile.get(scalaJSSourceMap).exists {
-      _.getPath == fastOptFile.data.getPath + ".map"
-    }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
+    _.getPath == fastOptFile.data.getPath + ".map"
+  }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
 }
 <<< binPack with named parameter values, danglingParentheses
 binPack.preset = true

diff --git a/scalafmt-tests/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/src/test/resources/newlines/source_keep.stat
@@ -2890,8 +2890,8 @@ object a {
 >>>
 object a {
   assert(fastOptFile.get(scalaJSSourceMap).exists {
-      _.getPath == fastOptFile.data.getPath + ".map"
-    }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
+    _.getPath == fastOptFile.data.getPath + ".map"
+  }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
 }
 <<< binPack with named parameter values, danglingParentheses
 binPack.preset = true

diff --git a/scalafmt-tests/src/test/resources/newlines/source_unfold.stat b/scalafmt-tests/src/test/resources/newlines/source_unfold.stat
@@ -3075,7 +3075,8 @@ object a {
       .get(scalaJSSourceMap)
       .exists {
         _.getPath == fastOptFile.data.getPath + ".map"
-      }, "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
+      },
+    "fastOptJS does not have the correct scalaJSSourceMap attribute").foo
 }
 <<< binPack with named parameter values, danglingParentheses
 binPack.preset = true

diff --git a/scalafmt-tests/src/test/resources/scalajs/Apply.stat b/scalafmt-tests/src/test/resources/scalajs/Apply.stat
@@ -1056,10 +1056,9 @@ object a {
 }
 >>>
 object a {
-  js.MethodDef(flags, methodIdent, originalName, jsParams, resultType,
-      Some {
-        genApplyMethod(genLoadModule(moduleClass), m, jsParams.map(_.ref))
-      })(OptimizerHints.empty, Unversioned)
+  js.MethodDef(flags, methodIdent, originalName, jsParams, resultType, Some {
+    genApplyMethod(genLoadModule(moduleClass), m, jsParams.map(_.ref))
+  })(OptimizerHints.empty, Unversioned)
 }
 <<< multi-nested with onelineSjs, keep, and mixed paren-brace args
 preset = default