From c131d9260deb0ab8b2c78db0d415825d6390f30a Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 27 Sep 2024 22:42:13 -0700 Subject: [PATCH 1/2] Test line-trailing infix after xml block --- .../resources/newlines/source_classic.stat | 30 +++++++++++++++ .../test/resources/newlines/source_fold.stat | 27 +++++++++++++ .../test/resources/newlines/source_keep.stat | 30 +++++++++++++++ .../resources/newlines/source_unfold.stat | 38 +++++++++++++++++++ 4 files changed, 125 insertions(+) diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat index 07a6bbb51b..855af52694 100644 --- a/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat +++ b/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat @@ -9632,3 +9632,33 @@ class UDFRegistration { ).toOption :: Nil } } +<<< #4133 infix after xml block +maxColumn = 80 +=== +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} +>>> +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar( + batch + ) ++ queued ++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat index a0418af38e..7ffd2bd256 100644 --- a/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat +++ b/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat @@ -9035,3 +9035,30 @@ class UDFRegistration { .toOption :: Nil } } +<<< #4133 infix after xml block +maxColumn = 80 +=== +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} +>>> +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ + queued ++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { Nil } + } + } +} 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 ef7b022397..b9a84b2ca5 100644 --- a/scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat +++ b/scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat @@ -9429,3 +9429,33 @@ class UDFRegistration { ).toOption :: Nil } } +<<< #4133 infix after xml block +maxColumn = 80 +=== +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} +>>> +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar( + batch + ) ++ queued ++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat index 4b36672c1a..1e0dc3f8b6 100644 --- a/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat +++ b/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat @@ -9710,3 +9710,41 @@ class UDFRegistration { .toOption :: Nil } } +<<< #4133 infix after xml block +maxColumn = 80 +=== +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +} +>>> +Idempotency violated +=> Diff (- obtained, + expected) + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { +- baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued +- ++ { +- if (firstFailureReason.nonEmpty) { +- // Waiting batches have not run yet, so must have no failure reasons. +- - +- } else { +- Nil ++ baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ ++ queued ++ ++ { ++ if (firstFailureReason.nonEmpty) { ++ // Waiting batches have not run yet, so must have no failure reasons. ++ - ++ } else { ++ Nil ++ } + } +- } + } From 25aeacd7a7c0b17e7cefd60870b45f19050bdebb Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:00:52 -0700 Subject: [PATCH 2/2] TreeOps: get owners via token range, not position This allows us to handle tokens which occupy no space, such as Xml.End or Xml.Start. --- .../scala/org/scalafmt/util/TreeOps.scala | 71 ++++++++++--------- .../resources/newlines/source_unfold.stat | 35 ++++----- 2 files changed, 50 insertions(+), 56 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala index ecf6124e0f..2ff81a60ee 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala @@ -860,34 +860,41 @@ object TreeOps { val allTokens = topSourceTree.tokens var prevParens: List[Token] = Nil - def treeAt(elemIdx: Int, elem: Tree, outerPrevLPs: Int): Int = { + def treeAt( + elemIdx: Int, + elem: Tree, + elemBeg: Token, + elemEnd: Token, + outerPrevLPs: Int, + ): Int = { if (TreeOps.isInfixApp(elem)) infixCount += 1 - val endPos = elem.pos.end - val allChildren: List[(Tree, Int)] = elem.children.flatMap { x => - val pos = x.pos - val startPos = pos.start - if (startPos == pos.end) None else Some((x, startPos)) - }.sortBy(_._2) + val treeBeg = elemBeg.start + val treeEnd = elemEnd.end + val allChildren: List[(Tree, Token, Token)] = { + for { + x <- elem.children + tokens = x.tokens if tokens.nonEmpty + beg = tokens.head if beg.start >= treeBeg // sometimes with implicit + end = tokens.last if end.end <= treeEnd + } yield (x, beg, end) + }.sortBy(_._2.start) allChildren match { case Nil => @tailrec - def tokenAt(idx: Int): Int = - if (idx == allTokens.length) idx - else { - val tok = allTokens(idx) - if (elem != topSourceTree && tok.start >= endPos) idx - else { - setOwner(tok, elem) - tokenAt(idx + 1) - } - } + def tokenAt(idx: Int): Int = { + val tok = allTokens(idx) + setOwner(tok, elem) + val nextIdx = idx + 1 + if (tok eq elemEnd) nextIdx else tokenAt(nextIdx) + } tokenAt(elemIdx) - case (firstChild, firstChildStart) :: rest => + case (firstChild, firstChildBeg, firstChildEnd) :: rest => var nextChild = firstChild - var nextChildStart = firstChildStart + var nextChildBeg = firstChildBeg + var nextChildEnd = firstChildEnd var children = rest var prevChild: Tree = null var prevLPs = outerPrevLPs @@ -895,23 +902,20 @@ object TreeOps { @tailrec def tokenAt(idx: Int): Int = - if (idx == allTokens.length) idx + if (idx > 0 && (elemEnd eq allTokens(idx - 1))) idx else { val tok = allTokens(idx) - val tokStart = tok.start - if (elem != topSourceTree && tokStart >= endPos) idx - else if (nextChild != null && tokStart >= nextChildStart) { + if (tok eq nextChildBeg) { if (prevChild != null) prevLPs = 0 prevChild = nextChild - val nextIdx = treeAt(idx, nextChild, prevLPs) + val nextIdx = treeAt(idx, nextChild, tok, nextChildEnd, prevLPs) children match { - case Nil => - nextChild = null - nextChildStart = endPos - case (head, start) :: rest => - nextChild = head + case Nil => nextChildBeg = null + case (head, beg, end) :: rest => children = rest - nextChildStart = start + nextChild = head + nextChildBeg = beg + nextChildEnd = end } prevComma = null tokenAt(nextIdx) @@ -921,8 +925,7 @@ object TreeOps { case _: Term.While | _: Term.ForClause => prevLPs == 1 && prevChild == firstChild // `expr` is first case _: Member.SyntaxValuesClause | _: Member.Tuple | - _: Term.Do | _: Term.AnonymousFunction => endPos == - tok.end + _: Term.Do | _: Term.AnonymousFunction => elemEnd eq tok case t: Init => prevChild ne t.tpe // include tpe case _: Ctor.Primary | _: Term.EnumeratorsBlock => true case _ => false @@ -944,7 +947,7 @@ object TreeOps { setOwner(tok, elem) } else { setOwner(tok, elem) - if (!tok.is[Trivia] && tokStart != tok.end) { + if (!tok.is[Trivia] && !tok.isEmpty) { prevComma = null prevChild = null if (tok.is[LeftParen]) { @@ -960,7 +963,7 @@ object TreeOps { } } - treeAt(0, topSourceTree, 0) + treeAt(0, topSourceTree, allTokens.head, allTokens.last, 0) val checkedNewlines = baseStyle.newlines.checkInfixConfig(infixCount) val initStyle = diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat index 1e0dc3f8b6..92f0435190 100644 --- a/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat +++ b/scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat @@ -9726,25 +9726,16 @@ object a { } } >>> -Idempotency violated -=> Diff (- obtained, + expected) - private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { -- baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ queued -- ++ { -- if (firstFailureReason.nonEmpty) { -- // Waiting batches have not run yet, so must have no failure reasons. -- - -- } else { -- Nil -+ baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ -+ queued -+ ++ { -+ if (firstFailureReason.nonEmpty) { -+ // Waiting batches have not run yet, so must have no failure reasons. -+ - -+ } else { -+ Nil -+ } - } -- } - } +object a { + private def waitingBatchRow(batch: BatchUIData): Seq[Node] = { + baseRow(batch) ++ createOutputOperationProgressBar(batch) ++ + queued ++ { + if (firstFailureReason.nonEmpty) { + // Waiting batches have not run yet, so must have no failure reasons. + - + } else { + Nil + } + } + } +}