From 07e985bffca5b796602d18d20dba8826f69fd0d7 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Thu, 19 Oct 2017 15:54:58 -0700 Subject: [PATCH] Fix cli print out completeness and cyclic handling issue (#671) * simplify * doc * rename * char * more tests * add tests then fix bugs * Small tweak to avoid quadratic calculation --- .../src/main/scala/coursier/util/Tree.scala | 76 ++++--------- .../test/scala/coursier/util/TreeTests.scala | 106 +++++++++++++++--- 2 files changed, 111 insertions(+), 71 deletions(-) diff --git a/core/shared/src/main/scala/coursier/util/Tree.scala b/core/shared/src/main/scala/coursier/util/Tree.scala index 5a12edb34..39c9ab692 100644 --- a/core/shared/src/main/scala/coursier/util/Tree.scala +++ b/core/shared/src/main/scala/coursier/util/Tree.scala @@ -1,73 +1,35 @@ package coursier.util -import scala.annotation.tailrec +import scala.collection.mutable.ArrayBuffer object Tree { def apply[T](roots: IndexedSeq[T])(children: T => Seq[T], show: T => String): String = { - val buffer = new StringBuilder - val printLine: (String) => Unit = { line => - buffer.append(line).append('\n') - } - - def last[E, O](seq: Seq[E])(f: E => O) = - seq.takeRight(1).map(f) - def init[E, O](seq: Seq[E])(f: E => O) = - seq.dropRight(1).map(f) - - /* - * Add elements to the stack - * @param elems elements to add - * @param isLast a list that contains whether an element is the last in its siblings or not. - */ - def childrenWithLast(elems: Seq[T], - isLast: Seq[Boolean]): Seq[(T, Seq[Boolean])] = { - - val isNotLast = isLast :+ false - - init(elems)(_ -> isNotLast) ++ - last(elems)(_ -> (isLast :+ true)) - } - /** - * Has to end with a "─" + * Recursively go down the resolution for the elems to construct the tree for print out. + * + * @param elems Seq of Elems that have been resolved + * @param ancestors a set of Elems to keep track for cycle detection + * @param prefix prefix for the print out + * @param acc accumulation method on a string */ - def showLine(isLast: Seq[Boolean]): String = { - val initPrefix = init(isLast) { - case true => " " - case false => "│ " - }.mkString + def recursivePrint(elems: Seq[T], ancestors: Set[T], prefix: String, acc: String => Unit): Unit = { + val unseenElems: Seq[T] = elems.filterNot(ancestors.contains) + val unseenElemsLen = unseenElems.length + for ((elem, idx) <- unseenElems.iterator.zipWithIndex) { + val isLast = idx == unseenElemsLen - 1 + val tee = if (isLast) "└─ " else "├─ " + acc(prefix + tee + show(elem)) - val lastPrefix = last(isLast) { - case true => "└─ " - case false => "├─ " - }.mkString - - initPrefix + lastPrefix - } - - // Depth-first traverse - @tailrec - def helper(stack: Seq[(T, Seq[Boolean])], seen: Set[T]): Unit = { - stack match { - case (elem, isLast) +: next => - printLine(showLine(isLast) + show(elem)) - - if (!seen(elem)) - helper(childrenWithLast(children(elem), isLast) ++ next, - seen + elem) - else - helper(next, seen) - case Seq() => + val extraPrefix = if (isLast) " " else "│ " + recursivePrint(children(elem), ancestors + elem, prefix + extraPrefix, acc) } } - helper(childrenWithLast(roots, Vector[Boolean]()), Set.empty) - - buffer - .dropRight(1) // drop last appended '\n' - .toString + val b = new ArrayBuffer[String] + recursivePrint(roots, Set(), "", b += _) + b.mkString("\n") } } diff --git a/tests/shared/src/test/scala/coursier/util/TreeTests.scala b/tests/shared/src/test/scala/coursier/util/TreeTests.scala index c41113850..71c895ebb 100644 --- a/tests/shared/src/test/scala/coursier/util/TreeTests.scala +++ b/tests/shared/src/test/scala/coursier/util/TreeTests.scala @@ -2,27 +2,105 @@ package coursier.util import utest._ +import scala.collection.mutable.ArrayBuffer + object TreeTests extends TestSuite { - case class Node(label: String, children: Node*) + + case class Node(label: String, children: ArrayBuffer[Node]) { + def addChild(x: Node): Unit = { + children.append(x) + } + + // The default behavior of hashcode will calculate things recursively, + // which will be infinite because we want to test cycles, so hardcoding + // the hashcode to 0 to get around the issue. + // TODO: make the hashcode to return something more interesting to + // improve performance. + override def hashCode(): Int = 0 + } val roots = Array( - Node("p1", - Node("c1"), - Node("c2")), - Node("p2", - Node("c3"), - Node("c4")) + Node("p1", ArrayBuffer( + Node("c1", ArrayBuffer.empty), + Node("c2", ArrayBuffer.empty))), + Node("p2", ArrayBuffer( + Node("c3", ArrayBuffer.empty), + Node("c4", ArrayBuffer.empty))) ) + val moreNestedRoots = Array( + Node("p1", ArrayBuffer( + Node("c1", ArrayBuffer( + Node("p2", ArrayBuffer.empty))))), + Node("p3", ArrayBuffer( + Node("d1", ArrayBuffer.empty)) + )) + + + // Constructing cyclic graph: + // a -> b -> c -> a + // -> e -> f + + val a = Node("a", ArrayBuffer.empty) + val b = Node("b", ArrayBuffer.empty) + val c = Node("c", ArrayBuffer.empty) + val e = Node("e", ArrayBuffer.empty) + val f = Node("f", ArrayBuffer.empty) + + a.addChild(b) + b.addChild(c) + c.addChild(a) + c.addChild(e) + e.addChild(f) + + val tests = TestSuite { - 'apply { + 'basic { val str = Tree[Node](roots)(_.children, _.label) - assert(str == """├─ p1 - #│ ├─ c1 - #│ └─ c2 - #└─ p2 - # ├─ c3 - # └─ c4""".stripMargin('#')) + assert(str == + """├─ p1 + #│ ├─ c1 + #│ └─ c2 + #└─ p2 + # ├─ c3 + # └─ c4""".stripMargin('#')) + } + + 'moreNested { + val str = Tree[Node](moreNestedRoots)(_.children, _.label) + assert(str == + """├─ p1 + #│ └─ c1 + #│ └─ p2 + #└─ p3 + # └─ d1""".stripMargin('#')) + } + + 'cyclic1 { + val str: String = Tree[Node](Array(a, e))(_.children, _.label) + assert(str == + """├─ a + #│ └─ b + #│ └─ c + #│ └─ e + #│ └─ f + #└─ e + # └─ f""".stripMargin('#')) + } + + 'cyclic2 { + val str: String = Tree[Node](Array(a, c))(_.children, _.label) + assert(str == + """├─ a + #│ └─ b + #│ └─ c + #│ └─ e + #│ └─ f + #└─ c + # ├─ a + # │ └─ b + # └─ e + # └─ f""".stripMargin('#')) } } }