Route: hide NL before dot for keep via a splittag

Revises #4021 and #4214.
scalameta · Sep 27, 2024 · dccaf4d · dccaf4d
1 parent 4df3ad8
commit dccaf4d
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 80 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
@@ -2940,16 +2940,21 @@ class FormatOps(
 
       (afterDelims.right match {
         case c: T.Dot => // check if Dot rule includes a break option
-          c -> GetSelectLike.onRightOpt(afterDelims).exists { x =>
+          c -> GetSelectLike.onRightOpt(afterDelims).map { x =>
             implicit val cfg = styleMap.at(afterDelims)
             cfg.newlines.getSelectChains match {
               case Newlines.classic =>
                 val (expireTree, nextSelect) =
                   findLastApplyAndNextSelect(x.tree, cfg.encloseSelectChains)
-                canStartSelectChain(x, nextSelect, expireTree)
-              case _ => true
+                Right(canStartSelectChain(x, nextSelect, expireTree))
+              case Newlines.keep => Left(Policy.on(c, "BP1L.NL") {
+                  case Decision(`afterDelims`, ss) => Decision
+                      .onlyNewlineSplits(ss)
+                      .map(_.preActivateFor(SplitTag.SelectChainBinPackNL))
+                })
+              case _ => Right(true)
             }
-          }
+          }.getOrElse(Right(false))
         case x @ LeftParenOrBracket() =>
           nextNonCommentSameLineAfter(afterDelims).right match {
             case _: T.Comment => null
@@ -2959,15 +2964,17 @@ class FormatOps(
                 cfg.configStyleCallSite.prefer &&
                 cfg.danglingParentheses.atCallSite(afterDelims.meta.rightOwner)
               } || next(afterDelims).hasBreak
-              c -> ok
+              c -> Right(ok)
           }
         case _ => null
       }) match {
         case null => (None, NoPolicy)
-        case (c, okToBreak) =>
-          val policyOpt =
-            if (!okToBreak) closeBreakPolicy()
-            else Some(decideNewlinesOnlyBeforeToken(c))
+        case (c, policyOrOkToBreak) =>
+          val policyOpt = policyOrOkToBreak match {
+            case Left(policy) => Some(policy)
+            case Right(okToBreak) if !okToBreak => closeBreakPolicy()
+            case _ => Some(decideNewlinesOnlyBeforeToken(c))
+          }
           (Some(c), policyOpt.fold(Policy.noPolicy)(policyWithDelay))
       }
     }

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
@@ -1780,9 +1780,8 @@ class Router(formatOps: FormatOps) {
                 ignore = next(ft).right.is[T.Comment],
               ))
             else Seq(
-              Split(NoSplit, 0)
-                .withSingleLine(getSlbEnd(), killOnFail = shouldKillOnFail()),
-              Split(Newline, Constants.ExceedColumnPenalty * 3),
+              Split(NoSplit, 0),
+              Split(Newline, 1).onlyFor(SplitTag.SelectChainBinPackNL),
             )
 
           case Newlines.unfold =>

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala
@@ -12,6 +12,7 @@ object SplitTag {
   case object OneArgPerLine extends SplitTag
   case object SelectChainFirstNL extends SplitTag
   case object SelectChainSecondNL extends SplitTag
+  case object SelectChainBinPackNL extends SplitTag
   case object InfixChainNoNL extends SplitTag
   case object OnelineWithChain extends SplitTag
   case object VerticalMultilineSingleLine extends SplitTag

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat
@@ -945,8 +945,9 @@ 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) { ... }'")
 }
@@ -8957,8 +8958,8 @@ object a {
 >>>
 object a {
   if (foo) bar match {
-    case nme.IMPORT | nme.OUTER | nme.ANON_CLASS_NAME | nme
-          .ANON_FUN_NAME | nme.CONSTRUCTOR => ()
+    case nme.IMPORT | nme.OUTER | nme.ANON_CLASS_NAME | nme.ANON_FUN_NAME | nme.CONSTRUCTOR =>
+      ()
   }
 }
 <<< #4133 control body with complex apply/select/try 1

diff --git a/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_keep.stat
@@ -6700,8 +6700,7 @@ object a:
                     errorButContinue(
                       em"""The start of this line does not match any of the previous indentation widths.
                           |Indentation width of current line : $nextWidth
-                          |This falls between previous widths: ${ir
-                          .width} and $lw"""
+                          |This falls between previous widths: ${ir.width} and $lw"""
                     ))
 <<< scala.js tucked close paren after optional braces region, !dangling
 binPack.preset = always
@@ -6742,8 +6741,7 @@ object a:
                     errorButContinue(
                       em"""The start of this line does not match any of the previous indentation widths.
                           |Indentation width of current line : $nextWidth
-                          |This falls between previous widths: ${ir
-                          .width} and $lw"""))
+                          |This falls between previous widths: ${ir.width} and $lw"""))
 <<< scala.js overflow within for-yield dangling
 binPack.preset = always
 danglingParentheses.preset = true
