From dccaf4de7860e90c548a3f77853cf43a3d34618b Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:11:59 -0700 Subject: [PATCH] Route: hide NL before dot for keep via a splittag Revises #4021 and #4214. --- .../org/scalafmt/internal/FormatOps.scala | 25 ++++--- .../scala/org/scalafmt/internal/Router.scala | 5 +- .../org/scalafmt/internal/SplitTag.scala | 1 + .../test/resources/newlines/source_keep.stat | 9 ++- .../resources/scala3/OptionalBraces_keep.stat | 74 ++++++++----------- .../src/test/resources/scalajs/Advanced.stat | 10 +-- .../src/test/resources/scalajs/DefDef.stat | 31 ++++---- 7 files changed, 75 insertions(+), 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 index 51f00ae422..01c8b7c657 100644 --- 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 index f134f1eacf..f61e6ab7a0 100644 --- 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 index e3d67f155c..18861d2075 100644 --- 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 index 46a5a605b0..359c76204b 100644 --- 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 index eebf56ae87..bee588d79a 100644 --- 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 index 4d97c8e982..0bf1c0eac8 100644 --- 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 index 7a9a0837ab..474fedce11 100644 --- 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)) )) ) ))