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

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 = {
+    def recursivePrint(elems: Seq[T], ancestors: Set[T], prefix: String, acc: String => Unit): Unit = {
-      val initPrefix = init(isLast) {
+      val unseenElems: Seq[T] = elems.filterNot(ancestors.contains)
-        case true => "   "
+      val unseenElemsLen = unseenElems.length
-        case false => "│  "
+      for ((elem, idx) <- unseenElems.iterator.zipWithIndex) {
-      }.mkString
+        val isLast = idx == unseenElemsLen - 1
+        val tee = if (isLast) "└─ " else "├─ "
+        acc(prefix + tee + show(elem))
 
-      val lastPrefix = last(isLast) {
+        val extraPrefix = if (isLast) "   " else "│  "
-        case true => "└─ "
+        recursivePrint(children(elem), ancestors + elem, prefix + extraPrefix, acc)
-        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() =>
       }
     }
 
-    helper(childrenWithLast(roots, Vector[Boolean]()), Set.empty)
+    val b = new ArrayBuffer[String]
-
+    recursivePrint(roots, Set(), "", b += _)
-    buffer
+    b.mkString("\n")
-      .dropRight(1) // drop last appended '\n'
-      .toString
   }
 
 }

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("p1", ArrayBuffer(
-      Node("c1"),
+      Node("c1", ArrayBuffer.empty),
-      Node("c2")),
+      Node("c2", ArrayBuffer.empty))),
-    Node("p2",
+    Node("p2", ArrayBuffer(
-      Node("c3"),
+      Node("c3", ArrayBuffer.empty),
-      Node("c4"))
+      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
+      assert(str ==
-        #│  ├─ c1
+        """├─ p1
-        #│  └─ c2
+          #│  ├─ c1
-        #└─ p2
+          #│  └─ c2
-        #   ├─ c3
+          #└─ p2
-        #   └─ c4""".stripMargin('#'))
+          #   ├─ 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('#'))
     }
   }
 }