Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FormatOps: fix ForYield as CtrlBody #4406

Merged
merged 3 commits into from
Oct 5, 2024
Merged

FormatOps: fix ForYield as CtrlBody #4406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9466617
Test complex applies/lambda/for-yield with tuple
kitbellew Oct 2, 2024
496edd9
FormatTokens: add `getHeadAndLastIfEnclosed`
kitbellew Oct 5, 2024
f08881a
FormatOps: fix ForYield as CtrlBody
kitbellew Oct 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,6 @@ class FormatOps(
spaceIndents: Seq[Indent] = Seq.empty,
)(implicit style: ScalafmtConfig, ft: FormatToken): Seq[Split] = {
val btokens = body.tokens
def bheadFT = getHead(btokens, body)
val blastFT = getLastNonTrivial(btokens, body)
val blast = blastFT.left
val expire = nextNonCommentSameLine(blastFT).left
Expand Down Expand Up @@ -1522,10 +1521,11 @@ class FormatOps(
implicit fileLine: FileLine,
) = getSplits(getSpaceSplit(penalty, policy), nlCost)
def getSlbSplits(
end: T = expire,
exclude: TokenRanges = TokenRanges.empty,
policy: Policy = Policy.NoPolicy,
)(implicit fileLine: FileLine) = (
getSlbSplit(expire, exclude, policy),
getSlbSplit(end, exclude, policy),
getNlSplit(if (policy.isEmpty) 0 else 1),
)
def hasStateColumn = spaceIndents.exists(_.hasStateColumn)
Expand All @@ -1541,15 +1541,21 @@ class FormatOps(
if (hasStateColumn) getSplits(getSpaceSplit(1)) else getSlbSplits()
case _: Term.Block | _: Term.Match | _: Type.Match |
_: Term.NewAnonymous => getSplits(getSpaceSplit(1))
case t: Term.ForYield => nextNonComment(bheadFT).right match { // skipping `for`
case x @ LeftParenOrBrace() =>
val exclude = TokenRanges(TokenRange(x, matching(x).left))
(t.body match {
case b: Term.Block => getBracesIfEnclosed(b)
.map(x => getPolicySplits(1, getSlb(x._1.left, exclude)))
case _ => None
}).getOrElse(getSlbSplits(exclude))
case _ => getSlbSplits()
case t: Term.ForYield => getDelimsIfEnclosed(t.enumsBlock) match {
case Some((forEnumHead, forEnumLast)) =>
val exclude =
TokenRanges(TokenRange(forEnumHead.left, forEnumLast.left))
val afterYield = (t.body match {
case b: Term.Block => getHeadAndLastIfEnclosed(b)
.map { case (forBodyHead, forBodyLastOpt) =>
if (forBodyLastOpt.isDefined) forBodyHead
else prevNonCommentBefore(forBodyHead)
}
case b => tokenBeforeOpt(b)
}).getOrElse(getLast(t))
val end = endOfSingleLineBlock(afterYield)
getSlbSplits(end, exclude, penalize(1))
case None => getSlbSplits()
}
case ia: Member.Infix =>
val lia = findLeftInfix(ia)
Expand Down
20 changes: 15 additions & 5 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatTokens.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,26 @@ class FormatTokens(leftTok2tok: Map[TokenHash, Int])(val arr: Array[FormatToken]
case _ => false
}

def getDelimsIfEnclosed(
def getHeadAndLastIfEnclosed(
tokens: Tokens,
tree: Tree,
): Option[(FormatToken, FormatToken)] = getHeadOpt(tokens, tree)
.flatMap { head =>
matchingOpt(head.left).flatMap { other =>
): Option[(FormatToken, Option[FormatToken])] = getHeadOpt(tokens, tree)
.map { head =>
head -> matchingOpt(head.left).flatMap { other =>
val last = getLastNonTrivial(tokens, tree)
if (last eq other) Some((head, last)) else None
if (last eq other) Some(last) else None
}
}
def getHeadAndLastIfEnclosed(
tree: Tree,
): Option[(FormatToken, Option[FormatToken])] =
getHeadAndLastIfEnclosed(tree.tokens, tree)

def getDelimsIfEnclosed(
tokens: Tokens,
tree: Tree,
): Option[(FormatToken, FormatToken)] = getHeadAndLastIfEnclosed(tokens, tree)
.flatMap { case (head, lastOpt) => lastOpt.map(last => (head, last)) }
def getDelimsIfEnclosed(tree: Tree): Option[(FormatToken, FormatToken)] =
getDelimsIfEnclosed(tree.tokens, tree)

Expand Down
258 changes: 258 additions & 0 deletions scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9893,3 +9893,261 @@ object Multipart {
//
}
}
<<< #4133 complex nested applies/lambdas with for and tuple, narrow
private object MemoMap {
def make(implicit trace: Trace): UIO[MemoMap] =
Ref.Synchronized
.map { ref =>
new MemoMap { self =>
final def getOrElseMemoize[E, A, B](scope: Scope)(
layer: ZLayer[A, E, B]
): ZIO[A, E, ZEnvironment[B]] =
ref.modifyZIO { map =>
map.get(layer) match {
case None =>
for {
memoized = (
promise.await.onExit {
case Exit.Failure(_) => ZIO.unit
case Exit.Success(_) => observers.update(_ + 1)
},
(exit: Exit[Any, Any]) => finalizerRef.get.flatMap(_(exit))
)
} yield (resource, if (layer.isFresh) map else map.updated(layer, memoized))
}
}.flatten
}
}
}
>>> { stateVisits = 311, stateVisits2 = 250 }
private object MemoMap {
def make(implicit
trace: Trace
): UIO[MemoMap] =
Ref.Synchronized
.map { ref =>
new MemoMap { self =>
final def getOrElseMemoize[
E,
A,
B
](scope: Scope)(
layer: ZLayer[A, E, B]
): ZIO[A, E, ZEnvironment[
B
]] =
ref.modifyZIO { map =>
map.get(layer) match {
case None =>
for {
memoized = (
promise.await
.onExit {
case Exit
.Failure(
_
) =>
ZIO.unit
case Exit
.Success(
_
) =>
observers
.update(
_ + 1
)
},
(exit: Exit[
Any,
Any
]) =>
finalizerRef.get
.flatMap(
_(exit)
)
)
} yield (
resource,
if (layer.isFresh)
map
else
map.updated(
layer,
memoized
)
)
}
}.flatten
}
}
}
<<< #4133 complex nested applies/lambdas with for and tuple, wide and long
maxColumn = 80
===
object a {
private object MemoMap {
def make(implicit trace: Trace): UIO[MemoMap] =
Ref.Synchronized
.make[Map[ZLayer[Nothing, Any, Any], (IO[Any, Any], Exit[Any, Any] => UIO[Any])]](Map.empty)
.map { ref =>
new MemoMap { self =>
final def getOrElseMemoize[E, A, B](scope: Scope)(
layer: ZLayer[A, E, B]
): ZIO[A, E, ZEnvironment[B]] =
ref.modifyZIO { map =>
map.get(layer) match {
case Some((acquire, release)) =>
val cached: ZIO[Any, E, ZEnvironment[B]] = acquire
.asInstanceOf[IO[E, (FiberRefs.Patch, ZEnvironment[B])]]
.flatMap { case (patch, b) => ZIO.patchFiberRefs(patch).as(b) }
.onExit {
case Exit.Success(_) => scope.addFinalizerExit(release)
case Exit.Failure(_) => ZIO.unit
}

ZIO.succeed((cached, map))
case None =>
for {
observers <- Ref.make(0)
promise <- Promise.make[E, (FiberRefs.Patch, ZEnvironment[B])]
finalizerRef <- Ref.make[Exit[Any, Any] => UIO[Any]](_ => ZIO.unit)

resource = ZIO.uninterruptibleMask { restore =>
for {
a <- ZIO.environment[A]
outerScope = scope
innerScope <- Scope.make
tp <-
restore(
layer
.scope(innerScope)
.flatMap(_.apply(self).diffFiberRefs)
).exit.flatMap {
case e @ Exit.Failure(cause) =>
promise.failCause(cause) *> innerScope.close(e) *> ZIO
.failCause(cause)

case Exit.Success((patch, b)) =>
for {
_ <- finalizerRef.set { (e: Exit[Any, Any]) =>
ZIO.whenZIO(observers.modify(n => (n == 1, n - 1)))(
innerScope.close(e)
)
}
_ <- observers.update(_ + 1)
outerFinalizer <-
outerScope.addFinalizerExit(e => finalizerRef.get.flatMap(_.apply(e)))
_ <- promise.succeed((patch, b))
} yield b
}
} yield tp
}

memoized = (
promise.await.onExit {
case Exit.Failure(_) => ZIO.unit
case Exit.Success(_) => observers.update(_ + 1)
},
(exit: Exit[Any, Any]) => finalizerRef.get.flatMap(_(exit))
)
} yield (resource, if (layer.isFresh) map else map.updated(layer, memoized))

}
}.flatten
}
}
}
}
>>> { stateVisits = 1062, stateVisits2 = 943 }
object a {
private object MemoMap {
def make(implicit trace: Trace): UIO[MemoMap] =
Ref.Synchronized
.make[Map[
ZLayer[Nothing, Any, Any],
(IO[Any, Any], Exit[Any, Any] => UIO[Any])
]](Map.empty)
.map { ref =>
new MemoMap { self =>
final def getOrElseMemoize[E, A, B](scope: Scope)(
layer: ZLayer[A, E, B]
): ZIO[A, E, ZEnvironment[B]] =
ref.modifyZIO { map =>
map.get(layer) match {
case Some((acquire, release)) =>
val cached: ZIO[Any, E, ZEnvironment[B]] = acquire
.asInstanceOf[IO[E, (FiberRefs.Patch, ZEnvironment[B])]]
.flatMap { case (patch, b) =>
ZIO.patchFiberRefs(patch).as(b)
}
.onExit {
case Exit.Success(_) => scope.addFinalizerExit(release)
case Exit.Failure(_) => ZIO.unit
}

ZIO.succeed((cached, map))
case None =>
for {
observers <- Ref.make(0)
promise <- Promise
.make[E, (FiberRefs.Patch, ZEnvironment[B])]
finalizerRef <- Ref.make[Exit[Any, Any] => UIO[Any]](_ =>
ZIO.unit
)

resource = ZIO.uninterruptibleMask { restore =>
for {
a <- ZIO.environment[A]
outerScope = scope
innerScope <- Scope.make
tp <-
restore(
layer
.scope(innerScope)
.flatMap(_.apply(self).diffFiberRefs)
).exit.flatMap {
case e @ Exit.Failure(cause) =>
promise.failCause(cause) *> innerScope.close(
e
) *> ZIO
.failCause(cause)

case Exit.Success((patch, b)) =>
for {
_ <- finalizerRef.set { (e: Exit[Any, Any]) =>
ZIO.whenZIO(
observers.modify(n => (n == 1, n - 1))
)(
innerScope.close(e)
)
}
_ <- observers.update(_ + 1)
outerFinalizer <-
outerScope.addFinalizerExit(e =>
finalizerRef.get.flatMap(_.apply(e))
)
_ <- promise.succeed((patch, b))
} yield b
}
} yield tp
}

memoized = (
promise.await.onExit {
case Exit.Failure(_) => ZIO.unit
case Exit.Success(_) => observers.update(_ + 1)
},
(exit: Exit[Any, Any]) =>
finalizerRef.get.flatMap(_(exit))
)
} yield (
resource,
if (layer.isFresh) map else map.updated(layer, memoized)
)

}
}.flatten
}
}
}
}
Loading
Loading