From 2966987cf64e51428edb029a52d5beb485d23908 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 21 Feb 2020 01:04:08 -0800 Subject: [PATCH 1/3] Add the failing tests from issue #1720 --- .../resources/align/DefaultWithAlign.stat | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat b/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat index 05532a69e2..276bd2596f 100644 --- a/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat +++ b/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat @@ -483,3 +483,58 @@ object x { // comment println("foo") // comment } +<<< #1720 original, two SL comments +type iovec = CStruct2[Ptr[Byte], // iov_base + CSize] // iov_len +>>> +Unable to format file due to bug in scalafmt +<<< #1720 with val +val iovec = CStruct2[Ptr[Byte], // iov_base + CSize](a) // iov_len +>>> +Unable to format file due to bug in scalafmt +<<< #1720 no align +align = none +=== +type iovec = CStruct2[Ptr[Byte], // iov_base + CSize] // iov_len +>>> +Unable to format file due to bug in scalafmt +<<< #1720 second SL comment +type iovec = CStruct2[Ptr[Byte], + CSize] // iov_len +>>> +type iovec = CStruct2[Ptr[Byte], CSize] // iov_len +<<< #1720 first SL comment +type iovec = CStruct2[Ptr[Byte], // iov_base + CSize] +>>> +Unable to format file due to bug in scalafmt +<<< #1720 nested multi-arg type with two SL comments +type iovec = CStruct2[Ptr[Byte, // iov_base + CSize]] // iov_len +>>> +type iovec = CStruct2[Ptr[ + Byte, // iov_base + CSize +]] // iov_len +<<< #1720 one arg with SL comment +type iovec = CStruct2[Ptr[Byte] // iov_base + ] +>>> +type iovec = CStruct2[ + Ptr[Byte] // iov_base +] +<<< #1720 first inline comment +type iovec = CStruct2[Ptr[Byte], /* iov_base */ + CSize] +>>> +Unable to format file due to bug in scalafmt +<<< #1720 first inline comment with apply +val iovec = CStruct2(Ptr(Byte), /* iov_base */ + CSize) +>>> +val iovec = CStruct2( + Ptr(Byte), /* iov_base */ + CSize +) From a802dc2578da45d34d9712803b0cc0498ad5f657 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sat, 22 Feb 2020 12:54:15 -0800 Subject: [PATCH 2/3] BestFirstSearch: refactor tracking slow states --- .../scalafmt/internal/BestFirstSearch.scala | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 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 f28320ae69..d4d0764461 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 @@ -45,10 +45,9 @@ private class BestFirstSearch private ( val noOptimizations = noOptimizationZones(tree) var explored = 0 var deepestYet = State.start - var statementCount = 0 val best = mutable.Map.empty[Int, State] - var pathologicalEscapes = 0 val visits = new Array[Int](tokens.length) + val keepSlowStates = !pruneSlowStates type StateHash = Long @@ -60,17 +59,11 @@ private class BestFirstSearch private ( /** * Returns true if it's OK to skip over state. */ - def shouldEnterState(curr: State): Boolean = { - val splitToken = tokens(curr.depth) - val insideOptimizationZone = - curr.policy.noDequeue || isInsideNoOptZone(splitToken) - def hasBestSolution = !pruneSlowStates || insideOptimizationZone || { + def shouldEnterState(curr: State): Boolean = + keepSlowStates || curr.policy.noDequeue || + isInsideNoOptZone(tokens(curr.depth)) || // TODO(olafur) document why/how this optimization works. - val result = !best.get(curr.depth).exists(_.alwaysBetter(curr)) - result - } - hasBestSolution - } + !best.get(curr.depth).exists(_.alwaysBetter(curr)) def shouldRecurseOnBlock( ft: FormatToken, @@ -208,7 +201,7 @@ private class BestFirstSearch private ( var optimalNotFound = true actualSplit.foreach { split => val nextState = curr.next(style, split, splitToken) - if (depth == 0 && split.modification.isNewline && + if (!keepSlowStates && depth == 0 && split.modification.isNewline && !best.contains(curr.depth)) { best.update(curr.depth, nextState) } From 92991f17bab382689e89ad6e99f48243373b9448 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sat, 22 Feb 2020 14:52:10 -0800 Subject: [PATCH 3/3] BestFirstSearch: if failed, retry no optimization Fixes #1720. --- .../scalafmt/internal/BestFirstSearch.scala | 13 ++++++++-- .../resources/align/DefaultWithAlign.stat | 25 +++++++++++++++---- 2 files changed, 31 insertions(+), 7 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 d4d0764461..85bced1ad0 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 @@ -47,7 +47,7 @@ private class BestFirstSearch private ( var deepestYet = State.start val best = mutable.Map.empty[Int, State] val visits = new Array[Int](tokens.length) - val keepSlowStates = !pruneSlowStates + var keepSlowStates = !pruneSlowStates type StateHash = Long @@ -247,7 +247,16 @@ private class BestFirstSearch private ( } def getBestPath: SearchResult = { - val state = shortestPath(State.start, tree.tokens.last) + val state = { + def run = shortestPath(State.start, tree.tokens.last) + val state = run + if (null != state) state + else { + best.clear() + keepSlowStates = true + run + } + } if (null != state) { runner.event(CompleteFormat(explored, state)) SearchResult(state, reachedEOF = true) diff --git a/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat b/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat index 276bd2596f..09e2c3441d 100644 --- a/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat +++ b/scalafmt-tests/src/test/resources/align/DefaultWithAlign.stat @@ -487,19 +487,28 @@ object x { type iovec = CStruct2[Ptr[Byte], // iov_base CSize] // iov_len >>> -Unable to format file due to bug in scalafmt +type iovec = CStruct2[ + Ptr[Byte], // iov_base + CSize +] // iov_len <<< #1720 with val val iovec = CStruct2[Ptr[Byte], // iov_base CSize](a) // iov_len >>> -Unable to format file due to bug in scalafmt +val iovec = CStruct2[ + Ptr[Byte], // iov_base + CSize +](a) // iov_len <<< #1720 no align align = none === type iovec = CStruct2[Ptr[Byte], // iov_base CSize] // iov_len >>> -Unable to format file due to bug in scalafmt +type iovec = CStruct2[ + Ptr[Byte], // iov_base + CSize +] // iov_len <<< #1720 second SL comment type iovec = CStruct2[Ptr[Byte], CSize] // iov_len @@ -509,7 +518,10 @@ type iovec = CStruct2[Ptr[Byte], CSize] // iov_len type iovec = CStruct2[Ptr[Byte], // iov_base CSize] >>> -Unable to format file due to bug in scalafmt +type iovec = CStruct2[ + Ptr[Byte], // iov_base + CSize +] <<< #1720 nested multi-arg type with two SL comments type iovec = CStruct2[Ptr[Byte, // iov_base CSize]] // iov_len @@ -529,7 +541,10 @@ type iovec = CStruct2[ type iovec = CStruct2[Ptr[Byte], /* iov_base */ CSize] >>> -Unable to format file due to bug in scalafmt +type iovec = CStruct2[ + Ptr[Byte], /* iov_base */ + CSize +] <<< #1720 first inline comment with apply val iovec = CStruct2(Ptr(Byte), /* iov_base */ CSize)