From b43f663b0e081647a7e4ca09a1af03b920a1a0a7 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Tue, 19 May 2020 13:57:10 -0400 Subject: [PATCH] Automatic conversion to Def.taskIf When `Def.task`, `:=`, `+=` etc contains `if` and only `if` expression automatically treat it as a conditional task even if the else clause contains `.value`. --- .../main/scala/sbt/std/TaskLinterDSL.scala | 15 ++++--- .../src/main/scala/sbt/std/TaskMacro.scala | 31 +++++++++---- .../test/scala/sbt/std/TaskConfigSpec.scala | 1 + .../src/test/scala/sbt/std/TaskPosSpec.scala | 22 +++++++++- .../test/scala/sbt/std/neg/TaskNegSpec.scala | 5 +++ sbt/src/sbt-test/actions/ifs/build.sbt | 44 ++++++++++++++----- sbt/src/sbt-test/actions/ifs/test | 13 +++++- 7 files changed, 99 insertions(+), 32 deletions(-) diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala index 534a96459..db2cfe204 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -211,15 +211,16 @@ object TaskLinterDSLFeedback { """.stripMargin def useOfValueInsideIfExpression(task: String): String = - s"""${startBold}The evaluation of `$task` happens always inside a regular task.$reset + s"""${startBold}value lookup of `$task` inside an `if` expression$reset | - |$ProblemHeader: `$task` is inside the if expression of a regular task. - | Regular tasks always evaluate task inside the bodies of if expressions. + |$ProblemHeader: `$task.value` is inside an `if` expression of a regular task. + | Regular tasks always evaluate task dependencies (`.value`) regardless of `if` expressions. |$SolutionHeader: - | 1. If you only want to evaluate it when the if predicate is true or false, use a dynamic task. - | 2. Make the static evaluation explicit by evaluating `$task` outside the if expression. - | 3. If you still want to force the static evaluation, you may annotate the task evaluation with `@sbtUnchecked`, e.g. `($task.value: @sbtUnchecked)`. - | 4. Add `import sbt.dsl.LinterLevel.Ignore` to your build file to disable all task linting. + | 1. Use a conditional task `Def.taskIf(...)` to evaluate it when the `if` predicate is true or false. + | 2. Or turn the task body into a single `if` expression; the task is then auto-converted to a conditional task. + | 3. Or make the static evaluation explicit by declaring `$task.value` outside the `if` expression. + | 4. If you still want to force the static lookup, you may annotate the task lookup with `@sbtUnchecked`, e.g. `($task.value: @sbtUnchecked)`. + | 5. Add `import sbt.dsl.LinterLevel.Ignore` to your build file to disable all task linting. """.stripMargin def missingValueForKey(key: String): String = diff --git a/main-settings/src/main/scala/sbt/std/TaskMacro.scala b/main-settings/src/main/scala/sbt/std/TaskMacro.scala index 22ac78595..c6c90fdf2 100644 --- a/main-settings/src/main/scala/sbt/std/TaskMacro.scala +++ b/main-settings/src/main/scala/sbt/std/TaskMacro.scala @@ -104,11 +104,26 @@ object TaskMacro { def taskMacroImpl[T: c.WeakTypeTag]( c: blackbox.Context - )(t: c.Expr[T]): c.Expr[Initialize[Task[T]]] = - Instance.contImpl[T, Id](c, FullInstance, FullConvert, MixedBuilder, TaskLinterDSL)( - Left(t), - Instance.idTransform[c.type] - ) + )(t: c.Expr[T]): c.Expr[Initialize[Task[T]]] = { + import c.universe._ + t.tree match { + // the tree matches `if` and only `if` + case If(cond, thenp, elsep) => + c.Expr[Initialize[Task[T]]](mkIfS(c)(cond, thenp, elsep)) + case _ => + Instance.contImpl[T, Id](c, FullInstance, FullConvert, MixedBuilder, TaskLinterDSL)( + Left(t), + Instance.idTransform[c.type] + ) + } + } + + def mkIfS( + c: blackbox.Context + )(cond: c.Tree, thenp: c.Tree, elsep: c.Tree): c.Tree = { + import c.universe._ + q"""Def.ifS(Def.task($cond))(Def.task($thenp))(Def.task($elsep))""" + } def taskDynMacroImpl[T: c.WeakTypeTag]( c: blackbox.Context @@ -122,13 +137,11 @@ object TaskMacro { c: blackbox.Context )(a: c.Expr[A]): c.Expr[Initialize[Task[A]]] = { import c.universe._ - def mkIfS(cond: Tree, thenp: Tree, elsep: Tree): Tree = - q"""Def.ifS(Def.task($cond))(Def.task($thenp))(Def.task($elsep))""" a.tree match { case Block(stat, If(cond, thenp, elsep)) => - c.Expr[Initialize[Task[A]]](mkIfS(Block(stat, cond), thenp, elsep)) + c.Expr[Initialize[Task[A]]](mkIfS(c)(Block(stat, cond), thenp, elsep)) case If(cond, thenp, elsep) => - c.Expr[Initialize[Task[A]]](mkIfS(cond, thenp, elsep)) + c.Expr[Initialize[Task[A]]](mkIfS(c)(cond, thenp, elsep)) case x => ContextUtil.unexpectedTree(x) } } diff --git a/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala b/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala index 447f28811..69b470634 100644 --- a/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala +++ b/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala @@ -52,6 +52,7 @@ class TaskConfigSpec extends fixture.FunSuite with fixture.TestDataFixture { |var condition = true | |val barNeg = Def.task[String] { + | val s = 1 | if (condition) fooNeg.value | else "" |} diff --git a/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala b/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala index 6c77a5563..851f72434 100644 --- a/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala +++ b/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala @@ -8,6 +8,20 @@ package sbt.std class TaskPosSpec { + // Starting sbt 1.4.0, Def.task can have task value lookups inside + // if branches since tasks with single if-expressions are automatically + // converted into a conditional task. + locally { + import sbt._, Def._ + val foo = taskKey[String]("") + val bar = taskKey[String]("") + val condition = true + Def.task[String] { + if (condition) foo.value + else bar.value + } + } + // Dynamic tasks can have task invocations inside if branches locally { import sbt._, Def._ @@ -147,8 +161,12 @@ class TaskPosSpec { locally { import sbt._, Def._ - def withKey(foo: => SettingKey[String]) = { - Def.task { if (true) foo.value } + def withKey(foo: => SettingKey[String]): Def.Initialize[Task[Unit]] = { + Def.task { + if (true) { + foo.value; () + } + } } val foo = settingKey[String]("") withKey(foo) diff --git a/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala b/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala index 50fbd7803..0f247a9a8 100644 --- a/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala +++ b/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala @@ -44,6 +44,7 @@ class TaskNegSpec extends fixture.FunSuite with fixture.TestDataFixture { |var condition = true | |val bazNeg = Def.task[String] { + | val s = 1 | if (condition) fooNeg.value | else barNeg.value |} @@ -66,6 +67,7 @@ class TaskNegSpec extends fixture.FunSuite with fixture.TestDataFixture { |def bi(s: String) = s + " " | |val bazNeg = Def.task[String] { + | val s = 1 | if (condition) "" + fooNeg.value | else bi(barNeg.value) |} @@ -87,6 +89,7 @@ class TaskNegSpec extends fixture.FunSuite with fixture.TestDataFixture { |val bazNeg = Def.taskDyn[String] { | if (condition) { | Def.task { + | val s = 1 | if (condition) { | fooNeg.value | } else "" @@ -114,6 +117,7 @@ class TaskNegSpec extends fixture.FunSuite with fixture.TestDataFixture { |val bazNeg = Def.taskDyn[String] { | if (condition) { | Def.task { + | val s = 1 | if (condition) { | val first = if (!condition && condition) { | fooNeg.value @@ -144,6 +148,7 @@ class TaskNegSpec extends fixture.FunSuite with fixture.TestDataFixture { |val bazNeg = Def.taskDyn[String] { | if (condition) { | Def.task { + | val s = 1 | if (condition) "" | else barNeg.value | } diff --git a/sbt/src/sbt-test/actions/ifs/build.sbt b/sbt/src/sbt-test/actions/ifs/build.sbt index 876b6663b..0d61fb75b 100644 --- a/sbt/src/sbt-test/actions/ifs/build.sbt +++ b/sbt/src/sbt-test/actions/ifs/build.sbt @@ -1,7 +1,12 @@ val condition = taskKey[Boolean]("") +val number = settingKey[Int]("") val trueAction = taskKey[Unit]("") val falseAction = taskKey[Unit]("") +val negAction = taskKey[Unit]("") +val zeroAction = taskKey[Unit]("") +val posAction = taskKey[Unit]("") val foo = taskKey[Unit]("") +val bar = taskKey[Unit]("") val output = settingKey[File]("") lazy val root = (project in file(".")) @@ -9,20 +14,35 @@ lazy val root = (project in file(".")) name := "ifs", output := baseDirectory.value / "output.txt", condition := true, - trueAction := { IO.write(output.value, s"true\n", append = true) }, - falseAction := { IO.write(output.value, s"false\n", append = true) }, + number := -1, + + // automatic conversion to Def.ifS + bar := { + if (number.value < 0) negAction.value + else if (number.value == 0) zeroAction.value + else posAction.value + }, + + // explicit call to Def.taskIf foo := (Def.taskIf { if (condition.value) trueAction.value else falseAction.value }).value, - TaskKey[Unit]("check") := { - val lines = IO.read(output.value).linesIterator.toList - assert(lines == List("true")) - () - }, - TaskKey[Unit]("check2") := { - val lines = IO.read(output.value).linesIterator.toList - assert(lines == List("false")) - () - }, + + trueAction := { IO.write(output.value, s"true\n", append = true) }, + falseAction := { IO.write(output.value, s"false\n", append = true) }, + negAction := { IO.write(output.value, s"neg\n", append = true) }, + zeroAction := { IO.write(output.value, s"zero\n", append = true) }, + posAction := { IO.write(output.value, s"pos\n", append = true) }, + + TaskKey[Unit]("checkTrue") := checkLines("true"), + TaskKey[Unit]("checkFalse") := checkLines("false"), + TaskKey[Unit]("checkNeg") := checkLines("neg"), + TaskKey[Unit]("checkZero") := checkLines("zero"), ) + +def checkLines(content: String) = Def.task { + val lines = IO.read(output.value).linesIterator.toList + assert(lines == List("true"), s"$content was expected but found: $lines") + () +} diff --git a/sbt/src/sbt-test/actions/ifs/test b/sbt/src/sbt-test/actions/ifs/test index 89589feca..ca5845809 100644 --- a/sbt/src/sbt-test/actions/ifs/test +++ b/sbt/src/sbt-test/actions/ifs/test @@ -1,7 +1,16 @@ > foo -> check +> checkTrue + +$ delete output.txt +> bar +> checkNeg $ delete output.txt > set condition := false > foo -> check2 +> checkFalse + +$ delete output.txt +> set number := 0 +> bar +> checkZero