Router: skip optimal in apply parens for keep

scalameta · Sep 3, 2024 · eeff171 · eeff171
1 parent 87c8d4a
commit eeff171
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 67 deletions.
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
@@ -380,15 +380,24 @@ class Router(formatOps: FormatOps) {
           }
           val toParens = initStyle.rewrite.bracesToParensForOneLineApply &&
             RedundantBraces.noSplitForParensOnRightBrace(closeFT).isDefined
-          val noCloseSpacePolicy = Policy ? toParens &&
-            Policy.End < close ==> Policy.on(close, "PAREN1LAPPLY") {
-              case Decision(FormatToken(_, `close`, _), ss) => ss.map { s =>
-                  if (s.isNL) s else s.withMod(NoSplit)
-                }
-            }
-          Split(if (toParens) NoSplit else xmlSpace(leftOwner), 0)
+          if (toParens) {
+            val noCloseSpacePolicy = Policy.End < close ==>
+              Policy.on(close, "PAREN1LAPPLY") {
+                case Decision(FormatToken(_, `close`, _), ss) => ss.map { s =>
+                    if (s.isNL) s else s.withMod(NoSplit)
+                  }
+              }
+            // copy logic from `( ...`, binpack=never, defining `slbSplit`
+            val isBeforeOpenParen = style.newlines.isBeforeOpenParenCallSite
+            Split(NoSplit, 0).withSingleLine(
+              if (isBeforeOpenParen) closeFT.left else rhsOptimalToken(closeFT),
+              noSyntaxNL = true,
+              killOnFail = true,
+              noOptimal = style.newlines.source eq Newlines.keep,
+            ).andPolicy(sldPolicy & noCloseSpacePolicy)
+          } else Split(xmlSpace(leftOwner), 0)
             .withSingleLine(expire, noSyntaxNL = true, killOnFail = true)
-            .andPolicy(sldPolicy).andPolicy(noCloseSpacePolicy)
+            .andPolicy(sldPolicy)
         }
 
         val splits = Seq(
@@ -1049,9 +1058,10 @@ class Router(formatOps: FormatOps) {
                   )
                   else if (splitsForAssign.isDefined) singleLine(3)
                   else singleLine(10)
+                val noOptimal = style.newlines.source eq Newlines.keep
                 val okIndent = rightIsComment || handleImplicit
                 Split(noSplitMod, 0, policy = noSplitPolicy)
-                  .withOptimalToken(optimal)
+                  .withOptimalToken(optimal, ignore = noOptimal)
                   .withIndent(indent, ignore = !okIndent)
               }
             val useoneArgPerLineSplit = (notTooManyArgs && align) ||
