Tests appear to pass using the new `Hierarchy`-first `Discovered` data format

5 files changed, 155 insertions, 150 deletions

diff --git a/core/src/main/scala/mill/Main.scala b/core/src/main/scala/mill/Main.scala
index 22f84d32..857242d6 100644
--- a/core/src/main/scala/mill/Main.scala
+++ b/core/src/main/scala/mill/Main.scala
@@ -1,18 +1,16 @@
 package mill
 
-import ammonite.Main
 import ammonite.main.Scripts
 import ammonite.ops._
-import ammonite.util.{Colors, Name, Res}
+import ammonite.util.Res
 import mill.define.Task
 import mill.discover._
 import mill.eval.Evaluator
 import mill.util.OSet
 
-
-import scala.annotation.tailrec
 import ammonite.main.Scripts.pathScoptRead
 import ammonite.repl.Repl
+import mill.discover.Router.EntryPoint
 object Main {
   def timed[T](t: => T) = {
     val startTime = System.currentTimeMillis()
@@ -32,17 +30,29 @@ object Main {
       val mapping = Discovered.mapping(obj)(discovered)
       val workspacePath = pwd / 'out
 
-      val mainRoutes = discovered.mains.map(x => (x.path :+ x.entryPoint.name, x: Info[T, _]))
-      val targetRoutes = discovered.targets.map(x => (x.path, x: Info[T, _]))
-      val routeList: Seq[(Seq[String], Info[T, _])] = mainRoutes ++ targetRoutes
-      val routeMap = routeList.toMap
-      routeMap.get(selector) match{
-        case Some(info) =>
-          val target = getTarget(obj, info, rest.toList)
+      def resolve[V](selector: List[String], hierarchy: Hierarchy[T, V]): Option[Task[Any]] = {
+        selector match{
+          case last :: Nil =>
+
+            def target: Option[Task[Any]] =
+              hierarchy.targets.find(_.label == last).map(_.run(hierarchy.node(obj)))
+            def command: Option[Task[Any]] = hierarchy.commands.find(_.name == last).flatMap(
+              _.invoke(hierarchy.node(obj), ammonite.main.Scripts.groupArgs(rest.toList)) match{
+                case Router.Result.Success(v) => Some(v)
+                case _ => None
+              }
+            )
+            target orElse command
+          case head :: tail =>
+            hierarchy.children
+              .collectFirst{ case (label, child) if label == head => resolve(tail, child) }
+              .flatten
+        }
+      }
+      resolve(selector.toList, discovered.hierarchy) match{
+        case Some(target) =>
           val evaluator = new Evaluator(workspacePath, mapping)
           val evaluated = evaluator.evaluate(OSet(target))
-
-
           evaluated.transitive.foreach{
             case t: define.Source => watch(t.handle.path)
             case _ => // do nothing
@@ -53,19 +63,6 @@ object Main {
     }
   }
 
-  def getTarget[T](obj: T, info: Info[T, _], args: List[String]) = info match{
-    case nestedEntryPoint: CommandInfo[T, _] =>
-      nestedEntryPoint.invoke(
-        obj,
-        ammonite.main.Scripts.groupArgs(args)
-      ) match{
-        case error: Router.Result.Error =>
-          throw new Exception("Failed to evaluate main method: " + error)
-        case mill.discover.Router.Result.Success(target) => target
-      }
-    case labelled: LabelInfo[T, _] => labelled.run(obj)
-  }
-
 
   def main(args: Array[String]): Unit = timed{
     case class Config(home: ammonite.ops.Path = pwd/'out/'ammonite,
diff --git a/core/src/main/scala/mill/define/Task.scala b/core/src/main/scala/mill/define/Task.scala
index 0853b06b..c256d3db 100644
--- a/core/src/main/scala/mill/define/Task.scala
+++ b/core/src/main/scala/mill/define/Task.scala
@@ -1,8 +1,10 @@
 package mill.define
 
 import mill.define.Applicative.Applyable
+import mill.define.Task.targetImpl
 import mill.eval.PathRef
 import mill.util.Args
+
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox.Context
 
@@ -25,6 +27,9 @@ abstract class Task[+T] extends Task.Ops[T] with Applyable[T]{
 }
 
 trait Target[+T] extends Task[T]
+object Target{
+  implicit def apply[T](t: T): Target[T] = macro targetImpl[T]
+}
 class TargetImpl[+T](t: Task[T], enclosing: String) extends Target[T] {
   val inputs = Seq(t)
   def evaluate(args: Args) = args[T](0)
@@ -45,7 +50,7 @@ class Source(path: ammonite.ops.Path) extends Task[PathRef]{
 }
 
 object Task extends Applicative.Applyer[Task, Task, Args]{
-
+  def apply[T](t: T): Target[T] = macro targetImpl[T]
   def underlying[A](v: Task[A]) = v
 
   def source(path: ammonite.ops.Path) = new Source(path)
@@ -59,7 +64,7 @@ object Task extends Applicative.Applyer[Task, Task, Args]{
     def evaluate(args: Args)  = t0
   }
 
-  implicit def apply[T](t: T): Target[T] = macro targetImpl[T]
+
 
   def apply[T](t: Task[T]): Target[T] = macro Cacher.impl0[Task, T]
 
diff --git a/core/src/main/scala/mill/discover/Discovered.scala b/core/src/main/scala/mill/discover/Discovered.scala
index ca1479b9..0108d105 100644
--- a/core/src/main/scala/mill/discover/Discovered.scala
+++ b/core/src/main/scala/mill/discover/Discovered.scala
@@ -1,55 +1,65 @@
 package mill.discover
 
+import mill.define.Task.Module
 import mill.define.{Target, Task}
 import mill.discover.Router.{EntryPoint, Result}
 
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox.Context
-sealed trait Info[T, V]
-class Discovered[T](val targets: Seq[LabelInfo[T, _]],
-                    val mains: Seq[CommandInfo[T, _]],
-                    val hierarchy: Hierarchy[T]){
-  def apply(t: T) = targets.map{case LabelInfo(a, f, b) => (a, f, b(t)) }
+
+class Discovered[T](val hierarchy: Hierarchy[T, _]){
+  def targets(obj: T) = {
+    def rec[V](segmentsRev: List[String], h: Hierarchy[T, V]): Seq[Labelled[_]]= {
+      val self = h.targets.map(t =>
+        t.labelled(h.node(obj), (t.label :: segmentsRev).reverse)
+      )
+      self ++ h.children.flatMap{case (label, c) => rec(label :: segmentsRev, c)}
+    }
+    rec(Nil, hierarchy)
+  }
+  def mains = {
+    def rec[V](segmentsRev: List[String], h: Hierarchy[T, V]): Seq[(Seq[String], EntryPoint[_])]= {
+      h.commands.map((segmentsRev.reverse, _)) ++
+      h.children.flatMap{case (label, c) => rec(label :: segmentsRev, c)}
+    }
+    rec(Nil, hierarchy)
+  }
 }
 
-case class Hierarchy[T](path: Seq[String], node: T => Any, children: List[Hierarchy[T]])
+case class Hierarchy[T, V](node: T => V,
+                           commands: Seq[EntryPoint[V]],
+                           targets: Seq[TargetInfo[V, _]],
+                           children: List[(String, Hierarchy[T, _])])
 
 case class Labelled[T](target: Task[T],
                        format: upickle.default.ReadWriter[T],
                        segments: Seq[String])
 
-case class LabelInfo[T, V](path: Seq[String],
-                           format: upickle.default.ReadWriter[V],
-                           run: T => Task[V]) extends Info[T, V]
-
-case class CommandInfo[T, V](path: Seq[String],
-                             resolve: T => V,
-                             entryPoint: EntryPoint[V]) extends Info[T, V]{
-  def invoke(target: T, groupedArgs: Seq[(String, Option[String])]): Result[Task[Any]] = {
-    entryPoint.invoke(resolve(target),groupedArgs)
-  }
+case class TargetInfo[T, V](label: String,
+                            format: upickle.default.ReadWriter[V],
+                            run: T => Task[V]) {
+  def labelled(t: T, segments: Seq[String]) = Labelled(run(t), format, segments)
 }
-object CommandInfo{
-  def make[T, V](path: Seq[String], resolve: T => V)
-                (entryPoint: EntryPoint[V]) = CommandInfo(path, resolve, entryPoint)
+object TargetInfo{
+  def make[T, V](label: String, func: T => Task[V])
+                (implicit f: upickle.default.ReadWriter[V]) = {
+    TargetInfo(label, f, func)
+  }
 }
+
 object Discovered {
   def consistencyCheck[T](base: T, d: Discovered[T]) = {
     val inconsistent = for{
-      LabelInfo(path, formatter, targetGen) <- d.targets
-      if targetGen(base) ne targetGen(base)
-    } yield path
+      (t1, t2) <- d.targets(base).zip(d.targets(base))
+      if t1.target ne t2.target
+    } yield t1.segments
     inconsistent
   }
-  def makeTuple[T, V](path: Seq[String], func: T => Task[V])(implicit f: upickle.default.ReadWriter[V]) = {
-    LabelInfo(path, f, func)
-  }
+
 
 
   def mapping[T: Discovered](t: T): Map[Task[_], Labelled[_]] = {
-    implicitly[Discovered[T]].apply(t)
-      .map(x => x._3 -> Labelled(x._3.asInstanceOf[Task[Any]], x._2.asInstanceOf[upickle.default.ReadWriter[Any]], x._1))
-      .toMap
+    implicitly[Discovered[T]].targets(t).map(x => x.target -> x).toMap
   }
 
   implicit def apply[T]: Discovered[T] = macro applyImpl[T]
@@ -58,34 +68,33 @@ object Discovered {
     import c.universe._
     val tpe = c.weakTypeTag[T].tpe
     def rec(segments: List[String],
-            t: c.Type): (Seq[(Seq[String], Tree)], Seq[Seq[String]], Tree) = {
+            t: c.Type): Tree = {
 
       val r = new Router(c)
-      val selfMains =
-        for(tree <- r.getAllRoutesForClass(t.asInstanceOf[r.c.Type]).asInstanceOf[Seq[c.Tree]])
-        yield (segments, tree)
-
-      val items = for {
-        m <- t.members.toSeq
-        if
-          (m.isTerm && (m.asTerm.isGetter || m.asTerm.isLazy)) ||
-          m.isModule ||
-          (m.isMethod && m.typeSignature.paramLists.isEmpty && m.typeSignature.resultType <:< c.weakTypeOf[Target[_]])
-        if !m.name.toString.contains('$')
+
+
+      val targets = for {
+        m <- t.members.toList
+        if m.isMethod &&
+           m.typeSignature.paramLists.isEmpty &&
+           m.typeSignature.resultType <:< c.weakTypeOf[Target[_]] &&
+           !m.name.toString.contains('$') &&
+           !m.name.toString.contains(' ')
       } yield {
-        val extendedSegments = m.name.toString :: segments
-        val self =
-          if (m.typeSignature.resultType <:< c.weakTypeOf[Target[_]]) Seq(extendedSegments)
-          else Nil
+        val x = Ident(TermName(c.freshName()))
+        q"mill.discover.TargetInfo.make(${m.name.toString}, ($x: ${m.typeSignature.resultType}) => $x.${m.name.toTermName})"
+      }
 
-        val (mains, children, hierarchy) = rec(extendedSegments, m.typeSignature)
 
-        val nonEmpty = mains.nonEmpty || children.nonEmpty
-        (mains, self ++ children, if (nonEmpty) Some(hierarchy) else None)
+
+      val childHierarchies = for{
+        m <- t.members.toList
+        if m.typeSignature <:< c.weakTypeOf[Module]
+      } yield {
+        val name = m.name.toString.trim
+        q"($name, ${rec(name :: segments, m.typeSignature)})"
       }
 
-      val (mains, targets, childHierarchyOpts) = items.unzip3
-      val childHierarchies = childHierarchyOpts.flatMap(_.toSeq).toList
       val hierarchySelector = {
         val base = q"${TermName(c.freshName())}"
         val ident = segments.reverse.foldLeft[Tree](base) { (prefix, name) =>
@@ -94,43 +103,21 @@ object Discovered {
         q"($base: $tpe) => $ident"
       }
 
-
-      Tuple3(
-        selfMains ++ mains.flatten,
-        targets.flatten,
-        q"mill.discover.Hierarchy[$tpe](${segments.reverse}, $hierarchySelector, $childHierarchies)"
-      )
+      val commands =
+        r.getAllRoutesForClass(t.asInstanceOf[r.c.Type])
+          .asInstanceOf[Seq[c.Tree]]
+          .toList
+
+      q"""mill.discover.Hierarchy[$tpe, $t](
+        $hierarchySelector,
+        $commands,
+        $targets,
+        $childHierarchies
+      )"""
     }
 
-    val (entryPoints, reversedPaths, hierarchy) = rec(Nil, tpe)
 
-    val result = for(reversedPath <- reversedPaths.toList) yield {
-      val base = q"${TermName(c.freshName())}"
-      val segments = reversedPath.reverse.toList
-      val ident = segments.foldLeft[Tree](base) { (prefix, name) =>
-        q"$prefix.${TermName(name)}"
-      }
 
-      q"mill.discover.Discovered.makeTuple($segments, ($base: $tpe) => $ident)"
-    }
-
-    val nested = for{
-      (reversedSegments, entry) <- entryPoints
-    } yield {
-      val segments = reversedSegments.reverse
-      val arg = TermName(c.freshName())
-      val select = segments.foldLeft[Tree](Ident(arg)) { (prefix, name) =>
-        q"$prefix.${TermName(name)}"
-      }
-      q"mill.discover.CommandInfo.make(Seq(..$segments), ($arg: $tpe) => $select)($entry)"
-    }
-
-    c.Expr[Discovered[T]](q"""
-      new _root_.mill.discover.Discovered(
-        $result,
-        ${nested.toList},
-        $hierarchy
-      )
-    """)
+    c.Expr[Discovered[T]](q"new _root_.mill.discover.Discovered(${rec(Nil, tpe)})")
   }
 }
diff --git a/core/src/test/scala/mill/DiscoveredTests.scala b/core/src/test/scala/mill/DiscoveredTests.scala
new file mode 100644
index 00000000..87d2e340
--- /dev/null
+++ b/core/src/test/scala/mill/DiscoveredTests.scala
@@ -0,0 +1,54 @@
+package mill
+
+import utest._
+import TestUtil.test
+import mill.define.Task.Module
+import mill.discover.{Discovered, Hierarchy}
+
+object DiscoveredTests extends TestSuite{
+
+  val tests = Tests{
+
+    'discovery{
+      class CanNest extends Module{
+        val single = test()
+        val invisible: Any = test()
+      }
+      object outer {
+        val single = test()
+        val invisible: Any = test()
+        object nested extends Module{
+          val single = test()
+          val invisible: Any = test()
+
+        }
+        val classInstance = new CanNest
+
+      }
+
+      val discovered = Discovered[outer.type]
+
+
+      def flatten(h: Hierarchy[outer.type, _]): Seq[Any] = {
+        h.node(outer) :: h.children.flatMap{case (label, c) => flatten(c)}
+      }
+      val flattenedHierarchy = flatten(discovered.hierarchy)
+
+      val expectedHierarchy = Seq(
+        outer,
+        outer.classInstance,
+        outer.nested,
+      )
+      assert(flattenedHierarchy == expectedHierarchy)
+
+      val mapped = discovered.targets(outer).map(x => x.segments -> x.target)
+
+      val expected = Seq(
+        (List("classInstance", "single"), outer.classInstance.single),
+        (List("nested", "single"), outer.nested.single),
+        (List("single"), outer.single)
+      )
+      assert(mapped.toSet == expected.toSet)
+    }
+  }
+}
diff --git a/core/src/test/scala/mill/GraphTests.scala b/core/src/test/scala/mill/GraphTests.scala
index a3eeb62f..7d4caaac 100644
--- a/core/src/test/scala/mill/GraphTests.scala
+++ b/core/src/test/scala/mill/GraphTests.scala
@@ -3,6 +3,7 @@ package mill
 import utest._
 import TestUtil.test
 import mill.define.Task
+import mill.define.Task.Module
 import mill.discover.{Discovered, Hierarchy}
 import mill.eval.Evaluator
 import mill.util.OSet
@@ -15,45 +16,6 @@ object GraphTests extends TestSuite{
     val graphs = new TestGraphs()
     import graphs._
 
-    'discovery{
-      class CanNest{
-        val single = test()
-        val invisible: Any = test()
-      }
-      object outer {
-        val single = test()
-        val invisible: Any = test()
-        object nested{
-          val single = test()
-          val invisible: Any = test()
-
-        }
-        val classInstance = new CanNest
-
-      }
-      val discovered = Discovered[outer.type]
-
-      val mapped = discovered.apply(outer).map(x => (x._1, x._3))
-      def flatten(h: Hierarchy[outer.type]): Seq[Any] = {
-        h.node(outer) :: h.children.flatMap(flatten)
-      }
-      val flattenedHierarchy = flatten(discovered.hierarchy)
-
-      val expectedHierarchy = Seq(
-        outer,
-        outer.classInstance,
-        outer.nested,
-      )
-      assert(flattenedHierarchy == expectedHierarchy)
-
-      val expected = Seq(
-        (List("classInstance", "single"), outer.classInstance.single),
-        (List("nested", "single"), outer.nested.single),
-        (List("single"), outer.single)
-      )
-      assert(mapped == expected)
-    }
-
     'failConsistencyChecks - {
       // Make sure these fail because `def`s without `Cacher` will re-evaluate
       // each time, returning different sets of targets.