From 9710e2df7e2674d1554ef59d54c34f6cbdc02605 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Wed, 15 May 2024 08:48:19 -0700 Subject: [PATCH 1/2] Add non-idempotent binpack.defnsite=oneline test --- .../src/test/resources/scalajs/DefDef.stat | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/scalafmt-tests/src/test/resources/scalajs/DefDef.stat b/scalafmt-tests/src/test/resources/scalajs/DefDef.stat index eca5ac4bd2..5261736e0a 100644 --- a/scalafmt-tests/src/test/resources/scalajs/DefDef.stat +++ b/scalafmt-tests/src/test/resources/scalajs/DefDef.stat @@ -541,7 +541,7 @@ newlines.source = keep binPack.preset = oneline danglingParentheses.preset = false newlines.configStyleDefnSite.prefer = true -# newlines.avoidForSimpleOverflow = [tooLong, punct, slc] +newlines.avoidForSimpleOverflow = [tooLong, punct, slc] === object a { def getArrayUnderlyingTypedArrayClassRef(elemTypeRef: NonArrayTypeRef)(implicit tracking: GlobalRefTracking, pos: Position): Option[WithGlobals[VarRef]] = { @@ -556,3 +556,25 @@ object a { // foo } } +<<< break at maxColumn, with overflow enabled 3 +preset = default +maxColumn = 80 +newlines.source = keep +binPack.preset = oneline +danglingParentheses.preset = false +newlines.configStyleDefnSite.prefer = true +newlines.avoidForSimpleOverflow = [tooLong, punct, slc] +=== +object a { + def foo = { + val registry = new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => ()) + } +} +>>> +Idempotency violated +=> Diff (- obtained, + expected) + val registry = +- new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => +- ()) ++ new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => ()) + } From b10c15a6739d4b0f44cabc354440b828252fa97d Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Wed, 15 May 2024 21:28:31 -0700 Subject: [PATCH 2/2] FormatOps: in CtrlBodySplits, add consistency Previously, the code was computing splits differently for tokens with a break and without one. Since we could potentially have no-break in the source but format with one, this could introduce non-idempotency. --- .../scala/org/scalafmt/internal/FormatOps.scala | 15 +++++++-------- .../src/test/resources/scalajs/DefDef.stat | 13 ++++++------- 2 files changed, 13 insertions(+), 15 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 5126c55d16..54be13ab0e 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 @@ -1644,21 +1644,20 @@ class FormatOps( classicNoBreakFunc: => Split, )(nlSplitFunc: Int => Split)(implicit style: ScalafmtConfig): Seq[Split] = checkComment(ft, nlSplitFunc) { x => + def getFolded(isKeep: Boolean) = + foldedNonComment(body, nlSplitFunc, isKeep = isKeep, spaceIndents) style.newlines.getBeforeMultiline match { + case Newlines.fold => getFolded(false) case Newlines.unfold => unfoldedNonComment(body, nlSplitFunc, spaceIndents, false) - case Newlines.classic | Newlines.keep if x.hasBreak => - Seq(nlSplitFunc(0).forThisLine) - case Newlines.classic => Option(classicNoBreakFunc).fold { - foldedNonComment(body, nlSplitFunc, isKeep = true, spaceIndents) - } { func => + case Newlines.classic if x.noBreak => + Option(classicNoBreakFunc).fold(getFolded(true)) { func => val spcSplit = func.forThisLine val nlSplit = nlSplitFunc(spcSplit.getCost(_ + 1, 0)).forThisLine Seq(spcSplit, nlSplit) } - case sh => // fold or keep without break - val isKeep = sh eq Newlines.keep - foldedNonComment(body, nlSplitFunc, isKeep, spaceIndents) + case Newlines.keep if x.noBreak => getFolded(true) + case _ => getFolded(true).filter(_.isNL) // keep/classic with break } } diff --git a/scalafmt-tests/src/test/resources/scalajs/DefDef.stat b/scalafmt-tests/src/test/resources/scalajs/DefDef.stat index 5261736e0a..4f26bfb72e 100644 --- a/scalafmt-tests/src/test/resources/scalajs/DefDef.stat +++ b/scalafmt-tests/src/test/resources/scalajs/DefDef.stat @@ -571,10 +571,9 @@ object a { } } >>> -Idempotency violated -=> Diff (- obtained, + expected) - val registry = -- new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => -- ()) -+ new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => ()) - } +object a { + def foo = { + val registry = + new js_FinalizationRegistry[js_Date, String, Any]((heldValue: String) => ()) + } +}