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