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 97cf76288..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 @@ -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] } @@ -125,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/dsl/LinterLevel.scala b/main-settings/src/main/scala/sbt/dsl/LinterLevel.scala new file mode 100644 index 000000000..028eecb15 --- /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.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 { + + /** + * 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/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala index 7ab117fe7..d560a84a5 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -8,10 +8,11 @@ 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 } -import scala.collection.mutable.{ HashSet => MutableSet } import scala.io.AnsiColor import scala.reflect.macros.blackbox @@ -19,18 +20,32 @@ 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) - 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 +56,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 } } @@ -60,29 +73,29 @@ 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 () } override def traverse(tree: ctx.universe.Tree): Unit = { tree match { - case ap @ Apply(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) => - val shouldIgnore = uncheckedWrappers.contains(ap) - val wrapperName = nme.decodedName.toString + case ap @ Apply(TypeApply(Select(_, name), tpe :: Nil), qual :: Nil) => + val shouldIgnore = uncheckedWrapper.contains(ap) + val wrapperName = name.decodedName.toString val (qualName, isSettingKey) = Option(qual.symbol) .map(sym => (sym.name.decodedName.toString, qual.tpe <:< typeOf[SettingKey[_]])) @@ -91,32 +104,26 @@ 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) 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) { @@ -125,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 => @@ -147,7 +155,18 @@ abstract class BaseTaskLinterDSL extends LinterDSL { } } } - (new traverser).traverse(tree) + 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 _ => () + } } } @@ -177,39 +196,47 @@ 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. + | 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) = + 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. + | 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) = + 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: + | 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) = + 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: + | 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/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/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) + } +} 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]("") | 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) + } +}