Enforce that only one task in a group can use `T.ctx().dest` https://github.com/lihaoyi/mill/issues/62

3 files changed, 50 insertions, 16 deletions

diff --git a/core/src/mill/eval/Evaluator.scala b/core/src/mill/eval/Evaluator.scala
index 0b5d372d..4028588b 100644
--- a/core/src/mill/eval/Evaluator.scala
+++ b/core/src/mill/eval/Evaluator.scala
@@ -162,8 +162,6 @@ class Evaluator[T](val outPath: Path,
 
             if (labelledNamedTask.task.flushDest) rm(paths.dest)
 
-            mkdir(paths.dest)
-
             val (newResults, newEvaluated) = evaluateGroup(
               group,
               results,
@@ -231,20 +229,40 @@ class Evaluator[T](val outPath: Path,
     }
 
     val multiLogger = resolveLogger(paths.map(_.log))
-
-    for (target <- nonEvaluatedTargets) {
-
-      newEvaluated.append(target)
-      val targetInputValues = target.inputs
+    var usedDest = Option.empty[(Task[_], Array[StackTraceElement])]
+    for (task <- nonEvaluatedTargets) {
+      val currentStack = new Exception().getStackTrace
+      newEvaluated.append(task)
+      val targetInputValues = task.inputs
         .map(x => newResults.getOrElse(x, results(x)))
         .collect{ case Result.Success((v, hashCode)) => v }
 
       val res =
-        if (targetInputValues.length != target.inputs.length) Result.Skipped
+        if (targetInputValues.length != task.inputs.length) Result.Skipped
         else {
           val args = new Ctx(
             targetInputValues.toArray[Any],
-            paths.map(_.dest).orNull,
+            () => usedDest match{
+              case Some((earlierTask, earlierStack)) if earlierTask != task =>
+                val inner = new Exception("Earlier usage of `dest`")
+                inner.setStackTrace(earlierStack)
+                throw new Exception(
+                  "`dest` can only be used in one place within each Target[T]",
+                  inner
+                )
+              case _ =>
+                usedDest = Some((
+                  task,
+                  new Exception().getStackTrace.dropRight(currentStack.length)
+                ))
+                paths match{
+                  case Some(dest) =>
+                    mkdir(dest.dest)
+                    dest.dest
+                  case None =>
+                    throw new Exception("No `dest` folder available here")
+                }
+            },
             groupBasePath.orNull,
             multiLogger
           )
@@ -256,11 +274,11 @@ class Evaluator[T](val outPath: Path,
             System.setOut(multiLogger.outputStream)
             Console.withOut(multiLogger.outputStream){
               Console.withErr(multiLogger.errorStream){
-                target.evaluate(args)
+                task.evaluate(args)
               }
             }
           }catch{ case NonFatal(e) =>
-            val currentStack = new Exception().getStackTrace
+
             Result.Exception(e, currentStack)
           }finally{
             System.setErr(err)
@@ -268,9 +286,9 @@ class Evaluator[T](val outPath: Path,
           }
         }
 
-      newResults(target) = for(v <- res) yield {
+      newResults(task) = for(v <- res) yield {
         (v,
-          if (target.isInstanceOf[Worker[_]]) inputsHash
+          if (task.isInstanceOf[Worker[_]]) inputsHash
           else v.hashCode
         )
       }
diff --git a/core/src/mill/util/Ctx.scala b/core/src/mill/util/Ctx.scala
index 9f8d2826..fb5384ef 100644
--- a/core/src/mill/util/Ctx.scala
+++ b/core/src/mill/util/Ctx.scala
@@ -36,7 +36,7 @@ object Ctx{
 
 }
 class Ctx(val args: IndexedSeq[_],
-          val dest: Path,
+          dest0: () => Path,
           val base: Path,
           val log: Logger)
   extends DestCtx
@@ -44,6 +44,7 @@ class Ctx(val args: IndexedSeq[_],
   with ArgCtx
   with BaseCtx{
 
+  def dest = dest0()
   def length = args.length
   def apply[T](index: Int): T = {
     if (index >= 0 && index < args.length) args(index).asInstanceOf[T]
diff --git a/core/test/src/mill/eval/FailureTests.scala b/core/test/src/mill/eval/FailureTests.scala
index 86eccb3f..d4758a12 100644
--- a/core/test/src/mill/eval/FailureTests.scala
+++ b/core/test/src/mill/eval/FailureTests.scala
@@ -1,6 +1,6 @@
 package mill.eval
-
-import mill.util.TestEvaluator
+import mill.T
+import mill.util.{TestEvaluator, TestUtil}
 import ammonite.ops.pwd
 import utest._
 import utest.framework.TestPath
@@ -85,6 +85,21 @@ object FailureTests extends TestSuite{
         expectedRawValues = Seq(Result.Skipped)
       )
     }
+    'multipleUsesOfDest - {
+      object build extends TestUtil.BaseModule {
+        // Using `T.ctx(  ).dest` twice in a single task is ok
+        def left = T{ + T.ctx().dest.toString.length + T.ctx().dest.toString.length }
+
+        // Using `T.ctx(  ).dest` once in two different tasks is not ok
+        val task = T.task{ T.ctx().dest.toString.length  }
+        def right = T{ task() + left() + T.ctx().dest.toString().length }
+      }
+
+      val check = new TestEvaluator(build, workspace, pwd)
+      val Right(_) = check(build.left)
+      val Left(Result.Exception(e, _)) = check(build.right)
+      assert(e.getMessage.contains("`dest` can only be used in one place"))
+    }
   }
 }