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-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 =

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)
     }
   }

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 ""
        |}

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)

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
         |    }

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")
+  ()
+}

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