@@ -6757,10 +6755,8 @@ object a:
    def foo =
      for template <- templates do
         val current_Map_String__Object =
-          template_templateFile_settings_getOrElse(
-            "page",
-            Map.empty
-          ).asInstanceOf[Map_String_Object]
+          template_templateFile_settings_getOrElse("page",
+            Map.empty).asInstanceOf[Map_String_Object]
 <<< scala.js overflow within for-yield !dangling
 binPack.preset = always
 danglingParentheses.preset = false
@@ -7047,8 +7043,9 @@ object a {
 >>>
 object a {
   @threadUnsafe lazy val AnnotationRetentionSourceAttr: TermSymbol =
-    requiredClass("java.lang.annotation.RetentionPolicy").linkedClass
-      .requiredValue("SOURCE")
+    requiredClass(
+        "java.lang.annotation.RetentionPolicy").linkedClass.requiredValue(
+        "SOURCE")
 }
 <<< #4133 overflow apply with binpack, dangling
 maxColumn = 70
@@ -7079,15 +7076,9 @@ object a:
 object a:
    if (args.isEmpty) {
      run(insertClasspathInArgs(args1,
-         List(
-           dottyCompiler,
-           dottyInterfaces,
-           asm,
-           dottyStaging,
-           dottyTastyInspector,
-           tastyCore,
-           compilerInterface
-         ).mkString(File.pathSeparator)))
+         List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
+           dottyTastyInspector, tastyCore,
+           compilerInterface).mkString(File.pathSeparator)))
    } else run(args)
 <<< #4133 overflow apply/selects with binpack, dangling
 maxColumn = 70
@@ -7103,15 +7094,11 @@ object a:
    if (args.isEmpty) {
      run(insertClasspathInArgs(
          args1,
-         List(
-           dottyCompiler,
-           dottyInterfaces,
-           asm,
-           dottyStaging,
-           dottyTastyInspector,
-           tastyCore,
-           compilerInterface
-         ).mkString(File.pathSeparator).mkString(File.pathSeparator)
+         List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
+           dottyTastyInspector, tastyCore,
+           compilerInterface).mkString(File.pathSeparator).mkString(
+           File.pathSeparator
+         )
        ))
    } else run(args)
 <<< #4133 overflow apply with binpack, !dangling
@@ -7144,8 +7131,8 @@ object a:
    if (args.isEmpty) {
      run(insertClasspathInArgs(args1,
          List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
-           dottyTastyInspector, tastyCore, compilerInterface).mkString(
-           File.pathSeparator)))
+           dottyTastyInspector, tastyCore,
+           compilerInterface).mkString(File.pathSeparator)))
    } else run(args)
 <<< #4133 overflow apply/selects with binpack, !dangling
 maxColumn = 70
@@ -7163,8 +7150,9 @@ object a:
        insertClasspathInArgs(
          args1,
          List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
-           dottyTastyInspector, tastyCore, compilerInterface)
-           .mkString(File.pathSeparator).mkString(File.pathSeparator)
+           dottyTastyInspector, tastyCore,
+           compilerInterface).mkString(File.pathSeparator).mkString(
+           File.pathSeparator)
        ))
    } else run(args)
 <<< #4133 overflow select with binpack, dangling
@@ -7178,8 +7166,9 @@ object a:
 >>>
 object a:
    val foo = map { postFile =>
-      val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(year)
-        .resolve(month).resolve(day).resolve(name)
+      val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(
+        year
+      ).resolve(month).resolve(day).resolve(name)
    }
 <<< #4133 overflow select with binpack, !dangling
 binPack.preset = always
@@ -7192,8 +7181,8 @@ object a:
 >>>
 object a:
    val foo = map { postFile =>
-      val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(year)
-        .resolve(month).resolve(day).resolve(name)
+      val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(
+        year).resolve(month).resolve(day).resolve(name)
    }
 <<< #4133 overflow selects/applies, nobreak-dot-break, binpack + !dangling
 maxColumn = 70
@@ -7228,13 +7217,12 @@ object a:
     )
 >>>
 object a:
