From b017eaee394e196149e76f756f18f4b9dec39272 Mon Sep 17 00:00:00 2001 From: jvican Date: Sun, 28 May 2017 18:15:47 +0200 Subject: [PATCH] Support DSL detection for nested ifs and anons Before, we were not preserving the value `insideXXX`. This commit makes sure that we handle much more complex scenarios and we report them successfully. Have a look at the tests. --- .../main/scala/sbt/std/TaskLinterDSL.scala | 8 +-- .../test/scala/sbt/std/neg/TaskNegSpec.scala | 54 ++++++++++++++++++- main/src/main/scala/sbt/Defaults.scala | 3 +- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala index 7dcaf2758..9390cee21 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -84,20 +84,22 @@ abstract class BaseTaskLinterDSL extends LinterDSL { traverse(qual) case If(condition, thenp, elsep) => traverse(condition) + val previousInsideIf = insideIf insideIf = true traverse(thenp) traverse(elsep) - insideIf = false + insideIf = previousInsideIf case Typed(expr, tpt: TypeTree) if tpt.original != null => handleUncheckedAnnotation(expr, tpt) traverse(expr) traverse(tpt) case Function(vparams, body) => - super.traverseTrees(vparams) + traverseTrees(vparams) if (!vparams.exists(_.mods.hasFlag(Flag.SYNTHETIC))) { + val previousInsideAnon = insideAnon insideAnon = true traverse(body) - insideAnon = false + insideAnon = previousInsideAnon } else traverse(body) case Block(stmts, expr) => if (!isDynamicTask) { 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 5c0c07b7f..ff9447502 100644 --- a/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala +++ b/main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala @@ -62,7 +62,7 @@ class TaskNegSpec extends FunSuite { } } - test("Fail on task invocation inside inside `if` of task returned by dynamic task") { + test("Fail on task invocation inside `if` of task returned by dynamic task") { expectError(TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg")) { """ |import sbt._ @@ -85,6 +85,37 @@ class TaskNegSpec extends FunSuite { } } + test("Fail on task invocation inside nested `if` of task returned by dynamic task") { + val fooNegCatch = TaskLinterDSLFeedback.useOfValueInsideIfExpression("fooNeg") + val barNegCatch = TaskLinterDSLFeedback.useOfValueInsideIfExpression("barNeg") + expectError(List(fooNegCatch, barNegCatch).mkString("\n")) { + """ + |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) { + | val first = if (!condition && condition) { + | fooNeg.value + | } else "" + | if ("true".toBoolean) first + | else { + | barNeg.value + | } + | } else "" + | } + | } else Def.task("") + |} + """.stripMargin + } + } + test("Fail on task invocation inside else of task returned by dynamic task") { expectError(TaskLinterDSLFeedback.useOfValueInsideIfExpression("barNeg")) { """ @@ -127,6 +158,27 @@ class TaskNegSpec extends FunSuite { } } + test("Fail on task invocation inside nested anonymous function returned by regular task") { + val fooNegError = TaskLinterDSLFeedback.useOfValueInsideAnon("fooNeg") + val barNegError = TaskLinterDSLFeedback.useOfValueInsideAnon("barNeg") + expectError(List(fooNegError, barNegError).mkString("\n")) { + """ + |import sbt._ + |import sbt.Def._ + | + |val fooNeg = taskKey[String]("") + |val barNeg = taskKey[String]("") + |var condition = true + | + |val bazNeg = Def.task[String] { + | val anon = () => { val _ = () => fooNeg.value; barNeg.value} + | if (condition) anon() + | else anon() + |} + """.stripMargin + } + } + test("Fail on task invocation inside complex anonymous function returned by regular task") { val fooNegError = TaskLinterDSLFeedback.useOfValueInsideAnon("fooNeg") expectError(fooNegError) { diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 7933200be..d6a1b4657 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -1186,6 +1186,7 @@ object Defaults extends BuildCommon { val s = streams.value val opts = forkOptions.value val options = javaOptions.value + val trap = trapExit.value if (fork.value) { s.log.debug(s"javaOptions: $options") new ForkRun(opts) @@ -1200,7 +1201,7 @@ object Defaults extends BuildCommon { mask) s.log.warn(s"$showJavaOptions will be ignored, $showFork is set to false") } - new Run(si, trapExit.value, tmp) + new Run(si, trap, tmp) } }