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 993072ab9..6bdbb2250 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 @@ -115,8 +115,7 @@ 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): Tree => Unit = { case s @ ApplyTree(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) => if (isWrapper(nme.decodedName.toString, tpe.tpe, qual)) ctx.error(s.pos, DynamicDependencyError) @@ -125,28 +124,6 @@ final class ContextUtil[C <: blackbox.Context](val ctx: C) { case _ => () } - class TaskMacroViolationDiscovery(isInvalid: PropertyChecker) extends Traverser { - var insideIf: Boolean = false - override def traverse(tree: ctx.universe.Tree): Unit = { - tree match { - case If(condition, thenp, elsep) => - super.traverse(condition) - insideIf = true - super.traverse(thenp) - super.traverse(elsep) - insideIf = false - case s @ ApplyTree(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) => - if (insideIf && isInvalid(nme.decodedName.toString, tpe.tpe, qual)) { - ctx.error(s.pos, "DSL error: a task value cannot be obtained inside if.") - } - case _ => super.traverse(tree) - } - } - } - - def checkMacroViolations(tree: Tree, isInvalid: PropertyChecker) = - new TaskMacroViolationDiscovery(isInvalid).traverse(tree) - /** 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 9a36e3ff8..3ffbb7d48 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 @@ -81,13 +81,16 @@ object Instance { c: blackbox.Context, i: Instance with Singleton, convert: Convert, - builder: TupleBuilder)(t: Either[c.Expr[T], c.Expr[i.M[T]]], inner: Transform[c.type, N])( + builder: TupleBuilder, + linter: LinterDSL + )( + t: Either[c.Expr[T], c.Expr[i.M[T]]], + inner: Transform[c.type, N] + )( implicit tt: c.WeakTypeTag[T], nt: c.WeakTypeTag[N[T]], it: c.TypeTag[i.type] ): c.Expr[i.M[N[T]]] = { - println("STARTING CONTIMPL") - println("=================") import c.universe.{ Apply => ApplyTree, _ } val util = ContextUtil[c.type](c) @@ -99,8 +102,6 @@ object Instance { case Left(l) => (l.tree, nt.tpe.dealias) case Right(r) => (r.tree, mttpe) } - println("TREE TYPE") - println(treeType) // 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 val functionSym = util.functionSymbol(tree.pos) @@ -187,17 +188,10 @@ object Instance { } // applies the transformation - println("TREE") - println(tree) - import util.PropertyChecker - val isInvalid: PropertyChecker = - (s: String, tpe: Type, tree: Tree) => t.isLeft && convert.asPredicate(c).apply(s, tpe, tree) - util.checkMacroViolations(tree, isInvalid) + linter.runLinter(c)(tree, t.isLeft) val tx = util.transformWrappers(tree, (n, tpe, t, replace) => sub(n, tpe, t, replace)) // resetting attributes must be: a) local b) done here and not wider or else there are obscure errors val tr = makeApp(inner(tx)) - println("FINAL TREE") - println(tr) c.Expr[i.M[N[T]]](tr) } diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/LinterDSL.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/LinterDSL.scala new file mode 100644 index 000000000..fc87e7316 --- /dev/null +++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/LinterDSL.scala @@ -0,0 +1,16 @@ +package sbt.internal.util.appmacro + +import scala.reflect.macros.blackbox + +trait LinterDSL { + def runLinter(ctx: blackbox.Context)(tree: ctx.Tree, isApplicableFromContext: => Boolean): Unit +} + +object LinterDSL { + object Empty extends LinterDSL { + override def runLinter(ctx: blackbox.Context)( + tree: ctx.Tree, + isApplicableFromContext: => Boolean + ): Unit = () + } +} diff --git a/main-settings/src/main/scala/sbt/std/SettingMacro.scala b/main-settings/src/main/scala/sbt/std/SettingMacro.scala index 7b5679769..f9643483a 100644 --- a/main-settings/src/main/scala/sbt/std/SettingMacro.scala +++ b/main-settings/src/main/scala/sbt/std/SettingMacro.scala @@ -2,9 +2,16 @@ package sbt package std import Def.Initialize -import sbt.internal.util.Types.{ idFun, Id } +import sbt.internal.util.Types.{ Id, idFun } import sbt.internal.util.AList -import sbt.internal.util.appmacro.{ Convert, Converted, Instance, MixedBuilder, MonadInstance } +import sbt.internal.util.appmacro.{ + Convert, + Converted, + Instance, + LinterDSL, + MixedBuilder, + MonadInstance +} object InitializeInstance extends MonadInstance { type M[x] = Initialize[x] @@ -41,15 +48,16 @@ object InitializeConvert extends Convert { } object SettingMacro { + import LinterDSL.{ Empty => EmptyLinter } def settingMacroImpl[T: c.WeakTypeTag](c: blackbox.Context)( t: c.Expr[T]): c.Expr[Initialize[T]] = - Instance.contImpl[T, Id](c, InitializeInstance, InitializeConvert, MixedBuilder)( + Instance.contImpl[T, Id](c, InitializeInstance, InitializeConvert, MixedBuilder, EmptyLinter)( Left(t), Instance.idTransform[c.type]) def settingDynMacroImpl[T: c.WeakTypeTag](c: blackbox.Context)( t: c.Expr[Initialize[T]]): c.Expr[Initialize[T]] = - Instance.contImpl[T, Id](c, InitializeInstance, InitializeConvert, MixedBuilder)( + Instance.contImpl[T, Id](c, InitializeInstance, InitializeConvert, MixedBuilder, EmptyLinter)( Right(t), Instance.idTransform[c.type]) } diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala new file mode 100644 index 000000000..47158753b --- /dev/null +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -0,0 +1,83 @@ +package sbt.std + +import sbt.internal.util.ConsoleAppender +import sbt.internal.util.appmacro.LinterDSL + +import scala.collection.mutable.{ HashSet => MutableSet } +import scala.io.AnsiColor +import scala.reflect.macros.blackbox + +object TaskLinterDSL extends LinterDSL { + override def runLinter(ctx: blackbox.Context)( + tree: ctx.Tree, + isApplicableFromContext: => Boolean + ): Unit = { + import ctx.universe._ + val isTask = FullConvert.asPredicate(ctx) + object traverser extends Traverser { + private val unchecked = symbolOf[sbt.unchecked].asClass + private val uncheckedWrappers = MutableSet.empty[Tree] + var insideIf: Boolean = false + override def traverse(tree: ctx.universe.Tree): Unit = { + tree match { + case If(condition, thenp, elsep) => + super.traverse(condition) + insideIf = true + super.traverse(thenp) + super.traverse(elsep) + insideIf = false + case s @ Apply(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) => + val shouldIgnore = uncheckedWrappers.contains(s) + val wrapperName = nme.decodedName.toString + if (insideIf && !shouldIgnore && isTask(wrapperName, tpe.tpe, qual)) { + val qualName = qual.symbol.name.decodedName.toString + ctx.error(s.pos, TaskLinterDSLFeedback.useOfValueInsideIfExpression(qualName)) + } + // Don't remove, makes the whole analysis work + case Typed(expr, tt) => + super.traverse(expr) + super.traverse(tt) + case Typed(expr, tt: TypeTree) if tt.original != null => + tt.original match { + case Annotated(annot, arg) => + annot.tpe match { + case AnnotatedType(annotations, _) => + val symAnnotations = annotations.map(_.tree.tpe.typeSymbol) + val isUnchecked = symAnnotations.contains(unchecked) + if (isUnchecked) { + val toAdd = arg match { + case Typed(`expr`, _) => `expr` + case `tree` => `tree` + } + uncheckedWrappers.add(toAdd) + } + case _ => + } + } + super.traverse(expr) + case tree => super.traverse(tree) + } + } + } + if (!isApplicableFromContext) () + else traverser.traverse(tree) + } +} + +object TaskLinterDSLFeedback { + private final val startBold = if (ConsoleAppender.formatEnabled) AnsiColor.BOLD else "" + private final val startRed = if (ConsoleAppender.formatEnabled) AnsiColor.RED else "" + private final val startGreen = if (ConsoleAppender.formatEnabled) AnsiColor.GREEN else "" + private final val reset = if (ConsoleAppender.formatEnabled) AnsiColor.RESET else "" + + def useOfValueInsideIfExpression(offendingValue: String) = + s"""${startBold}The evaluation of `${offendingValue}` happens always inside a regular task.$reset + | + |${startRed}PROBLEM${reset}: `${offendingValue}` is inside the if expression of a regular task. + | Regular tasks always evaluate task inside the bodies of if expressions. + | + |${startGreen}SOLUTION${reset}: + | 1. If you only want to evaluate it when the if predicate is true, use a dynamic task. + | 2. Otherwise, make the static evaluation explicit by evaluating `${offendingValue}` outside the if expression. + """.stripMargin +} diff --git a/main-settings/src/main/scala/sbt/std/TaskMacro.scala b/main-settings/src/main/scala/sbt/std/TaskMacro.scala index 5139a0a17..5b6adfecf 100644 --- a/main-settings/src/main/scala/sbt/std/TaskMacro.scala +++ b/main-settings/src/main/scala/sbt/std/TaskMacro.scala @@ -2,8 +2,15 @@ package sbt package std import Def.{ Initialize, Setting } -import sbt.internal.util.Types.{ const, idFun, Id } -import sbt.internal.util.appmacro.{ ContextUtil, Converted, Instance, MixedBuilder, MonadInstance } +import sbt.internal.util.Types.{ Id, const, idFun } +import sbt.internal.util.appmacro.{ + ContextUtil, + Converted, + Instance, + LinterDSL, + MixedBuilder, + MonadInstance +} import Instance.Transform import sbt.internal.util.complete.{ DefaultParsers, Parser } import sbt.internal.util.{ AList, LinePosition, NoPosition, SourcePosition } @@ -82,15 +89,17 @@ object TaskMacro { """`<<=` operator is deprecated. Use `key := { x.value }` or `key ~= (old => { newValue })`. |See http://www.scala-sbt.org/0.13/docs/Migrating-from-sbt-012x.html""".stripMargin + import LinterDSL.{ Empty => EmptyLinter } + 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)( + Instance.contImpl[T, Id](c, FullInstance, FullConvert, MixedBuilder, TaskLinterDSL)( Left(t), Instance.idTransform[c.type]) def taskDynMacroImpl[T: c.WeakTypeTag](c: blackbox.Context)( t: c.Expr[Initialize[Task[T]]]): c.Expr[Initialize[Task[T]]] = - Instance.contImpl[T, Id](c, FullInstance, FullConvert, MixedBuilder)( + Instance.contImpl[T, Id](c, FullInstance, FullConvert, MixedBuilder, EmptyLinter)( Right(t), Instance.idTransform[c.type]) @@ -356,7 +365,9 @@ object TaskMacro { } val cond = c.Expr[T](conditionInputTaskTree(c)(t.tree)) Instance - .contImpl[T, M](c, InitializeInstance, InputInitConvert, MixedBuilder)(Left(cond), inner) + .contImpl[T, M](c, InitializeInstance, InputInitConvert, MixedBuilder, EmptyLinter)( + Left(cond), + inner) } private[this] def conditionInputTaskTree(c: blackbox.Context)(t: c.Tree): c.Tree = { @@ -397,13 +408,17 @@ object TaskMacro { val inner: Transform[c.type, M] = new Transform[c.type, M] { def apply(in: c.Tree): c.Tree = f(c.Expr[T](in)).tree } - Instance.contImpl[T, M](c, ParserInstance, ParserConvert, MixedBuilder)(Left(t), inner) + Instance.contImpl[T, M](c, ParserInstance, ParserConvert, MixedBuilder, LinterDSL.Empty)( + Left(t), + inner) } private[this] def iTaskMacro[T: c.WeakTypeTag](c: blackbox.Context)( t: c.Expr[T]): c.Expr[Task[T]] = Instance - .contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder)(Left(t), Instance.idTransform) + .contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder, EmptyLinter)( + Left(t), + Instance.idTransform) private[this] def inputTaskDynMacro0[T: c.WeakTypeTag](c: blackbox.Context)( t: c.Expr[Initialize[Task[T]]]): c.Expr[Initialize[InputTask[T]]] = { @@ -488,13 +503,13 @@ object TaskMacro { object PlainTaskMacro { def task[T](t: T): Task[T] = macro taskImpl[T] def taskImpl[T: c.WeakTypeTag](c: blackbox.Context)(t: c.Expr[T]): c.Expr[Task[T]] = - Instance.contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder)( + Instance.contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder, TaskLinterDSL)( Left(t), Instance.idTransform[c.type]) def taskDyn[T](t: Task[T]): Task[T] = macro taskDynImpl[T] def taskDynImpl[T: c.WeakTypeTag](c: blackbox.Context)(t: c.Expr[Task[T]]): c.Expr[Task[T]] = - Instance.contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder)( + Instance.contImpl[T, Id](c, TaskInstance, TaskConvert, MixedBuilder, LinterDSL.Empty)( Right(t), Instance.idTransform[c.type]) } diff --git a/main-settings/src/main/scala/sbt/unchecked.scala b/main-settings/src/main/scala/sbt/unchecked.scala new file mode 100644 index 000000000..7599e9bb8 --- /dev/null +++ b/main-settings/src/main/scala/sbt/unchecked.scala @@ -0,0 +1,13 @@ +package sbt + +import scala.annotation.Annotation + +/** An annotation to designate that the annotated entity + * should not be considered for additional sbt compiler checks. + * These checks ensure that the DSL is predictable and prevents + * users from doing dangerous things at the cost of a stricter + * code structure. + * + * @since 1.0.0 + */ +class unchecked extends Annotation diff --git a/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala b/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala index c97ce92ba..9cf91e080 100644 --- a/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala +++ b/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala @@ -26,4 +26,16 @@ class TaskPosSpec { else bar } } + + locally { + import sbt._ + import sbt.Def._ + val foo = taskKey[String]("") + val bar = taskKey[String]("") + var condition = true + val baz = Def.task[String] { + if (condition) foo.value: @unchecked + else bar.value: @unchecked + } + } } diff --git a/main-settings/src/test/scala/sbt/std/UsageTest.scala b/main-settings/src/test/scala/sbt/std/UsageTest.scala index 5e149b173..106ff6e4e 100644 --- a/main-settings/src/test/scala/sbt/std/UsageTest.scala +++ b/main-settings/src/test/scala/sbt/std/UsageTest.scala @@ -2,7 +2,7 @@ package sbt.std import sbt.internal.util.complete import sbt.internal.util.complete.DefaultParsers -import sbt.{Def, InputTask, Task} +import sbt.{ Def, InputTask, Task } /*object UseTask { @@ -21,7 +21,7 @@ import sbt.{Def, InputTask, Task} object Assign { import java.io.File - import Def.{Initialize, inputKey, macroValueT, parserToInput, settingKey, taskKey} + import Def.{ Initialize, inputKey, macroValueT, parserToInput, settingKey, taskKey } // import UseTask.{x,y,z,a,set,plain} val ak = taskKey[Int]("a") 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 17f16d3f8..1d238af45 100644 --- a/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala +++ b/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala @@ -1,29 +1,29 @@ package sbt.std.neg import org.scalatest.FunSuite +import sbt.std.TaskLinterDSLFeedback import sbt.std.TestUtil._ class TaskNegSpec extends FunSuite { import tools.reflect.ToolBoxError def expectError(errorSnippet: String, - compileOptions: String = "-Xmacro-settings:debug-spores", + compileOptions: String = "-Xfatal-warnings", baseCompileOptions: String = s"-cp $toolboxClasspath")(code: String) = { val errorMessage = intercept[ToolBoxError] { eval(code, s"$compileOptions $baseCompileOptions") - println("ERROR: The snippet compiled successfully.") }.getMessage val userMessage = s""" |FOUND: $errorMessage |EXPECTED: $errorSnippet """.stripMargin - println(userMessage) assert(errorMessage.contains(errorSnippet), userMessage) } - test("Fail on task invocation inside if of regular task") { - - expectError("DSL error: a task value cannot be obtained inside if.") { + test("Fail on task invocation inside if it is used inside a regular task") { + val fooNegError = TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg") + val barNegError = TaskLinterDSLFeedback.useOfValueInsideIfExpression("barNeg") + expectError(List(fooNegError, barNegError).mkString("\n")) { """ |import sbt._ |import sbt.Def._ @@ -32,11 +32,56 @@ class TaskNegSpec extends FunSuite { |val barNeg = taskKey[String]("") |var condition = true | - |val bazNeg: Initialize[Task[String]] = Def.task[String] { + |val bazNeg = Def.task[String] { | if (condition) fooNeg.value | else barNeg.value |} """.stripMargin } } + + test("Fail on task invocation inside inside if of task returned by dynamic task") { + expectError(TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg")) { + """ + |import sbt._ + |import sbt.Def._ + | + |val fooNeg = taskKey[String]("") + |val barNeg = taskKey[String]("") + |var condition = true + | + |val bazNeg = Def.taskDyn[String] { + | if (condition) { + | Def.task { + | if (condition) { + | fooNeg.value + | } else "" + | } + | } else Def.task("") + |} + """.stripMargin + } + } + + test("Fail on task invocation inside inside else of task returned by dynamic task") { + expectError(TaskLinterDSLFeedback.useOfValueInsideIfExpression("barNeg")) { + """ + |import sbt._ + |import sbt.Def._ + | + |val fooNeg = taskKey[String]("") + |val barNeg = taskKey[String]("") + |var condition = true + | + |val bazNeg = Def.taskDyn[String] { + | if (condition) { + | Def.task { + | if (condition) "" + | else barNeg.value + | } + | } else Def.task("") + |} + """.stripMargin + } + } } diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 558f7d15f..ff0f1879a 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -1867,8 +1867,9 @@ object Classpaths { // Override the default to handle mixing in the sbtPlugin + scala dependencies. allDependencies := { val base = projectDependencies.value ++ libraryDependencies.value + val dependency = sbtDependency.value val pluginAdjust = - if (sbtPlugin.value) sbtDependency.value.withConfigurations(Some(Provided.name)) +: base + if (sbtPlugin.value) dependency.withConfigurations(Some(Provided.name)) +: base else base if (scalaHome.value.isDefined || ivyScala.value.isEmpty || !managedScalaInstance.value) pluginAdjust @@ -2124,11 +2125,11 @@ object Classpaths { } } - val evictionOptions = { + val evictionOptions = Def.taskDyn { if (executionRoots.value.exists(_.key == evicted.key)) - EvictionWarningOptions.empty - else (evictionWarningOptions in update).value - } + Def.task(EvictionWarningOptions.empty) + else Def.task((evictionWarningOptions in update).value) + }.value LibraryManagement.cachedUpdate( s.cacheStoreFactory.sub(updateCacheName.value), diff --git a/project/NightlyPlugin.scala b/project/NightlyPlugin.scala index 58c1917dc..a7eb76b5c 100644 --- a/project/NightlyPlugin.scala +++ b/project/NightlyPlugin.scala @@ -10,7 +10,8 @@ object NightlyPlugin extends AutoPlugin { val includeTestDependencies = settingKey[Boolean]("Doesn't declare test dependencies.") def testDependencies = libraryDependencies ++= ( - if (includeTestDependencies.value) Seq(scalaCheck % Test, specs2 % Test, junit % Test, scalatest % Test) + if (includeTestDependencies.value) + Seq(scalaCheck % Test, specs2 % Test, junit % Test, scalatest % Test) else Seq() ) } diff --git a/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt b/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt index 75a14e146..21876e1e5 100644 --- a/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt +++ b/sbt/src/sbt-test/source-dependencies/trait-member-modified/build.sbt @@ -3,11 +3,12 @@ import xsbti.Maybe import xsbti.compile.{PreviousResult, CompileAnalysis, MiniSetup} previousCompile in Compile := { + val previous = (previousCompile in Compile).value if (!CompileState.isNew) { val res = new PreviousResult(Maybe.nothing[CompileAnalysis], Maybe.nothing[MiniSetup]) CompileState.isNew = true res - } else (previousCompile in Compile).value + } else previous } /* Performs checks related to compilations: diff --git a/sbt/src/sbt-test/source-dependencies/transitive-memberRef/build.sbt b/sbt/src/sbt-test/source-dependencies/transitive-memberRef/build.sbt index 7f4c0fd78..d04e2c0fd 100644 --- a/sbt/src/sbt-test/source-dependencies/transitive-memberRef/build.sbt +++ b/sbt/src/sbt-test/source-dependencies/transitive-memberRef/build.sbt @@ -6,11 +6,12 @@ logLevel := Level.Debug // Reset compile status because scripted tests are run in batch mode previousCompile in Compile := { + val previous = (previousCompile in Compile).value if (!CompileState.isNew) { val res = new PreviousResult(Maybe.nothing[CompileAnalysis], Maybe.nothing[MiniSetup]) CompileState.isNew = true res - } else (previousCompile in Compile).value + } else previous } // disable sbt's heuristic which recompiles everything in case