From ab2df045ab3dc1379082a90ef9c97f38cac81792 Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Mon, 10 Dec 2018 15:16:38 -0800
Subject: [PATCH 1/6] Lint TaskLinterDSL for intellij

This cleans up all of the yellow intellij warnings for me. It still
complains about some typos, but those manifest as green squiggled lines
which are less annoying.
---
 .../main/scala/sbt/std/TaskLinterDSL.scala    | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
index 7ab117fe7..1800dab8b 100644
--- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
+++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
@@ -80,9 +80,9 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
 
       override def traverse(tree: ctx.universe.Tree): Unit = {
         tree match {
-          case ap @ Apply(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) =>
+          case ap @ Apply(TypeApply(Select(_, name), tpe :: Nil), qual :: Nil) =>
             val shouldIgnore = uncheckedWrappers.contains(ap)
-            val wrapperName = nme.decodedName.toString
+            val wrapperName = name.decodedName.toString
             val (qualName, isSettingKey) =
               Option(qual.symbol)
                 .map(sym => (sym.name.decodedName.toString, qual.tpe <:< typeOf[SettingKey[_]]))
@@ -177,39 +177,39 @@ object TaskLinterDSLFeedback {
   private final val startGreen = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.GREEN else ""
   private final val reset = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.RESET else ""
 
-  private final val ProblemHeader = s"${startRed}problem${reset}"
-  private final val SolutionHeader = s"${startGreen}solution${reset}"
+  private final val ProblemHeader = s"${startRed}problem$reset"
+  private final val SolutionHeader = s"${startGreen}solution$reset"
 
-  def useOfValueInsideAnon(task: String) =
+  def useOfValueInsideAnon(task: String): String =
     s"""${startBold}The evaluation of `$task` inside an anonymous function is prohibited.$reset
        |
-       |${ProblemHeader}: Task invocations inside anonymous functions are evaluated independently of whether the anonymous function is invoked or not.
-       |${SolutionHeader}:
+       |$ProblemHeader: Task invocations inside anonymous functions are evaluated independently of whether the anonymous function is invoked or not.
+       |$SolutionHeader:
        |  1. Make `$task` evaluation explicit outside of the function body if you don't care about its evaluation.
        |  2. Use a dynamic task to evaluate `$task` and pass that value as a parameter to an anonymous function.
     """.stripMargin
 
-  def useOfValueInsideIfExpression(task: String) =
+  def useOfValueInsideIfExpression(task: String): String =
     s"""${startBold}The evaluation of `$task` happens always inside a regular task.$reset
        |
-       |${ProblemHeader}: `$task` is inside the if expression of a regular task.
+       |$ProblemHeader: `$task` is inside the if expression of a regular task.
        |  Regular tasks always evaluate task inside the bodies of if expressions.
-       |${SolutionHeader}:
+       |$SolutionHeader:
        |  1. If you only want to evaluate it when the if predicate is true or false, use a dynamic task.
        |  2. Otherwise, make the static evaluation explicit by evaluating `$task` outside the if expression.
     """.stripMargin
 
-  def missingValueForKey(key: String) =
+  def missingValueForKey(key: String): String =
     s"""${startBold}The key `$key` is not being invoked inside the task definition.$reset
        |
-       |${ProblemHeader}:  Keys missing `.value` are not initialized and their dependency is not registered.
-       |${SolutionHeader}: Replace `$key` by `$key.value` or remove it if unused.
+       |$ProblemHeader:  Keys missing `.value` are not initialized and their dependency is not registered.
+       |$SolutionHeader: Replace `$key` by `$key.value` or remove it if unused.
     """.stripMargin
 
-  def missingValueForInitialize(expr: String) =
+  def missingValueForInitialize(expr: String): String =
     s"""${startBold}The setting/task `$expr` is not being invoked inside the task definition.$reset
        |
-       |${ProblemHeader}:  Settings/tasks missing `.value` are not initialized and their dependency is not registered.
-       |${SolutionHeader}: Replace `$expr` by `($expr).value` or remove it if unused.
+       |$ProblemHeader:  Settings/tasks missing `.value` are not initialized and their dependency is not registered.
+       |$SolutionHeader: Replace `$expr` by `($expr).value` or remove it if unused.
     """.stripMargin
 }

From 1c2bab093b9d1fc750db5961766e8e84f11c4640 Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Mon, 10 Dec 2018 17:24:22 -0500
Subject: [PATCH 2/6] Switch to more functional style in BaseTaskLinterDSL

I found it somewhat difficult to reason about the state of the tree
Traverser because of the usage of mutable variables and data structures.
For example, I determined that the uncheckedWrappers were never used
non-locally so a Set didn't seem to be the right data structure. It was
reasonably straightforward to switch to a more functional style by
parameterizing the method local traverser class.

Bonus:
  - remove unused variable disableNoValueReport
  - add scaladoc to document the input parameters of the traverser class
    so that it's a bit easier to understand for future maintainers.
---
 .../main/scala/sbt/std/TaskLinterDSL.scala    | 60 ++++++++++---------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
index 1800dab8b..7d037db39 100644
--- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
+++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
@@ -11,7 +11,6 @@ import sbt.SettingKey
 import sbt.internal.util.ConsoleAppender
 import sbt.internal.util.appmacro.{ Convert, LinterDSL }
 
-import scala.collection.mutable.{ HashSet => MutableSet }
 import scala.io.AnsiColor
 import scala.reflect.macros.blackbox
 
@@ -22,15 +21,26 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
   override def runLinter(ctx: blackbox.Context)(tree: ctx.Tree): Unit = {
     import ctx.universe._
     val isTask = convert.asPredicate(ctx)
-    class traverser extends Traverser {
-      private val unchecked = symbolOf[sbt.sbtUnchecked].asClass
-      private val initializeType = typeOf[sbt.Def.Initialize[_]]
-      private val uncheckedWrappers = MutableSet.empty[Tree]
-      var insideIf: Boolean = false
-      var insideAnon: Boolean = false
-      var disableNoValueReport: Boolean = false
+    val unchecked = symbolOf[sbt.sbtUnchecked].asClass
+    val initializeType = typeOf[sbt.Def.Initialize[_]]
 
-      def handleUncheckedAnnotation(exprAtUseSite: Tree, tt: TypeTree): Unit = {
+    /**
+     * Lints a task tree.
+     * @param insideIf indicates whether or not the current tree is enclosed in an if statement.
+     *                It is generally illegal to call `.value` on a task within such a tree unless
+     *                the tree has been annotated with `@sbtUnchecked`.
+     * @param insideAnon indicates whether or not the current tree is enclosed in an anonymous
+     *                   function. It is generally illegal to call `.value` on a task within such
+     *                   a tree unless the tree has been annotated with `@sbtUnchecked`.
+     * @param uncheckedWrapper an optional tree that is provided to lint a tree in the form:
+     *                         `tree.value: @sbtUnchecked` for some tree. This can be used to
+     *                         prevent the linter from rejecting task evaluation within a
+     *                         conditional or an anonymous function.
+     */
+    class traverser(insideIf: Boolean, insideAnon: Boolean, uncheckedWrapper: Option[Tree])
+        extends Traverser {
+
+      def extractUncheckedWrapper(exprAtUseSite: Tree, tt: TypeTree): Option[Tree] = {
         tt.original match {
           case Annotated(annot, _) =>
             Option(annot.tpe) match {
@@ -41,16 +51,14 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
                 if (isUnchecked) {
                   // Use expression at use site, arg contains the old expr
                   // Referential equality between the two doesn't hold
-                  val removedSbtWrapper = exprAtUseSite match {
+                  Some(exprAtUseSite match {
                     case Typed(t, _) => t
                     case _           => exprAtUseSite
-                  }
-                  uncheckedWrappers.add(removedSbtWrapper)
-                  ()
-                }
-              case _ =>
+                  })
+                } else None
+              case _ => None
             }
-          case _ =>
+          case _ => None
         }
       }
 
@@ -81,7 +89,7 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
       override def traverse(tree: ctx.universe.Tree): Unit = {
         tree match {
           case ap @ Apply(TypeApply(Select(_, name), tpe :: Nil), qual :: Nil) =>
-            val shouldIgnore = uncheckedWrappers.contains(ap)
+            val shouldIgnore = uncheckedWrapper.contains(ap)
             val wrapperName = name.decodedName.toString
             val (qualName, isSettingKey) =
               Option(qual.symbol)
@@ -101,22 +109,16 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
             traverse(qual)
           case If(condition, thenp, elsep) =>
             traverse(condition)
-            val previousInsideIf = insideIf
-            insideIf = true
-            traverse(thenp)
-            traverse(elsep)
-            insideIf = previousInsideIf
+            val newTraverser = new traverser(insideIf = true, insideAnon, uncheckedWrapper)
+            newTraverser.traverse(thenp)
+            newTraverser.traverse(elsep)
           case Typed(expr, tpt: TypeTree) if tpt.original != null =>
-            handleUncheckedAnnotation(expr, tpt)
-            traverse(expr)
+            new traverser(insideIf, insideAnon, extractUncheckedWrapper(expr, tpt)).traverse(expr)
             traverse(tpt)
           case Function(vparams, body) =>
             traverseTrees(vparams)
             if (!vparams.exists(_.mods.hasFlag(Flag.SYNTHETIC))) {
-              val previousInsideAnon = insideAnon
-              insideAnon = true
-              traverse(body)
-              insideAnon = previousInsideAnon
+              new traverser(insideIf, insideAnon = true, uncheckedWrapper).traverse(body)
             } else traverse(body)
           case Block(stmts, expr) =>
             if (!isDynamicTask) {
@@ -147,7 +149,7 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
         }
       }
     }
-    (new traverser).traverse(tree)
+    new traverser(insideIf = false, insideAnon = false, uncheckedWrapper = None).traverse(tree)
   }
 }
 

From 2e027640d337e860529afc8b599f01021343f42e Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Mon, 10 Dec 2018 15:20:39 -0800
Subject: [PATCH 3/6] Add solutions for dynamic task evaluation

There are many cases where one would want to force evaluation of the
task even when contained in a lambda (see
https://github.com/sbt/sbt/issues/3266). The @sbtUnchecked annotation is
one way to disable the linter that prevents this, but it is obscure. If
the annotation is to exist, I think it should be presented as a
solution.
---
 main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
index 7d037db39..ef535f274 100644
--- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
+++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
@@ -189,6 +189,7 @@ object TaskLinterDSLFeedback {
        |$SolutionHeader:
        |  1. Make `$task` evaluation explicit outside of the function body if you don't care about its evaluation.
        |  2. Use a dynamic task to evaluate `$task` and pass that value as a parameter to an anonymous function.
+       |  3. Annotate the `$task` evaluation with `@sbtUnchecked`, e.g. `($task.value: @sbtUnchecked)`.
     """.stripMargin
 
   def useOfValueInsideIfExpression(task: String): String =
@@ -198,7 +199,8 @@ object TaskLinterDSLFeedback {
        |  Regular tasks always evaluate task inside the bodies 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. Otherwise, make the static evaluation explicit by evaluating `$task` outside the if expression.
+       |  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)`.
     """.stripMargin
 
   def missingValueForKey(key: String): String =

From 32792f2b9d3f8ea2c02f1678982414089f91d17c Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Tue, 11 Dec 2018 11:52:06 -0800
Subject: [PATCH 4/6] Make the task linter configurable

The user should be able to configure whether or not the task linting is
strictly enforced. In my opinion, the linter is generally pretty good
about warning users that a particular definition may not behave the way
the user expects. Unfortunately, it is fairly common that the user
explicitly wants this behavior and making the linter abort compilation
is a frustrating user experience.

To fix this, I add the LinterLevel trait to a new sbt.dsl package in the
core-macros project. The user can configure the linter to:
1) abort when an issue is detected. This is the current default behavior.
2) print a warning when an issues is detected
3) skip linting all together

Linter configuration is managed by importing the corresponding implicit
object from the LinterLevel companion object.

Fixes #3266
---
 .../sbt/internal/util/appmacro/Instance.scala |  7 +-
 .../src/main/scala/sbt/dsl/LinterLevel.scala  | 68 +++++++++++++++++
 .../main/scala/sbt/std/TaskLinterDSL.scala    | 41 +++++++---
 .../test/scala/sbt/std/TaskConfigSpec.scala   | 76 +++++++++++++++++++
 4 files changed, 179 insertions(+), 13 deletions(-)
 create mode 100644 main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
 create mode 100644 main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala

diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
index 97cf76288..99e8b2ab7 100644
--- a/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
+++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
@@ -8,8 +8,8 @@
 package sbt.internal.util
 package appmacro
 
-import Classes.Applicative
-import Types.Id
+import sbt.internal.util.Classes.Applicative
+import sbt.internal.util.Types.Id
 
 /**
  * The separate hierarchy from Applicative/Monad is for two reasons.
@@ -28,8 +28,7 @@ trait MonadInstance extends Instance {
   def flatten[T](in: M[M[T]]): M[T]
 }
 
-import scala.reflect._
-import macros._
+import scala.reflect.macros._
 
 object Instance {
   type Aux[M0[_]] = Instance { type M[x] = M0[x] }
diff --git a/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala b/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
new file mode 100644
index 000000000..d1c1d28a0
--- /dev/null
+++ b/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
@@ -0,0 +1,68 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.dsl
+
+/**
+ * Controls the kind of linting performed during task macro expansion. The linting level is
+ * controlled by implicitly bringing an instance of LinterLevel into scope:* {{{
+ *   val foo = taskKey[Unit]("")
+ *   val bar = taskKey[Unit]("")
+ *   // This compiles because of the import in the block
+ *   val fooTask = {
+ *     import sbt.dsl.LinterLevel.Ignore
+ *     Def.task {
+ *       if (true) bar.value
+ *       else bar.value
+ *     }
+ *   }
+ *   // because the import above was in a local scope, it does not apply here, so this won't
+ *   // compile:
+ *   //val barTask = Def.task {
+ *   //  if (true) foo.value
+ *   //  else foo.value
+ *   //}
+ *
+ *   import sbt.dsl.LinterLevel.Ignore
+ *   // Both defs compile because the Ignore LinterLevel is implicitly brought into scope and
+ *   // picked up by both task defs underneath
+ *   val newFooTask = Def.task {
+ *     if (true) bar.value
+ *     else bar.value
+ *   }
+ *   val barTask = Def.task {
+ *     if (true) foo.value
+ *     else foo.value
+ *   }
+ * }}}
+ * To make this work, the instances are all defined as implicit case objects. Moreover, the
+ * the [[LinterLevel.Abort]] setting is made default by placing [[LinterLevel.Warn]] and
+ * [[LinterLevel.Ignore]] in the [[LinterLevelLowPriority]] trait that the [[LinterLevel]]
+ * companion object extends.
+ */
+sealed trait LinterLevel
+object LinterLevel extends LinterLevelLowPriority {
+
+  /**
+   * Abort the macro expansion if any linter check fails.
+   */
+  implicit case object Abort extends LinterLevel
+}
+
+private[dsl] trait LinterLevelLowPriority {
+
+  /**
+   * Do not perform any linting.
+   */
+  implicit case object Ignore extends LinterLevel
+
+  /**
+   * Apply the linter but print warnings instead of aborting macro expansion when linter violations
+   * are found.
+   */
+  implicit case object Warn extends LinterLevel
+}
diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
index ef535f274..d560a84a5 100644
--- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
+++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
@@ -8,6 +8,8 @@
 package sbt.std
 
 import sbt.SettingKey
+import sbt.dsl.LinterLevel
+import sbt.dsl.LinterLevel.{ Abort, Warn }
 import sbt.internal.util.ConsoleAppender
 import sbt.internal.util.appmacro.{ Convert, LinterDSL }
 
@@ -18,7 +20,9 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
   def isDynamicTask: Boolean
   def convert: Convert
 
-  override def runLinter(ctx: blackbox.Context)(tree: ctx.Tree): Unit = {
+  private def impl(
+      ctx: blackbox.Context
+  )(tree: ctx.Tree, lint: (ctx.Position, String) => Unit): Unit = {
     import ctx.universe._
     val isTask = convert.asPredicate(ctx)
     val unchecked = symbolOf[sbt.sbtUnchecked].asClass
@@ -26,6 +30,7 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
 
     /**
      * Lints a task tree.
+     *
      * @param insideIf indicates whether or not the current tree is enclosed in an if statement.
      *                It is generally illegal to call `.value` on a task within such a tree unless
      *                the tree has been annotated with `@sbtUnchecked`.
@@ -68,21 +73,21 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
       def detectAndErrorOnKeyMissingValue(i: Ident): Unit = {
         if (isKey(i.tpe)) {
           val keyName = i.name.decodedName.toString
-          ctx.error(i.pos, TaskLinterDSLFeedback.missingValueForKey(keyName))
+          lint(i.pos, TaskLinterDSLFeedback.missingValueForKey(keyName))
         } else ()
       }
 
       def detectAndErrorOnKeyMissingValue(s: Select): Unit = {
         if (isKey(s.tpe)) {
           val keyName = s.name.decodedName.toString
-          ctx.error(s.pos, TaskLinterDSLFeedback.missingValueForKey(keyName))
+          lint(s.pos, TaskLinterDSLFeedback.missingValueForKey(keyName))
         } else ()
       }
 
       def detectAndErrorOnKeyMissingValue(a: Apply): Unit = {
         if (isInitialize(a.tpe)) {
           val expr = "X / y"
-          ctx.error(a.pos, TaskLinterDSLFeedback.missingValueForInitialize(expr))
+          lint(a.pos, TaskLinterDSLFeedback.missingValueForInitialize(expr))
         } else ()
       }
 
@@ -99,11 +104,11 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
             if (!isSettingKey && !shouldIgnore && isTask(wrapperName, tpe.tpe, qual)) {
               if (insideIf && !isDynamicTask) {
                 // Error on the use of value inside the if of a regular task (dyn task is ok)
-                ctx.error(ap.pos, TaskLinterDSLFeedback.useOfValueInsideIfExpression(qualName))
+                lint(ap.pos, TaskLinterDSLFeedback.useOfValueInsideIfExpression(qualName))
               }
               if (insideAnon) {
                 // Error on the use of anonymous functions in any task or dynamic task
-                ctx.error(ap.pos, TaskLinterDSLFeedback.useOfValueInsideAnon(qualName))
+                lint(ap.pos, TaskLinterDSLFeedback.useOfValueInsideAnon(qualName))
               }
             }
             traverse(qual)
@@ -127,7 +132,8 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
                * and dangerous because we may miss some corner cases. Instead, we report
                * on the easiest cases in which we are certain that the user does not want
                * to have a stale key reference. Those are idents in the rhs of a val definition
-               * whose name is `_` and those idents that are in statement position inside blocks. */
+               * whose name is `_` and those idents that are in statement position inside blocks.
+               */
               stmts.foreach {
                 // TODO: Consider using unused names analysis to be able to report on more cases
                 case ValDef(_, valName, _, rhs) if valName == termNames.WILDCARD =>
@@ -151,6 +157,17 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
     }
     new traverser(insideIf = false, insideAnon = false, uncheckedWrapper = None).traverse(tree)
   }
+
+  override def runLinter(ctx: blackbox.Context)(tree: ctx.Tree): Unit = {
+    import ctx.universe._
+    ctx.inferImplicitValue(weakTypeOf[LinterLevel]) match {
+      case t if t.tpe =:= weakTypeOf[Abort.type] =>
+        impl(ctx)(tree, (pos, msg) => ctx.error(pos, msg))
+      case t if t.tpe =:= weakTypeOf[Warn.type] =>
+        impl(ctx)(tree, (pos, msg) => ctx.warning(pos, msg))
+      case _ => ()
+    }
+  }
 }
 
 object TaskLinterDSL extends BaseTaskLinterDSL {
@@ -190,6 +207,7 @@ object TaskLinterDSLFeedback {
        |  1. Make `$task` evaluation explicit outside of the function body if you don't care about its evaluation.
        |  2. Use a dynamic task to evaluate `$task` and pass that value as a parameter to an anonymous function.
        |  3. 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.
     """.stripMargin
 
   def useOfValueInsideIfExpression(task: String): String =
@@ -201,19 +219,24 @@ object TaskLinterDSLFeedback {
        |  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.
     """.stripMargin
 
   def missingValueForKey(key: String): String =
     s"""${startBold}The key `$key` is not being invoked inside the task definition.$reset
        |
        |$ProblemHeader:  Keys missing `.value` are not initialized and their dependency is not registered.
-       |$SolutionHeader: Replace `$key` by `$key.value` or remove it if unused.
+       |$SolutionHeader:
+       |  1. Replace `$key` by `$key.value` or remove it if unused
+       |  2. Add `import sbt.dsl.LinterLevel.Ignore` to your build file to disable all task linting.
     """.stripMargin
 
   def missingValueForInitialize(expr: String): String =
     s"""${startBold}The setting/task `$expr` is not being invoked inside the task definition.$reset
        |
        |$ProblemHeader:  Settings/tasks missing `.value` are not initialized and their dependency is not registered.
-       |$SolutionHeader: Replace `$expr` by `($expr).value` or remove it if unused.
+       |$SolutionHeader:
+       |  1. Replace `$expr` by `($expr).value` or remove it if unused.
+       |  2. Add `import sbt.dsl.LinterLevel.Ignore` to your build file to disable all task linting.
     """.stripMargin
 }
diff --git a/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala b/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala
new file mode 100644
index 000000000..e29f5b0c1
--- /dev/null
+++ b/main-settings/src/test/scala/sbt/std/TaskConfigSpec.scala
@@ -0,0 +1,76 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.std
+
+import org.scalatest.FunSuite
+import sbt.std.TestUtil._
+
+import scala.tools.reflect.{ FrontEnd, ToolBoxError }
+
+class TaskConfigSpec extends FunSuite {
+  private def expectError(
+      errorSnippet: String,
+      compileOptions: String = "",
+      baseCompileOptions: String = s"-cp $toolboxClasspath",
+  )(code: String) = {
+    val errorMessage = intercept[ToolBoxError] {
+      eval(code, s"$compileOptions $baseCompileOptions")
+      println(s"Test failed -- compilation was successful! Expected:\n$errorSnippet")
+    }.getMessage
+    val userMessage =
+      s"""
+         |FOUND: $errorMessage
+         |EXPECTED: $errorSnippet
+      """.stripMargin
+    assert(errorMessage.contains(errorSnippet), userMessage)
+  }
+  private class CachingToolbox {
+    private[this] val m = scala.reflect.runtime.currentMirror
+    private[this] var _infos: List[FrontEnd#Info] = Nil
+    private[this] val frontEnd = new FrontEnd {
+      override def display(info: Info): Unit = _infos ::= info
+      override def interactive(): Unit = {}
+    }
+
+    import scala.tools.reflect.ToolBox
+    val toolbox = m.mkToolBox(frontEnd, options = s"-cp $toolboxClasspath")
+    def eval(code: String): Any = toolbox.eval(toolbox.parse(code))
+    def infos: List[FrontEnd#Info] = _infos
+  }
+
+  private def taskDef(linterLevelImport: Option[String]): String =
+    s"""
+       |import sbt._
+       |import sbt.Def._
+       |${linterLevelImport.getOrElse("")}
+       |
+       |val fooNeg = taskKey[String]("")
+       |var condition = true
+       |
+       |val barNeg = Def.task[String] {
+       |  if (condition) fooNeg.value
+       |  else ""
+       |}
+      """.stripMargin
+  private val fooNegError = TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg")
+
+  test("Fail on task invocation inside if it is used inside a regular task") {
+    val fooNegError = TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg")
+    expectError(List(fooNegError).mkString("\n"))(taskDef(Some("import sbt.dsl.LinterLevel.Abort")))
+  }
+  test("Succeed if the linter level is set to warn") {
+    val toolbox = new CachingToolbox
+    assert(toolbox.eval(taskDef(None)) == ((): Unit))
+    assert(toolbox.infos.exists(i => i.severity.count == 1 && i.msg.contains(fooNegError)))
+  }
+  test("Succeed if the linter level is set to ignore") {
+    val toolbox = new CachingToolbox
+    assert(toolbox.eval(taskDef(Some("import sbt.dsl.LinterLevel.Ignore"))) == ((): Unit))
+    assert(toolbox.infos.isEmpty)
+  }
+}

From d42b3ee2fdad6427ef1db5bb7b50a94555b4da2c Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Tue, 11 Dec 2018 18:16:10 -0800
Subject: [PATCH 5/6] Make the TaskLinterDSL warn by default

I am generally of the opinion that a linter should not abort progress by
default. I do, however, think that it should be on by default, making
warn a happy compromise.
---
 .../src/main/scala/sbt/dsl/LinterLevel.scala  | 28 +++++++++----------
 .../test/scala/sbt/std/neg/TaskNegSpec.scala  | 14 ++++++++++
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala b/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
index d1c1d28a0..028eecb15 100644
--- a/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
+++ b/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala
@@ -40,29 +40,29 @@ package sbt.dsl
  *   }
  * }}}
  * To make this work, the instances are all defined as implicit case objects. Moreover, the
- * the [[LinterLevel.Abort]] setting is made default by placing [[LinterLevel.Warn]] and
+ * the [[LinterLevel.Warn]] setting is made default by placing [[LinterLevel.Abort]] and
  * [[LinterLevel.Ignore]] in the [[LinterLevelLowPriority]] trait that the [[LinterLevel]]
  * companion object extends.
  */
 sealed trait LinterLevel
 object LinterLevel extends LinterLevelLowPriority {
 
-  /**
-   * Abort the macro expansion if any linter check fails.
-   */
-  implicit case object Abort extends LinterLevel
-}
-
-private[dsl] trait LinterLevelLowPriority {
-
-  /**
-   * Do not perform any linting.
-   */
-  implicit case object Ignore extends LinterLevel
-
   /**
    * Apply the linter but print warnings instead of aborting macro expansion when linter violations
    * are found.
    */
   implicit case object Warn extends LinterLevel
 }
+
+private[dsl] trait LinterLevelLowPriority {
+
+  /**
+   * Abort the macro expansion if any linter check fails.
+   */
+  implicit case object Abort extends LinterLevel
+
+  /**
+   * Do not perform any linting.
+   */
+  implicit case object Ignore extends LinterLevel
+}
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 4093b06f8..c33cc4952 100644
--- a/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala
+++ b/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala
@@ -39,6 +39,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -59,6 +60,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -78,6 +80,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -103,6 +106,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -132,6 +136,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -155,6 +160,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -176,6 +182,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -196,6 +203,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |var condition = true
@@ -215,6 +223,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |val barNeg = taskKey[String]("")
@@ -235,6 +244,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg = taskKey[String]("")
         |
@@ -252,6 +262,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg2 = taskKey[String]("")
         |
@@ -269,6 +280,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg2 = taskKey[String]("")
         |
@@ -289,6 +301,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg3 = taskKey[String]("")
         |def avoidDCE = {println(""); ""}
@@ -308,6 +321,7 @@ class TaskNegSpec extends FunSuite {
       """
         |import sbt._
         |import sbt.Def._
+        |import sbt.dsl.LinterLevel.Abort
         |
         |val fooNeg4 = taskKey[String]("")
         |

From 01b2e86a544038bfaaec403283d04e2d51b7d9b5 Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Tue, 11 Dec 2018 16:24:30 -0800
Subject: [PATCH 6/6] Fix Def cannot be used inside a taskDyn #3110

The illegalReference check did not actually validate whether the illegal
reference actually referred to an M[_] (which is pretty much always
Initialize[_]]). The means by which this failure was induces were fairly
obscure and go through multiple levels of macro transformations that I
attempt to explain in the comment in IllegalReferenceSpec.

Fixes #3110
---
 build.sbt                                     |  1 +
 .../internal/util/appmacro/ContextUtil.scala  | 33 ++++++-
 .../sbt/internal/util/appmacro/Instance.scala |  2 +-
 .../src/main/scala/sbt/std/TaskMacro.scala    |  2 +-
 .../test/scala/sbt/IllegalReferenceSpec.scala | 90 +++++++++++++++++++
 5 files changed, 122 insertions(+), 6 deletions(-)
 create mode 100644 sbt/src/test/scala/sbt/IllegalReferenceSpec.scala

diff --git a/build.sbt b/build.sbt
index 5f3ee5142..50b5f97ec 100644
--- a/build.sbt
+++ b/build.sbt
@@ -621,6 +621,7 @@ lazy val sbtProj = (project in file("sbt"))
       version,
       // WORKAROUND https://github.com/sbt/sbt-buildinfo/issues/117
       BuildInfoKey.map((fullClasspath in Compile).taskValue) { case (ident, cp) => ident -> cp.files },
+      BuildInfoKey.map((dependencyClasspath in Compile).taskValue) { case (ident, cp) => ident -> cp.files },
       classDirectory in Compile,
       classDirectory in Test,
     ),
diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala
index 1f92c8824..e40cdeac9 100644
--- a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala
+++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala
@@ -114,12 +114,24 @@ final class ContextUtil[C <: blackbox.Context](val ctx: C) {
     defs
   }
 
+  /**
+   * A reference is illegal if it is to an M instance defined within the scope of the macro call.
+   * As an approximation, disallow referenced to any local definitions `defs`.
+   */
+  def illegalReference(defs: collection.Set[Symbol], sym: Symbol, mType: Type): Boolean =
+    sym != null && sym != NoSymbol && defs.contains(sym) && {
+      sym match {
+        case m: MethodSymbol => m.returnType.erasure <:< mType
+        case _               => sym.typeSignature <:< mType
+      }
+    }
+
   /**
    * A reference is illegal if it is to an M instance defined within the scope of the macro call.
    * As an approximation, disallow referenced to any local definitions `defs`.
    */
   def illegalReference(defs: collection.Set[Symbol], sym: Symbol): Boolean =
-    sym != null && sym != NoSymbol && defs.contains(sym)
+    illegalReference(defs, sym, weakTypeOf[Any])
 
   type PropertyChecker = (String, Type, Tree) => Boolean
 
@@ -127,15 +139,28 @@ final class ContextUtil[C <: blackbox.Context](val ctx: C) {
    * A function that checks the provided tree for illegal references to M instances defined in the
    *  expression passed to the macro and for illegal dereferencing of M instances.
    */
-  def checkReferences(defs: collection.Set[Symbol], isWrapper: PropertyChecker): Tree => Unit = {
+  def checkReferences(
+      defs: collection.Set[Symbol],
+      isWrapper: PropertyChecker,
+      mType: Type
+  ): Tree => Unit = {
     case s @ ApplyTree(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) =>
-      if (isWrapper(nme.decodedName.toString, tpe.tpe, qual))
+      if (isWrapper(nme.decodedName.toString, tpe.tpe, qual)) {
         ctx.error(s.pos, DynamicDependencyError)
-    case id @ Ident(name) if illegalReference(defs, id.symbol) =>
+      }
+    case id @ Ident(name) if illegalReference(defs, id.symbol, mType) =>
       ctx.error(id.pos, DynamicReferenceError + ": " + name)
     case _ => ()
   }
 
+  @deprecated("1.3.0", "Use that variant that specifies the M instance types to exclude")
+  /**
+   * A function that checks the provided tree for illegal references to M instances defined in the
+   *  expression passed to the macro and for illegal dereferencing of M instances.
+   */
+  def checkReferences(defs: collection.Set[Symbol], isWrapper: PropertyChecker): Tree => Unit =
+    checkReferences(defs, isWrapper, weakTypeOf[Any])
+
   /** Constructs a ValDef with a parameter modifier, a unique name, with the provided Type and with an empty rhs. */
   def freshMethodParameter(tpe: Type): ValDef =
     ValDef(parameterModifiers, freshTermName("p"), TypeTree(tpe), EmptyTree)
diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
index 99e8b2ab7..3f5066d66 100644
--- a/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
+++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/Instance.scala
@@ -124,7 +124,7 @@ object Instance {
     // Local definitions `defs` in the macro.  This is used to ensure references are to M instances defined outside of the macro call.
     // Also `refCount` is the number of references, which is used to create the private, synthetic method containing the body
     val defs = util.collectDefs(tree, isWrapper)
-    val checkQual: Tree => Unit = util.checkReferences(defs, isWrapper)
+    val checkQual: Tree => Unit = util.checkReferences(defs, isWrapper, mttpe.erasure)
 
     type In = Input[c.universe.type]
     var inputs = List[In]()
diff --git a/main-settings/src/main/scala/sbt/std/TaskMacro.scala b/main-settings/src/main/scala/sbt/std/TaskMacro.scala
index bb5cc46fc..650914c7a 100644
--- a/main-settings/src/main/scala/sbt/std/TaskMacro.scala
+++ b/main-settings/src/main/scala/sbt/std/TaskMacro.scala
@@ -493,7 +493,7 @@ object TaskMacro {
       isParserWrapper(n, tpe, tr) || isTaskWrapper(n, tpe, tr)
     val ttree = t.tree
     val defs = util.collectDefs(ttree, isAnyWrapper)
-    val checkQual = util.checkReferences(defs, isAnyWrapper)
+    val checkQual = util.checkReferences(defs, isAnyWrapper, weakTypeOf[Initialize[InputTask[Any]]])
 
     // the Symbol for the anonymous function passed to the appropriate Instance.map/flatMap/pure method
     // this Symbol needs to be known up front so that it can be used as the owner of synthetic vals
diff --git a/sbt/src/test/scala/sbt/IllegalReferenceSpec.scala b/sbt/src/test/scala/sbt/IllegalReferenceSpec.scala
new file mode 100644
index 000000000..b4a0c17d2
--- /dev/null
+++ b/sbt/src/test/scala/sbt/IllegalReferenceSpec.scala
@@ -0,0 +1,90 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt
+
+import org.scalatest.FunSuite
+
+import scala.tools.reflect.{ FrontEnd, ToolBoxError }
+
+class IllegalReferenceSpec extends FunSuite {
+  lazy val toolboxClasspath: String = {
+    val mainClassesDir = buildinfo.TestBuildInfo.classDirectory
+    val testClassesDir = buildinfo.TestBuildInfo.test_classDirectory
+    val depsClasspath = buildinfo.TestBuildInfo.dependencyClasspath
+    mainClassesDir +: testClassesDir +: depsClasspath mkString java.io.File.pathSeparator
+  }
+  def eval(code: String, compileOptions: String = ""): Any = {
+    val m = scala.reflect.runtime.currentMirror
+    import scala.tools.reflect.ToolBox
+    val tb = m.mkToolBox(options = compileOptions)
+    tb.eval(tb.parse(code))
+  }
+  private def expectError(
+      errorSnippet: String,
+      compileOptions: String = "",
+      baseCompileOptions: String = s"-cp $toolboxClasspath",
+  )(code: String) = {
+    val errorMessage = intercept[ToolBoxError] {
+      eval(code, s"$compileOptions $baseCompileOptions")
+      println(s"Test failed -- compilation was successful! Expected:\n$errorSnippet")
+    }.getMessage
+    val userMessage =
+      s"""
+         |FOUND: $errorMessage
+         |EXPECTED: $errorSnippet
+      """.stripMargin
+    assert(errorMessage.contains(errorSnippet), userMessage)
+  }
+  private class CachingToolbox {
+    private[this] val m = scala.reflect.runtime.currentMirror
+    private[this] var _infos: List[FrontEnd#Info] = Nil
+    private[this] val frontEnd = new FrontEnd {
+      override def display(info: Info): Unit = _infos ::= info
+      override def interactive(): Unit = {}
+    }
+
+    import scala.tools.reflect.ToolBox
+    val toolbox = m.mkToolBox(frontEnd, options = s"-cp $toolboxClasspath")
+    def eval(code: String): Any = toolbox.eval(toolbox.parse(code))
+    def infos: List[FrontEnd#Info] = _infos
+  }
+
+  test("Def.sequential should be legal within Def.taskDyn") {
+    val toolbox = new CachingToolbox
+    // This example was taken from @dos65 in https://github.com/sbt/sbt/issues/3110
+    val build =
+      s"""
+         |import sbt._
+         |import Keys._
+         |
+         |Def.taskDyn[Int] {
+         |  // Calling baseDirectory.value will cause the task macros to add a reference to
+         |  // `Def.toITask(sbt.Keys.baseDirectory)`. This, in turn, causes `Def` to be added
+         |  // to a list of local definitions. Later on, we dereference `Def` with
+         |  // `Def.sequential` which used to erroneously cause an illegal dynamic reference.
+         |  baseDirectory.value
+         |  Def.sequential(Def.task(42))
+         |}.dependencies.headOption.map(_.key.label)
+       """.stripMargin
+    assert(toolbox.eval(build) == Some("baseDirectory"))
+    assert(toolbox.infos.isEmpty)
+  }
+  test("Local task defs should be illegal within Def.task") {
+    val build =
+      s"""
+         |import sbt._
+         |import Keys._
+         |
+         |Def.task[Int] {
+         |  def foo = Def.task(5)
+         |  foo.value
+         |}.dependencies.headOption.map(_.key.label)
+       """.stripMargin
+    expectError("Illegal dynamic reference: foo")(build)
+  }
+}