Add StateTransform class

This commit cleans up the approach for transforming the sbt state upon
completion of a task returning State. I add a new approach where a task
can return an instance of StateTransform, which is just a wrapper around
State. I then update EvaluateTask to apply this stateTransform rather
than the (optional) state transformation that may be stored in the Task
info parameter. By requiring that the user return StateTransform rather
than State directly, we ensure that no existing tasks that depend on the
state transformation function embedded in the Task info break. In sbt 2,
I could see the possibility of making this automatic (and probably
removing the state transformation function via attribute).

The problem with using the transformState attribute key is that it is
applied non-deterministically. This means that if you decorate a task
returning State, then the state transformation may or may not be
correctly applied.

I tracked this non-determinism down to the stateTransform
method in EvaluateTask. It iterates through the task result map and
chains all of the defined transformState attribute values. Because the
result is a map, this order is not specified. This chaining is arguably
a bad design because State => State does not imply commutivity. Indeed,
the problem here was that my state transformation functions were
constant functions, which are obviously non-commutative. I believe that
this logic likely written under the assumption that there would be no
more than one of these tranformations in a given result map.

main-settings/src/main/scala/sbt/Def.scala

@@ -14,7 +14,7 @@ import sbt.KeyRanks.{ DTask, Invisible }
 import sbt.Scope.{ GlobalScope, ThisScope }
 import sbt.internal.util.Types.const
 import sbt.internal.util.complete.Parser
-import sbt.internal.util.{ AttributeKey, Attributed, ConsoleAppender, Init }
+import sbt.internal.util._
 import sbt.util.Show
 
 /** A concrete settings system that uses `sbt.Scope` for the scope type. */

main/src/main/scala/sbt/EvaluateTask.scala

@@ -14,6 +14,7 @@ import sbt.Def.{ ScopedKey, Setting, dummyState }
 import sbt.Keys.{ TaskProgress => _, name => _, _ }
 import sbt.Project.richInitializeTask
 import sbt.Scope.Global
+import sbt.internal.Aggregation.KeyValue
 import sbt.internal.TaskName._
 import sbt.internal.TransitiveGlobs._
 import sbt.internal.util._