-   lazy val `stdlib-bootstrapped` =
-     project.in(file("stdlib-bootstrapped")).withCommonSettings(
-       Bootstrapped).dependsOn(dottyCompiler(
-       Bootstrapped) % "provided; compile->runtime; test->test")
-       .settings(
-         moduleName := "scala-library"
-       )
+   lazy val `stdlib-bootstrapped` = project.in(
+     file("stdlib-bootstrapped")).withCommonSettings(
+     Bootstrapped).dependsOn(dottyCompiler(
+     Bootstrapped) % "provided; compile->runtime; test->test").settings(
+     moduleName := "scala-library"
+   )
 <<< #4133 overflow selects/applies, break-dot-nobreak, binpack + !dangling
 maxColumn = 70
 binPack.preset = always

diff --git a/scalafmt-tests/shared/src/test/resources/scalajs/Advanced.stat b/scalafmt-tests/shared/src/test/resources/scalajs/Advanced.stat
@@ -27,9 +27,9 @@ newlines.source = keep
 ===
 debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1.map(_.tpe) + ", pt = " + pt + ", lobounds = " + tparams.map(_.tpe.lowerBound) + ", parambounds = " + tparams.map(_.info)) //debug
 >>>
-debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1
-  .map(_.tpe) + ", pt = " + pt + ", lobounds = " + tparams
-  .map(_.tpe.lowerBound) + ", parambounds = " + tparams.map(
+debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1.map(
+    _.tpe) + ", pt = " + pt + ", lobounds = " + tparams.map(
+    _.tpe.lowerBound) + ", parambounds = " + tparams.map(
     _.info)) //debug
 <<< overflow infix with applies
 maxColumn = 74
@@ -38,5 +38,5 @@ newlines.source = keep
 realeasy && isJavaAtLeast(9) && !perFile.exists(releaseFlag.matches) && toolArgsFor(sources)(ToolName.javaVersion).isEmpty
 >>>
 realeasy && isJavaAtLeast(9) && !perFile.exists(
-    releaseFlag.matches) && toolArgsFor(sources)(ToolName.javaVersion)
-  .isEmpty
+    releaseFlag.matches) && toolArgsFor(sources)(
+    ToolName.javaVersion).isEmpty
diff --git a/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat b/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat
@@ -820,9 +820,9 @@ val memberSel: Elems =
                             Ol(
                                 elems = {
                                   if (
-                                      !universe.settings.docGroups.value || tpl
-                                        .members.map(_.group).distinct.forall(
-                                            _ == ModelFactory.defaultGroup)
+                                      !universe.settings.docGroups.value || tpl.members.map(
+                                          _.group).distinct.forall(
+                                          _ == ModelFactory.defaultGroup)
                                   )
                                     NoElems
                                   else
@@ -834,8 +834,7 @@ val memberSel: Elems =
                                       elems = Span(
                                           elems = Txt("Alphabetic"))) :: {
                                     if (
-                                        tpl.linearizationTemplates
-                                          .isEmpty && tpl.conversions.isEmpty
+                                        tpl.linearizationTemplates.isEmpty && tpl.conversions.isEmpty
                                     )
                                       NoElems
                                     else
@@ -847,8 +846,7 @@ val memberSel: Elems =
                             )
                     ) ++ (
                         if (
-                            tpl.linearizationTemplates.isEmpty && tpl
-                              .conversions.isEmpty
+                            tpl.linearizationTemplates.isEmpty && tpl.conversions.isEmpty
                         ) NoElems
                         else {
                           (if (tpl.linearizationTemplates.isEmpty) NoElems
@@ -886,13 +884,15 @@ val memberSel: Elems =
                                              tpl.conversions.map { conv =>
                                                val name =
                                                  conv.conversionQualifiedName
-                                               val hide = universe.settings
-                                                 .hiddenImplicits(name)
+                                               val hide =
+                                                 universe.settings.hiddenImplicits(
+                                                     name)
                                                Li(`class` = "in", name = name,
-                                                   `data-hidden` = hide.toString,
+                                                   `data-hidden` =
+                                                     hide.toString,
                                                    elems = Span(
-                                                       elems = Txt("by " + conv
-                                                         .conversionShortName)))
+                                                       elems = Txt(
+                                                           "by " + conv.conversionShortName)))
                                              }
                                            }
                                        )
@@ -930,10 +930,9 @@ val memberSel: Elems =
                                                   elems = Txt("Protected")))
                                       ) ++ List(Li(`class` = "private out",
                                               elems = Span(
-                                                  elems = Txt("Private"))))
-                                        .filter(_ =>
-                                          universe.settings.visibilityPrivate
-                                            .value))
+                                                  elems = Txt(
+                                                      "Private")))).filter(_ =>
+                                        universe.settings.visibilityPrivate.value))
                         ))
               )
       ))