From e418ebf3f981c43e717d8b6b6d25f1e0407704bb Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Wed, 25 Sep 2024 09:44:16 -0700 Subject: [PATCH 1/3] Test scala.js using implicit, brackets and comment --- .../src/test/resources/scalajs/DefDef.stat | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat b/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat index bc85c36b2b..690998b0bc 100644 --- a/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat +++ b/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat @@ -981,3 +981,59 @@ val template: Elems = List( deprValueMembers)) ) )) +<<< #4133 implicit with bracket and comment in type +maxColumn = 78 +newlines.source = keep +=== +override def flatten[B](implicit asTraversable: A => /*<:>> +Idempotency violated +=> Diff (- obtained, + expected) +-override def flatten[B]( +- implicit asTraversable: A => /*<: /*<: /*<:>> +Idempotency violated +=> Diff (- obtained, + expected) +-override def flatten[B]( +- implicit_asTraversable: A => /*<: /*<: hadComment_GenTraversableOnce[B]): Stream[B] = { + // foo +} +>>> +Idempotency violated +=> Diff (- obtained, + expected) +-override def flatten[B]( +- implicit_asTraversable: A => hadComment_GenTraversableOnce[ ++override def flatten[B](implicit_asTraversable: A => hadComment_GenTraversableOnce[ + B]): Stream[B] = { +<<< #4133 no implicit with bracket and no comment in type, no arrow +maxColumn = 78 +newlines.source = keep +=== +override def flatten[B](implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[B]): Stream[B] = { + // foo +} +>>> +Idempotency violated +=> Diff (- obtained, + expected) +-override def flatten[B]( +- implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[ ++override def flatten[B](implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[ + B]): Stream[B] = { From f2e607e3305727543b6058010477cff15f42dcf2 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:11:48 -0700 Subject: [PATCH 2/3] BestFirstSearch: use Left for non-optimal state --- .../scalafmt/internal/BestFirstSearch.scala | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala index 57bbb574ed..5c2fa1fd92 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala @@ -192,11 +192,16 @@ private class BestFirstSearch private (range: Set[Range])(implicit case Left(x) => return Left(killOnFail(opt, x)) } } - val furtherState = traverseSameLine(nextNextState) - if (null eq furtherState) Left(killOnFail(opt, nextNextState)) - else if (furtherState.appliedPenalty > nextNextState.appliedPenalty) - Left(nextNextState) - else Either.cond(!opt.recurseOnly, furtherState, furtherState) + def checkPenalty(state: State, orElse: => Either[State, State]) = + if (state.appliedPenalty > nextNextState.appliedPenalty) + Left(nextNextState) + else orElse + traverseSameLine(nextNextState) match { + case x @ Left(s) => + if (s eq null) Left(killOnFail(opt, nextNextState)) + else checkPenalty(s, x) + case x @ Right(s) => checkPenalty(s, if (opt.recurseOnly) Left(s) else x) + } } private def getActiveSplits(ft: FormatToken, state: State, maxCost: Int)( @@ -223,15 +228,15 @@ private class BestFirstSearch private (range: Set[Range])(implicit @tailrec private def traverseSameLine( state: State, - )(implicit queue: StateQueue): State = - if (state.depth >= tokens.length) state + )(implicit queue: StateQueue): Either[State, State] = + if (state.depth >= tokens.length) Right(state) else { val splitToken = tokens(state.depth) implicit val style: ScalafmtConfig = styleMap.at(splitToken) getActiveSplits(splitToken, state, Int.MaxValue) match { - case Seq() => null // dead end if empty + case Seq() => Left(null) // dead end if empty case Seq(split) => - if (split.isNL) state + if (split.isNL) Right(state) else { implicit val nextState: State = getNext(state, split, allAltAreNL = false) @@ -240,8 +245,8 @@ private class BestFirstSearch private (range: Set[Range])(implicit case ss if state.appliedPenalty == 0 && RightParenOrBracket(splitToken.right) => - traverseSameLineZeroCost(ss.filter(_.costWithPenalty == 0), state) - case _ => state + traverseSameLineZeroCost(ss, state) + case _ => Right(state) } } @@ -249,21 +254,23 @@ private class BestFirstSearch private (range: Set[Range])(implicit private def traverseSameLineZeroCost( splits: Seq[Split], state: State, - )(implicit style: ScalafmtConfig, queue: StateQueue): State = splits match { - case Seq(split) if !split.isNL => - val nextState: State = getNext(state, split, allAltAreNL = false) - if (nextState.split.costWithPenalty > 0) state - else if (nextState.depth >= tokens.length) nextState - else { - val nextToken = tokens(nextState.depth) - if (RightParenOrBracket(nextToken.right)) { - implicit val style: ScalafmtConfig = styleMap.at(nextToken) - val nextSplits = getActiveSplits(nextToken, nextState, maxCost = 0) - traverseSameLineZeroCost(nextSplits, nextState) - } else nextState - } - case _ => state - } + )(implicit style: ScalafmtConfig, queue: StateQueue): Either[State, State] = + splits.filter(_.costWithPenalty == 0) match { + case Seq(split) if !split.isNL => + val nextState: State = getNext(state, split, allAltAreNL = false) + if (nextState.split.costWithPenalty > 0) Right(state) + else if (nextState.depth >= tokens.length) Right(nextState) + else { + val nextToken = tokens(nextState.depth) + if (RightParenOrBracket(nextToken.right)) { + implicit val style: ScalafmtConfig = styleMap.at(nextToken) + val nextSplits = + getActiveSplits(nextToken, nextState, maxCost = Int.MaxValue) + traverseSameLineZeroCost(nextSplits, nextState) + } else Right(nextState) + } + case _ => Right(state) + } def getBestPath: SearchResult = { initStyle.runner.event(FormatEvent.Routes(routes)) From 9ac7084a501c9c59c2466736513f4074171270de Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:13:01 -0700 Subject: [PATCH 3/3] BestFirstSearch: optimal has low-penalty NL split --- .../scalafmt/internal/BestFirstSearch.scala | 22 +++++-- .../src/test/resources/default/String.stat | 6 +- .../src/test/resources/scalajs/DefDef.stat | 61 +++++++++---------- 3 files changed, 48 insertions(+), 41 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala index 5c2fa1fd92..8fd35b3027 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala @@ -223,6 +223,13 @@ private class BestFirstSearch private (range: Set[Range])(implicit splits.sortBy(_.costWithPenalty) } + private def stateAsOptimal(state: State, splits: Seq[Split]) = { + val isOptimal = splits.exists { s => + s.isNL && s.penalty < Constants.ShouldBeNewline + } || tokens.isRightCommentWithBreak(tokens(state.depth)) + if (isOptimal) Some(state) else None + } + /** Follow states having single active non-newline split */ @tailrec @@ -246,7 +253,7 @@ private class BestFirstSearch private (range: Set[Range])(implicit if state.appliedPenalty == 0 && RightParenOrBracket(splitToken.right) => traverseSameLineZeroCost(ss, state) - case _ => Right(state) + case ss => stateAsOptimal(state, ss).toRight(state) } } @@ -254,11 +261,13 @@ private class BestFirstSearch private (range: Set[Range])(implicit private def traverseSameLineZeroCost( splits: Seq[Split], state: State, - )(implicit style: ScalafmtConfig, queue: StateQueue): Either[State, State] = + nlState: => Option[State] = None, + )(implicit style: ScalafmtConfig, queue: StateQueue): Either[State, State] = { + def newNlState: Option[State] = stateAsOptimal(state, splits).orElse(nlState) splits.filter(_.costWithPenalty == 0) match { case Seq(split) if !split.isNL => val nextState: State = getNext(state, split, allAltAreNL = false) - if (nextState.split.costWithPenalty > 0) Right(state) + if (nextState.split.costWithPenalty > 0) newNlState.toRight(state) else if (nextState.depth >= tokens.length) Right(nextState) else { val nextToken = tokens(nextState.depth) @@ -266,11 +275,12 @@ private class BestFirstSearch private (range: Set[Range])(implicit implicit val style: ScalafmtConfig = styleMap.at(nextToken) val nextSplits = getActiveSplits(nextToken, nextState, maxCost = Int.MaxValue) - traverseSameLineZeroCost(nextSplits, nextState) - } else Right(nextState) + traverseSameLineZeroCost(nextSplits, nextState, newNlState) + } else newNlState.toRight(nextState) } - case _ => Right(state) + case _ => newNlState.toRight(state) } + } def getBestPath: SearchResult = { initStyle.runner.event(FormatEvent.Routes(routes)) diff --git a/scalafmt-tests/shared/src/test/resources/default/String.stat b/scalafmt-tests/shared/src/test/resources/default/String.stat index 76cb218999..e7673a52dd 100644 --- a/scalafmt-tests/shared/src/test/resources/default/String.stat +++ b/scalafmt-tests/shared/src/test/resources/default/String.stat @@ -504,9 +504,9 @@ object a { } >>> object a { - s"foo1 ${bar} foo2 ${baz(bar)} foo3 ${ - qux(baz, bar) - } foo4" + s"foo1 ${ + bar + } foo2 ${baz(bar)} foo3 ${qux(baz, bar)} foo4" s""" |foo1 ${bar} foo2 ${baz(bar)} foo3 ${ qux(baz, bar) diff --git a/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat b/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat index 690998b0bc..7a9a0837ab 100644 --- a/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat +++ b/scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat @@ -240,8 +240,8 @@ object a { >>> object a { object b { - def applyDynamicNamed(name: String)( - fields: (String, Any)*): Object with Dynamic = sys.error("stub") + def applyDynamicNamed(name: String)(fields: (String, + Any)*): Object with Dynamic = sys.error("stub") } } <<< weird breaking #252, oneline @@ -255,8 +255,8 @@ object a { >>> object a { object b { - def applyDynamicNamed(name: String)( - fields: (String, Any)*): Object with Dynamic = sys.error("stub") + def applyDynamicNamed(name: String)(fields: (String, + Any)*): Object with Dynamic = sys.error("stub") } } <<< toFunction8 #255 avoidInResultType, sometimesBeforeColonInMethodReturnType @@ -648,8 +648,9 @@ newlines.source = keep === @inline implicit def enrichAsScalaFromToIntFunction[T](jf: java.util.function.ToIntFunction[T]): RichToIntFunctionAsFunction1[T] = new RichToIntFunctionAsFunction1[T](jf) >>> -@inline implicit def enrichAsScalaFromToIntFunction[T](jf: java.util - .function.ToIntFunction[T]): RichToIntFunctionAsFunction1[T] = +@inline implicit def enrichAsScalaFromToIntFunction[T]( + jf: java.util.function.ToIntFunction[ + T]): RichToIntFunctionAsFunction1[T] = new RichToIntFunctionAsFunction1[T](jf) <<< #4133 overflow selects with apply-type 2 maxColumn = 70 @@ -658,8 +659,8 @@ newlines.source = keep def asScalaFromDoubleFunction[R](jf: java.util.function.DoubleFunction[R]): scala.Function1[java.lang.Double, R] >>> def asScalaFromDoubleFunction[R]( - jf: java.util.function.DoubleFunction[R]): scala.Function1[ - java.lang.Double, R] + jf: java.util.function.DoubleFunction[ + R]): scala.Function1[java.lang.Double, R] <<< #4133 complex nested applies with long infix args maxColumn = 80 newlines.source = keep @@ -989,12 +990,11 @@ override def flatten[B](implicit asTraversable: A => /*<:>> -Idempotency violated -=> Diff (- obtained, + expected) --override def flatten[B]( -- implicit asTraversable: A => /*<: /*<: /*<: /*<:>> -Idempotency violated -=> Diff (- obtained, + expected) --override def flatten[B]( -- implicit_asTraversable: A => /*<: /*<: /*<: hadComment_GenTraversableOn // foo } >>> -Idempotency violated -=> Diff (- obtained, + expected) --override def flatten[B]( -- implicit_asTraversable: A => hadComment_GenTraversableOnce[ -+override def flatten[B](implicit_asTraversable: A => hadComment_GenTraversableOnce[ - B]): Stream[B] = { +override def flatten[B]( + implicit_asTraversable: A => hadComment_GenTraversableOnce[ + B]): Stream[B] = { + // foo +} <<< #4133 no implicit with bracket and no comment in type, no arrow maxColumn = 78 newlines.source = keep @@ -1031,9 +1029,8 @@ override def flatten[B](implicit_asTraversable: A_aw_hadComment_GenTraversableOn // foo } >>> -Idempotency violated -=> Diff (- obtained, + expected) --override def flatten[B]( -- implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[ -+override def flatten[B](implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[ - B]): Stream[B] = { +override def flatten[B]( + implicit_asTraversable: A_aw_hadComment_GenTraversableOnce[ + B]): Stream[B] = { + // foo +}