Tighten up `LabelledTarget` to contain a `Target[T]` instead of a `Task[T]`

7 files changed, 46 insertions, 33 deletions

diff --git a/core/src/main/scala/mill/define/Task.scala b/core/src/main/scala/mill/define/Task.scala
index 70f93d35..7635a74f 100644
--- a/core/src/main/scala/mill/define/Task.scala
+++ b/core/src/main/scala/mill/define/Task.scala
@@ -26,9 +26,15 @@ abstract class Task[+T] extends Task.Ops[T] with Applyable[T]{
   def sideHash: Int = 0
 
   def flushDest: Boolean = true
+
+  def asTarget: Option[Target[T]] = None
+  def asCommand: Option[Command[T]] = None
+  def asPersistent: Option[Persistent[T]] = None
 }
 
-trait Target[+T] extends Task[T]
+trait Target[+T] extends Task[T]{
+  override def asTarget = Some(this)
+}
 object Target extends Applicative.Applyer[Task, Task, Args]{
 
   implicit def apply[T](t: T): Target[T] = macro targetImpl[T]
@@ -113,11 +119,13 @@ class TargetImpl[+T](t: Task[T], enclosing: String) extends Target[T] {
 class Command[+T](t: Task[T]) extends Task[T] {
   val inputs = Seq(t)
   def evaluate(args: Args) = args[T](0)
+  override def asCommand = Some(this)
 }
 class Persistent[+T](t: Task[T]) extends Target[T] {
   val inputs = Seq(t)
   def evaluate(args: Args) = args[T](0)
   override def flushDest = false
+  override def asPersistent = Some(this)
 }
 object Source{
   implicit def apply(p: ammonite.ops.Path) = new Source(p)
diff --git a/core/src/main/scala/mill/discover/Discovered.scala b/core/src/main/scala/mill/discover/Discovered.scala
index e0232c1a..499a0736 100644
--- a/core/src/main/scala/mill/discover/Discovered.scala
+++ b/core/src/main/scala/mill/discover/Discovered.scala
@@ -34,7 +34,7 @@ object Discovered {
 
 
 
-  def mapping[T: Discovered](t: T): Map[Task[_], LabelledTarget[_]] = {
+  def mapping[T: Discovered](t: T): Map[Target[_], LabelledTarget[_]] = {
     implicitly[Discovered[T]].targets(t).map(x => x.target -> x).toMap
   }
 
diff --git a/core/src/main/scala/mill/discover/Mirror.scala b/core/src/main/scala/mill/discover/Mirror.scala
index 8456d976..ca89bdcb 100644
--- a/core/src/main/scala/mill/discover/Mirror.scala
+++ b/core/src/main/scala/mill/discover/Mirror.scala
@@ -35,7 +35,7 @@ object Mirror{
   /**
     * A target after being materialized in a concrete build
     */
-  case class LabelledTarget[V](target: Task[V],
+  case class LabelledTarget[V](target: Target[V],
                                format: upickle.default.ReadWriter[V],
                                segments: Seq[String])
 
diff --git a/core/src/main/scala/mill/eval/Evaluator.scala b/core/src/main/scala/mill/eval/Evaluator.scala
index e93c2dd0..db364e20 100644
--- a/core/src/main/scala/mill/eval/Evaluator.scala
+++ b/core/src/main/scala/mill/eval/Evaluator.scala
@@ -1,33 +1,30 @@
 package mill.eval
 
 import ammonite.ops._
-import mill.define.Task
+import mill.define.{Target, Task}
 import mill.discover.Mirror.LabelledTarget
 import mill.util.{Args, MultiBiMap, OSet}
 
 import scala.collection.mutable
 class Evaluator(workspacePath: Path,
-                labeling: Map[Task[_], LabelledTarget[_]]){
+                labeling: Map[Target[_], LabelledTarget[_]]){
 
   def evaluate(goals: OSet[Task[_]]): Evaluator.Results = {
     mkdir(workspacePath)
 
     val transitive = Evaluator.transitiveTargets(goals)
     val topoSorted = Evaluator.topoSorted(transitive)
-    val sortedGroups = Evaluator.groupAroundImportantTargets(
-      topoSorted,
-      t => labeling.contains(t) || goals.contains(t)
-    )
+    val sortedGroups = Evaluator.groupAroundImportantTargets(topoSorted){
+      case t: Target[_] if labeling.contains(t) || goals.contains(t) => Right(labeling(t))
+      case t if goals.contains(t) => Left(t)
+    }
 
     val evaluated = new OSet.Mutable[Task[_]]
     val results = mutable.LinkedHashMap.empty[Task[_], Any]
 
     for ((terminal, group)<- sortedGroups.items()){
       val (newResults, newEvaluated) = evaluateGroupCached(
-        labeling.get(terminal) match{
-          case Some(labeled) => Right(labeled)
-          case None => Left(terminal)
-        },
+        terminal,
         group,
         results
       )
@@ -121,23 +118,23 @@ class Evaluator(workspacePath: Path,
 object Evaluator{
   class TopoSorted private[Evaluator](val values: OSet[Task[_]])
   case class Results(values: Seq[Any], evaluated: OSet[Task[_]], transitive: OSet[Task[_]])
-  def groupAroundImportantTargets(topoSortedTargets: TopoSorted,
-                                  important: Task[_] => Boolean): MultiBiMap[Task[_], Task[_]] = {
+  def groupAroundImportantTargets[T](topoSortedTargets: TopoSorted)
+                                    (important: PartialFunction[Task[_], T]): MultiBiMap[T, Task[_]] = {
 
-    val output = new MultiBiMap.Mutable[Task[_], Task[_]]()
-    for (target <- topoSortedTargets.values if important(target)) {
+    val output = new MultiBiMap.Mutable[T, Task[_]]()
+    for ((target, t) <- topoSortedTargets.values.flatMap(t => important.lift(t).map((t, _)))) {
 
       val transitiveTargets = new OSet.Mutable[Task[_]]
       def rec(t: Task[_]): Unit = {
         if (transitiveTargets.contains(t)) () // do nothing
-        else if (important(t) && t != target) () // do nothing
+        else if (important.isDefinedAt(t) && t != target) () // do nothing
         else {
           transitiveTargets.append(t)
           t.inputs.foreach(rec)
         }
       }
       rec(target)
-      output.addAll(target, topoSorted(transitiveTargets).values)
+      output.addAll(t, topoSorted(transitiveTargets).values)
     }
     output
   }
diff --git a/core/src/test/scala/mill/EvaluationTests.scala b/core/src/test/scala/mill/EvaluationTests.scala
index 053fdd09..d6c928e5 100644
--- a/core/src/test/scala/mill/EvaluationTests.scala
+++ b/core/src/test/scala/mill/EvaluationTests.scala
@@ -52,10 +52,10 @@ object EvaluationTests extends TestSuite{
     val topoSorted = Evaluator.topoSorted(
       Evaluator.transitiveTargets(OSet.from(goals))
     )
-    val grouped = Evaluator.groupAroundImportantTargets(
-      topoSorted,
-      t => labeling.contains(t) || goals.contains(t)
-    )
+    val grouped = Evaluator.groupAroundImportantTargets(topoSorted) {
+      case t: Target[_] if labeling.contains(t) || goals.contains(t) => t
+      case t if goals.contains(t) => t
+    }
     grouped.keyCount
   }
 
diff --git a/core/src/test/scala/mill/GraphTests.scala b/core/src/test/scala/mill/GraphTests.scala
index a9a31971..00ea3cc3 100644
--- a/core/src/test/scala/mill/GraphTests.scala
+++ b/core/src/test/scala/mill/GraphTests.scala
@@ -1,7 +1,7 @@
 package mill
 
 import utest._
-import mill.define.Task
+import mill.define.{Target, Task}
 import mill.discover.Discovered
 import mill.eval.Evaluator
 import mill.util.OSet
@@ -109,14 +109,16 @@ object GraphTests extends TestSuite{
     }
 
     'groupAroundNamedTargets - {
-      def check[T: Discovered, R <: Task[Int]](base: T,
-                                               target: R,
-                                               expected: OSet[(R, Int)]) = {
+      def check[T: Discovered, R <: Target[Int]](base: T,
+                                                 target: R,
+                                                 expected: OSet[(R, Int)]) = {
 
         val mapping = Discovered.mapping(base)
-        val topoSortedTransitive = Evaluator.topoSorted(Evaluator.transitiveTargets(OSet(target)))
+        val topoSorted = Evaluator.topoSorted(Evaluator.transitiveTargets(OSet(target)))
 
-        val grouped = Evaluator.groupAroundImportantTargets(topoSortedTransitive, mapping.contains)
+        val grouped = Evaluator.groupAroundImportantTargets(topoSorted) {
+          case t: Target[_] if mapping.contains(t) => t
+        }
         val flattened = OSet.from(grouped.values().flatMap(_.items))
 
         TestUtil.checkTopological(flattened)
@@ -124,7 +126,7 @@ object GraphTests extends TestSuite{
           val grouping = grouped.lookupKey(terminal)
           assert(
             grouping.size == expectedSize,
-            grouping.filter(mapping.contains) == OSet(terminal)
+            grouping.flatMap(_.asTarget: Option[Target[_]]).filter(mapping.contains) == OSet(terminal)
           )
         }
       }
diff --git a/core/src/test/scala/mill/JavaCompileJarTests.scala b/core/src/test/scala/mill/JavaCompileJarTests.scala
index 8050ef6a..77add85a 100644
--- a/core/src/test/scala/mill/JavaCompileJarTests.scala
+++ b/core/src/test/scala/mill/JavaCompileJarTests.scala
@@ -3,7 +3,7 @@ package mill
 
 import ammonite.ops._
 import ImplicitWd._
-import mill.define.Task
+import mill.define.{Target, Task}
 import mill.discover.Discovered
 import mill.eval.Evaluator
 import mill.modules.Jvm.jarUp
@@ -54,13 +54,19 @@ object JavaCompileJarTests extends TestSuite{
         val evaluated = evaluator.evaluate(OSet(t))
         Tuple2(
           evaluated.values(0).asInstanceOf[T],
-          evaluated.evaluated.filter(x => mapping.contains(x) || x.isInstanceOf[mill.define.Command[_]]).size
+          evaluated.evaluated.collect{
+            case t: Target[_] if mapping.contains(t) => t
+            case t: mill.define.Command[_] => t
+          }.size
         )
       }
       def check(targets: OSet[Task[_]], expected: OSet[Task[_]]) = {
         val evaluator = new Evaluator(workspacePath, mapping)
 
-        val evaluated = evaluator.evaluate(targets).evaluated.filter(mapping.contains)
+        val evaluated = evaluator.evaluate(targets)
+          .evaluated
+          .flatMap(_.asTarget)
+          .filter(mapping.contains)
         assert(evaluated == expected)
       }