@@ -479,8 +480,13 @@ object EvaluateTask {
       results: RMap[Task, Result],
       state: State,
       root: Task[T]
-  ): (State, Result[T]) =
-    (stateTransform(results)(state), results(root))
+  ): (State, Result[T]) = {
+    val newState = results(root) match {
+      case Value(KeyValue(_, st: StateTransform) :: Nil) => st.state
+      case _                                             => stateTransform(results)(state)
+    }
+    (newState, results(root))
+  }
   def stateTransform(results: RMap[Task, Result]): State => State =
     Function.chain(
       results.toTypedSeq flatMap {

main/src/main/scala/sbt/Keys.scala

@@ -116,7 +116,7 @@ object Keys {
   val watchService = settingKey[() => WatchService]("Service to use to monitor file system changes.").withRank(BMinusSetting).withRank(DSetting)
   val watchStartMessage = settingKey[Int => Option[String]]("The message to show when triggered execution waits for sources to change. The parameter is the current watch iteration count.").withRank(DSetting)
   // The watchTasks key should really be named watch, but that is already taken by the deprecated watch key. I'd be surprised if there are any plugins that use it so I think we should consider breaking binary compatibility to rename this task.
-  val watchTasks = InputKey[State]("watch", "Watch a task (or multiple tasks) and rebuild when its file inputs change or user input is received. The semantics are more or less the same as the `~` command except that it cannot transform the state on exit. This means that it cannot be used to reload the build.").withRank(DSetting)
+  val watchTasks = InputKey[StateTransform]("watch", "Watch a task (or multiple tasks) and rebuild when its file inputs change or user input is received. The semantics are more or less the same as the `~` command except that it cannot transform the state on exit. This means that it cannot be used to reload the build.").withRank(DSetting)
   val watchTrackMetaBuild = settingKey[Boolean]("Toggles whether or not changing the build files (e.g. **/*.sbt, project/**/(*.scala | *.java)) should automatically trigger a project reload").withRank(DSetting)
   val watchTriggeredMessage = settingKey[(Int, Event[FileAttributes]) => Option[String]]("The message to show before triggered execution executes an action after sources change. The parameters are the path that triggered the build and the current watch iteration count.").withRank(DSetting)
 

main/src/main/scala/sbt/StateTransform.scala

@@ -0,0 +1,20 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt
+
+final class StateTransform(val state: State) {
+  override def equals(o: Any): Boolean = o match {
+    case that: StateTransform => this.state == that.state
+    case _                    => false
+  }
+  override def hashCode: Int = state.hashCode
+  override def toString: String = s"StateTransform($state)"
+}
+object StateTransform {
+  def apply(state: State): State = state
+}

main/src/main/scala/sbt/internal/Continuous.scala

@@ -23,10 +23,9 @@ import sbt.Scope.Global
 import sbt.internal.FileManagement.FileTreeRepositoryOps
 import sbt.internal.LabeledFunctions._
 import sbt.internal.io.WatchState
-import sbt.internal.util.Types.const
 import sbt.internal.util.complete.Parser._
 import sbt.internal.util.complete.{ Parser, Parsers }
-import sbt.internal.util.{ AttributeKey, AttributeMap, Util }
+import sbt.internal.util.{ AttributeKey, Util }
 import sbt.io._
 import sbt.util.{ Level, _ }
 
@@ -122,16 +121,13 @@ object Continuous extends DeprecatedContinuous {
    * we have to modify the Task.info to apply the state transformation after the task completes.
    * @return the [[InputTask]]
    */
-  private[sbt] def continuousTask: Def.Initialize[InputTask[State]] =
+  private[sbt] def continuousTask: Def.Initialize[InputTask[StateTransform]] =
     Def.inputTask {
       val (initialCount, command) = continuousParser.parsed
-      runToTermination(Keys.state.value, command, initialCount, isCommand = false)
-    }(_.mapTask { t =>
-      val postTransform = t.info.postTransform {
-        case (state: State, am: AttributeMap) => am.put(Keys.transformState, const(state))
-      }
-      Task(postTransform, t.work)
-    })
+      new StateTransform(
+        runToTermination(Keys.state.value, command, initialCount, isCommand = false)
+      )
+    }
 
   private[this] val DupedSystemIn =
     AttributeKey[DupedInputStream](

sbt/src/sbt-test/watch/on-termination/build.sbt

@@ -0,0 +1,3 @@
+import sbt.watch.task.Build
+
+val root = Build.root

sbt/src/sbt-test/watch/on-termination/project/Build.scala

@@ -0,0 +1,29 @@
+package sbt.watch.task
+
+import sbt._
+import Keys._
+
+object Build {
+  val reloadFile = settingKey[File]("file to toggle whether or not to reload")
+  val setStringValue = inputKey[Unit]("set a global string to a value")
+  val checkStringValue = inputKey[Unit]("check the value of a global")
+  def setStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask {
+    val Seq(stringFile, string) = Def.spaceDelimited().parsed.map(_.trim)
+    IO.write(file(stringFile), string)
+  }
+  def checkStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask {
+    val Seq(stringFile, string) = Def.spaceDelimited().parsed
+    assert(IO.read(file(stringFile)) == string)
+  }
+  lazy val root = (project in file(".")).settings(
+    reloadFile := baseDirectory.value / "reload",
+    setStringValue / watchTriggers += baseDirectory.value * "foo.txt",
+    setStringValue := setStringValueImpl.evaluated,
+    checkStringValue := checkStringValueImpl.evaluated,
+    watchOnTriggerEvent := { (_, _) => Watch.CancelWatch },
+    watchTasks := Def.inputTask {
+      val prev = watchTasks.evaluated
+      new StateTransform(prev.state.fail)
+    }.evaluated
+  )
+}

sbt/src/sbt-test/watch/on-termination/test

@@ -0,0 +1,7 @@
+# This tests that we can override the state transformation in the watch task
+# In the build, watchOnEvent should return CancelWatch which should be successful, but we
+# override watchTasks to fail the state instead
+
+-> watch root / setStringValue foo.txt bar
+
+> checkStringValue foo.txt bar