From a83c280db123f814d5fd21a634017361fb83372d Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sun, 17 Nov 2019 09:16:29 -0800 Subject: [PATCH] Improve StateTransform The StateTransform class introduced in 9cdeb7120ed7bdb9291e24060a16a268eeb9cfcf did not cleanly integrate with logic for transforming the state using the `transformState` task attribute. For one thing, the state transform was only applied if the root node returned StateTransform but no transformation would occur if a task had a dependency that returned StateTransform. Moreover, the transformation would override all other transformations that may have occurred during task evaluation. This commit updates StateTransform to act very similarly to the transformState attribute. Instead of wrapping a `State` instance, it now wraps a transformation function from `State => State`. This function can be applied on top of or prior to the other transformations via the `transformState`. For binary compatibility with 1.3.0, I had to add the stateProxy function as a constructor parameter in order to implement the `state` method. The proxy function will generally throw an exception unless the legacy api is used. To avoid that, I private[sbt]'d the legacy api so that binary compatibility is preserved but any builds targeting > 1.4.x will be required to use the new api. Unfortunately I couldn't private[sbt] StateTransform.apply(state: State) because mima interpreted it as a method type change becuase I added StateTransform.apply(transform: State => State). This may be a mima bug given that StateTransform.apply(state: State) would be jvm public even when private[sbt], but I figured it was quite unlikely that any users were using this method anyway since it was incorrectly implemented in 1.3.0 to return `state` instead of `new StateTransform(state)`. --- main/src/main/scala/sbt/EvaluateTask.scala | 11 ++--- main/src/main/scala/sbt/StateTransform.scala | 43 ++++++++++++++++--- .../main/scala/sbt/internal/Continuous.scala | 3 +- .../sbt/internal/nio/CheckBuildSources.scala | 8 ++-- sbt/src/sbt-test/nio/transform/build.sbt | 31 +++++++++++++ sbt/src/sbt-test/nio/transform/test | 13 ++++++ .../watch/dynamic-inputs/project/Build.scala | 4 +- .../watch/on-termination/project/Build.scala | 2 +- 8 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 sbt/src/sbt-test/nio/transform/build.sbt create mode 100644 sbt/src/sbt-test/nio/transform/test diff --git a/main/src/main/scala/sbt/EvaluateTask.scala b/main/src/main/scala/sbt/EvaluateTask.scala index 2e1105a50..166851cc6 100644 --- a/main/src/main/scala/sbt/EvaluateTask.scala +++ b/main/src/main/scala/sbt/EvaluateTask.scala @@ -515,17 +515,14 @@ object EvaluateTask { state: State, root: Task[T] ): (State, Result[T]) = { - val newState = results(root) match { - case Value(KeyValue(_, st: StateTransform) :: Nil) => st.state - case _ => stateTransform(results)(state) - } - (newState, results(root)) + (stateTransform(results)(state), results(root)) } def stateTransform(results: RMap[Task, Result]): State => State = Function.chain( results.toTypedSeq flatMap { - case results.TPair(Task(info, _), Value(v)) => info.post(v) get transformState - case _ => Nil + case results.TPair(_, Value(KeyValue(_, st: StateTransform))) => Some(st.transform) + case results.TPair(Task(info, _), Value(v)) => info.post(v) get transformState + case _ => Nil } ) diff --git a/main/src/main/scala/sbt/StateTransform.scala b/main/src/main/scala/sbt/StateTransform.scala index abea411d5..66d5d9165 100644 --- a/main/src/main/scala/sbt/StateTransform.scala +++ b/main/src/main/scala/sbt/StateTransform.scala @@ -7,14 +7,43 @@ 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)" +/** + * Provides a mechanism for a task to transform the state based on the result of the task. For + * example: + * {{{ + * val foo = AttributeKey[String]("foo") + * val setFoo = taskKey[StateTransform]("") + * setFoo := { + * state.value.get(foo) match { + * case None => StateTransform(_.put(foo, "foo")) + * case _ => StateTransform(identity) + * } + * } + * val getFoo = taskKey[Option[String]] + * getFoo := state.value.get(foo) + * }}} + * Prior to a call to `setFoo`, `getFoo` will return `None`. After a call to `setFoo`, `getFoo` will + * return `Some("foo")`. + */ +final class StateTransform private (val transform: State => State, stateProxy: () => State) { + @deprecated("Exists only for binary compatibility with 1.3.x.", "1.4.0") + private[sbt] def state: State = stateProxy() + @deprecated("1.4.0", "Use the constructor that takes a transform function.") + private[sbt] def this(state: State) = this((_: State) => state, () => state) } + object StateTransform { + @deprecated("Exists only for binary compatibility with 1.3.x", "1.4.0") def apply(state: State): State = state + + /** + * Create an instance of [[StateTransform]]. + * @param transform the transformation to apply after task evaluation has completed + * @return the [[StateTransform]]. + */ + def apply(transform: State => State) = + new StateTransform( + transform, + () => throw new IllegalStateException("No state was added to the StateTransform.") + ) } diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index 34b37f443..697d362a1 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -117,7 +117,8 @@ private[sbt] object Continuous extends DeprecatedContinuous { private[sbt] def continuousTask: Def.Initialize[InputTask[StateTransform]] = Def.inputTask { val (initialCount, commands) = continuousParser.parsed - new StateTransform(runToTermination(state.value, commands, initialCount, isCommand = false)) + val newState = runToTermination(state.value, commands, initialCount, isCommand = false) + StateTransform(_ => newState) } private[sbt] val dynamicInputs = taskKey[Option[mutable.Set[DynamicInput]]]( diff --git a/main/src/main/scala/sbt/internal/nio/CheckBuildSources.scala b/main/src/main/scala/sbt/internal/nio/CheckBuildSources.scala index a2e54fb24..41e8b762f 100644 --- a/main/src/main/scala/sbt/internal/nio/CheckBuildSources.scala +++ b/main/src/main/scala/sbt/internal/nio/CheckBuildSources.scala @@ -24,7 +24,7 @@ private[sbt] object CheckBuildSources { val st: State = state.value val firstTime = st.get(hasCheckedMetaBuild).fold(true)(_.compareAndSet(false, true)) (onChangedBuildSource in Scope.Global).value match { - case IgnoreSourceChanges => new StateTransform(st) + case IgnoreSourceChanges => StateTransform(identity) case o => import sbt.nio.FileStamp.Formats._ logger.debug("Checking for meta build source updates") @@ -47,16 +47,16 @@ private[sbt] object CheckBuildSources { logger.info(s"$prefix\nReloading sbt...") val remaining = Exec("reload", None, None) :: st.currentCommand.toList ::: st.remainingCommands - new StateTransform(st.copy(currentCommand = None, remainingCommands = remaining)) + StateTransform(_.copy(currentCommand = None, remainingCommands = remaining)) } else { val tail = "Apply these changes by running `reload`.\nAutomatically reload the " + "build when source changes are detected by setting " + "`Global / onChangedBuildSource := ReloadOnSourceChanges`.\nDisable this " + "warning by setting `Global / onChangedBuildSource := IgnoreSourceChanges`." logger.warn(s"$prefix\n$tail") - new StateTransform(st) + StateTransform(identity) } - case _ => new StateTransform(st) + case _ => StateTransform(identity) } } } diff --git a/sbt/src/sbt-test/nio/transform/build.sbt b/sbt/src/sbt-test/nio/transform/build.sbt new file mode 100644 index 000000000..9aba3478f --- /dev/null +++ b/sbt/src/sbt-test/nio/transform/build.sbt @@ -0,0 +1,31 @@ +import complete.DefaultParsers._ +import complete.Parser +import sbt.Def.Initialize + +val keep = taskKey[Int]("") +val checkKeep = inputKey[Unit]("") + +keep := (getPrevious(keep) map { case None => 3; case Some(x) => x + 1 } keepAs keep).value +checkKeep := inputCheck((ctx, s) => Space ~> str(getFromContext(keep, ctx, s))).evaluated + +val foo = AttributeKey[Int]("foo") +val transform = taskKey[StateTransform]("") +transform := { + keep.value + StateTransform(_.put(foo, 1)) +} + +def inputCheck[T](f: (ScopedKey[_], State) => Parser[T]): Initialize[InputTask[Unit]] = + InputTask(resolvedScoped(ctx => (s: State) => f(ctx, s)))(dummyTask) + +val checkTransform = taskKey[Unit]("") +checkTransform := { + assert(state.value.get(foo).contains(1)) +} + +def dummyTask: Any => Initialize[Task[Unit]] = + (_: Any) => + maxErrors map { _ => + () + } +def str(o: Option[Int]) = o match { case None => "blue"; case Some(i) => i.toString } diff --git a/sbt/src/sbt-test/nio/transform/test b/sbt/src/sbt-test/nio/transform/test new file mode 100644 index 000000000..f1acac98d --- /dev/null +++ b/sbt/src/sbt-test/nio/transform/test @@ -0,0 +1,13 @@ +> checkKeep blue + +-> checkTransform + +> transform + +> checkKeep 3 + +> checkTransform + +> transform + +> checkKeep 4 \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/dynamic-inputs/project/Build.scala b/sbt/src/sbt-test/watch/dynamic-inputs/project/Build.scala index 2ef4e9b51..105442243 100644 --- a/sbt/src/sbt-test/watch/dynamic-inputs/project/Build.scala +++ b/sbt/src/sbt-test/watch/dynamic-inputs/project/Build.scala @@ -38,8 +38,8 @@ object Build { checkStringValue := checkStringValueImpl.evaluated, watchOnFileInputEvent := { (_, _) => Watch.CancelWatch }, watchTasks := Def.inputTask { - val prev = watchTasks.evaluated - new StateTransform(prev.state.fail) + watchTasks.evaluated + StateTransform(_.fail) }.evaluated ) } diff --git a/sbt/src/sbt-test/watch/on-termination/project/Build.scala b/sbt/src/sbt-test/watch/on-termination/project/Build.scala index 724bc77a0..da346139f 100644 --- a/sbt/src/sbt-test/watch/on-termination/project/Build.scala +++ b/sbt/src/sbt-test/watch/on-termination/project/Build.scala @@ -25,7 +25,7 @@ object Build { watchOnFileInputEvent := { (_, _) => Watch.CancelWatch }, watchTasks := Def.inputTask { val prev = watchTasks.evaluated - new StateTransform(prev.state.fail) + StateTransform(_.fail) }.evaluated ) } \ No newline at end of file