@@ -1429,7 +1439,12 @@ class Router(formatOps: FormatOps) {
           if (!initStyle.rewrite.bracesToParensForOneLineApply) None
           else RedundantBraces
             .noSplitForParensOnRightBrace(tokens(matching(lb))).map { rbft =>
-              Split(NoSplit, 0).withSingleLine(rbft.left)
+              // copy logic from `( ...`, binpack=never, defining `slbSplit`
+              val isBeforeOpenParen = style.newlines.isBeforeOpenParenCallSite
+              val optimal: T =
+                if (isBeforeOpenParen) rbft.left else rhsOptimalToken(rbft)
+              val noOptimal = style.newlines.source eq Newlines.keep
+              Split(NoSplit, 0).withSingleLine(optimal, noOptimal = noOptimal)
             }
         Seq(slbParensSplit.getOrElse(Split.ignored), Split(Space, 0))
 

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala
@@ -184,6 +184,7 @@ case class Split(
       rank: Int = 0,
       extend: Boolean = false,
       ignore: Boolean = false,
+      noOptimal: Boolean = false,
   )(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
     if (isIgnored || ignore) this
     else {
@@ -196,6 +197,7 @@ case class Split(
         killOnFail,
         rank,
         extend,
+        noOptimal = noOptimal,
       )
     }
 
@@ -207,8 +209,9 @@ case class Split(
       killOnFail: Boolean = false,
       rank: Int = 0,
       extend: Boolean = false,
+      noOptimal: Boolean = false,
   )(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
-    withOptimalToken(optimal, killOnFail, extend = extend)
+    withOptimalToken(optimal, killOnFail, extend = extend, ignore = noOptimal)
       .withSingleLineNoOptimal(
         expire,
         exclude,

diff --git a/scalafmt-tests/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/src/test/resources/newlines/source_keep.stat
@@ -945,8 +945,8 @@ object a {
 }
 >>>
 object a {
-  (intercept[java.lang.IllegalStateException] { in.readObject })
-    .getMessage should ===(
+  (intercept[
+    java.lang.IllegalStateException] { in.readObject }).getMessage should ===(
     "Trying to deserialize a serialized ActorRef without an ActorSystem in scope." +
       " Use 'akka.serialization.Serialization.currentSystem.withValue(system) { ... }'")
 }
@@ -2069,8 +2069,9 @@ object a {
 }
 >>>
 object a {
-  plugins(service.pluginType) += service
-    .pluginName -> service
+  plugins(
+    service.pluginType
+  ) += service.pluginName -> service
 }
 <<< 8.7
 maxColumn = 80
@@ -2237,9 +2238,9 @@ val a = b match {
         ) ::
         JField(
           "env",
-          Extraction.decompose(i.env)(
-            DefaultFormats
-          )
+          Extraction.decompose(
+            i.env
+          )(DefaultFormats)
         ) ::
         JField(
           "sparkConf",
@@ -3608,8 +3609,9 @@ pubSubMessage match {
   case message: UploadMessage => {
     val services = makeUploadServices()
 
-    UploadSubscriber(services)
-      .processEff(message)
+    UploadSubscriber(
+      services
+    ).processEff(message)
   }
   case message: IngestionMessage => (
       IngestionSubscriber.defaultServices
@@ -6160,8 +6162,9 @@ object a {
         // bridge is needed to compile these projects.
       dependsOn(
         `scala3-compiler` % Provided
-      ).settings(commonJavaSettings)
-      .settings(foo)
+      ).settings(
+        commonJavaSettings
+      ).settings(foo)
 }
 <<< try with match expr
 object a {
@@ -8501,11 +8504,11 @@ object a {
 }
 >>>
 object a {
-  val current
-      : Map[String, Object] = template.templateFile.settings.getOrElse(
-    "page",
-    Map.empty
-  ).asInstanceOf[Map[String, Object]]
+  val current: Map[String, Object] =
+    template.templateFile.settings.getOrElse(
+      "page",
+      Map.empty
+    ).asInstanceOf[Map[String, Object]]
 }
 <<< redundant braces around nested lambda, parens around outer
 rewrite.rules = [RedundantBraces]

diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat
@@ -6211,17 +6211,13 @@ object a {
 }
 >>>
 object a {
-  val (diff: Set[File], msg: String) =
-    if packlibJars.size > hashbangClasspathJars.size then
-       (
-         packlibJars.toSet -- hashbangJars.toSet,
-         "only in packlib classpath"
-       )
-    else
-       (
-         hashbangJars.toSet -- packlibJars.toSet,
-         "only in hashbang classpath"
-       )
+  val (
+    diff: Set[File],
+    msg: String
+  ) = if packlibJars.size > hashbangClasspathJars.size then
+     (packlibJars.toSet -- hashbangJars.toSet, "only in packlib classpath")
+  else
+     (hashbangJars.toSet -- packlibJars.toSet, "only in hashbang classpath")
 }
 <<< complex code with selects and brace lambda 1
 maxColumn = 81
@@ -6282,17 +6278,14 @@ object a:
 >>>
 object a:
    rootsForRelease_foreach(root =>
-     Files.walkingIterator().useScalaFilter(Files__Is__Directory).foreach {
-       p =>
-          val_moduleNamePathElementCount_е_0
-          if (
-            p_getNameCount_g_root_getNameCount_p_moduleNamePathElementCount
-          ) {
-            val_packageDotted_e_p_subpath_moduleNamePathElementCountFooRoot__p__toString_replace
-            index_getOrElseUpdate(
-              packageDotted__new_collectionMutableListBuffer
-            )
-          }
+     Files.walkingIterator().useScalaFilter(
+       Files__Is__Directory
+     ).foreach { p =>
+        val_moduleNamePathElementCount_е_0
+        if (p_getNameCount_g_root_getNameCount_p_moduleNamePathElementCount) {
+          val_packageDotted_e_p_subpath_moduleNamePathElementCountFooRoot__p__toString_replace
+          index_getOrElseUpdate(packageDotted__new_collectionMutableListBuffer)
+        }
      }
    )
 <<< braces to parens in one-line apply: overflow with braces, fit with parens
@@ -6759,13 +6752,16 @@ object a:
       ).asInstanceOf[Tuple]
   )
 >>>
-Idempotency violated
-=> Diff (- obtained, + expected)
-         (baz match
--             case qux =>
--               TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
-+             case qux => TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
-                  Object
+object a:
+   def foo = (bar match
+      case _ =>
+        (baz match
+             case qux =>
+               TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
+                 Object
+               ].toArray_asInstanceOf[IArray_Object] // TODO
+        ).asInstanceOf[Tuple]
+   )
 <<< #4133 enclosed match within enclosed match, with overflow and comment, and rewrite
 rewrite.scala3.removeOptionalBraces = yes
 ===
@@ -6777,13 +6773,16 @@ object a:
       }).asInstanceOf[Tuple]
   })
 >>>
-Idempotency violated
-=> Diff (- obtained, + expected)
-         (baz match
--             case qux =>
--               TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
-+             case qux => TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
-                  Object
+object a:
+   def foo = (bar match
+      case _ =>
+        (baz match
+             case qux =>
+               TupleXXL_fromIArray_xs_productIterator_map_asInstanceOf[
+                 Object
+               ].toArray_asInstanceOf[IArray_Object] // TODO
+        ).asInstanceOf[Tuple]
+   )
 <<< #4133 complex overflow apply
 object a:
   def load(config: Seq[String], revision: Option[String], projectRoot: Path = Paths.get("").toAbsolutePath)(using CompilerContext): SourceLinks =
@@ -6806,8 +6805,7 @@ object a:
        projectRoot: Path = Paths.get("").toAbsolutePath
    )(using CompilerContext): SourceLinks =
      PathBased.parse(config, projectRoot)(using
-       SourceLinkParser(revision)
-     ) match {
+       SourceLinkParser(revision)) match {
        case PathBased.ParsingResult(errors, sourceLinks) =>
          if errors.nonEmpty then
             report.warning(
@@ -6827,7 +6825,9 @@ object a:
 >>>
 object a:
    val hashbangClasspathJars =
-     hashbangJars.map(_.name).sorted.distinct // get jar basenames, remove duplicates
+     hashbangJars.map {
+       _.name
+     }.sorted.distinct // get jar basenames, remove duplicates
 <<< #4133 avoid braces to parens rewrite when overflow 2
 rewrite.rules = [RedundantBraces]
 ===
@@ -6836,7 +6836,9 @@ object a:
 >>>
 object a:
    val hashbangClasspathJars =
-     hashbangJars_map(_.name).sorted_distinct // get jar basenames, remove duplicates
+     hashbangJars_map {
+       _.name
+     }.sorted_distinct // get jar basenames, remove duplicates
 <<< redundant block within block, outer semicolon-terminated
 rewrite {
   rules = [RedundantBraces]