luke
/
sbt
mirror of https://github.com/sbt/sbt.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3225 from scalacenter/improvements-dsl-checks 

[sbt 1.0] Add missing .value DSL check and other improvements
This commit is contained in:
eugene yokota 2017-05-29 13:55:21 -04:00 committed by GitHub
parent 9fc9304638 c85fb95851
commit 13ec092be3
4 changed files with 322 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

130
main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala
View File

@ -5,7 +5,6 @@ import sbt.internal.util.appmacro.{ Convert, Converted, LinterDSL }
import scala.collection.mutable.{ HashSet => MutableSet }
import scala.io.AnsiColor
import scala.reflect.internal.util.Position
import scala.reflect.macros.blackbox
abstract class BaseTaskLinterDSL extends LinterDSL {
@ -17,9 +16,13 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
val isTask = convert.asPredicate(ctx)
class traverser extends Traverser {
private val unchecked = symbolOf[sbt.sbtUnchecked].asClass
private val taskKeyType = typeOf[sbt.TaskKey[_]]
private val settingKeyType = typeOf[sbt.SettingKey[_]]
private val inputKeyType = typeOf[sbt.InputKey[_]]
private val uncheckedWrappers = MutableSet.empty[Tree]
var insideIf: Boolean = false
var insideAnon: Boolean = false
var disableNoValueReport: Boolean = false
def handleUncheckedAnnotation(exprAtUseSite: Tree, tt: TypeTree): Unit = {
tt.original match {
@ -44,42 +47,75 @@ abstract class BaseTaskLinterDSL extends LinterDSL {
}
}
@inline def isKey(tpe: Type): Boolean =
tpe <:< taskKeyType || tpe <:< settingKeyType || tpe <:< inputKeyType
def detectAndErrorOnKeyMissingValue(i: Ident): Unit = {
if (isKey(i.tpe)) {
val keyName = i.name.decodedName.toString
ctx.error(i.pos, TaskLinterDSLFeedback.missingValueForKey(keyName))
} else ()
}
override def traverse(tree: ctx.universe.Tree): Unit = {
tree match {
case s @ Apply(TypeApply(Select(selectQual, nme), tpe :: Nil), qual :: Nil) =>
val shouldIgnore = uncheckedWrappers.contains(s)
case ap @ Apply(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) =>
val shouldIgnore = uncheckedWrappers.contains(ap)
val wrapperName = nme.decodedName.toString
if (!shouldIgnore && isTask(wrapperName, tpe.tpe, qual)) {
val qualName =
if (qual.symbol != null) qual.symbol.name.decodedName.toString
else s.pos.lineContent
else ap.pos.lineContent
if (insideIf && !isDynamicTask) {
// Error on the use of value inside the if of a regular task (dyn task is ok)
ctx.error(s.pos, TaskLinterDSLFeedback.useOfValueInsideIfExpression(qualName))
ctx.error(ap.pos, TaskLinterDSLFeedback.useOfValueInsideIfExpression(qualName))
}
if (insideAnon) {
// Error on the use of anonymous functions in any task or dynamic task
ctx.error(s.pos, TaskLinterDSLFeedback.useOfValueInsideAnon(qualName))
ctx.error(ap.pos, TaskLinterDSLFeedback.useOfValueInsideAnon(qualName))
}
} else traverse(selectQual)
}
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 f @ Function(vparams, body) =>
super.traverseTrees(vparams)
case Function(vparams, body) =>
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) {
/* The missing .value analysis is dumb on purpose because it's expensive.
* Detecting valid use cases of idents whose type is an sbt key is difficult
* 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. */
stmts.foreach {
// TODO: Consider using unused names analysis to be able to report on more cases
case ValDef(_, valName, _, rhs) if valName == termNames.WILDCARD =>
rhs match {
case i: Ident => detectAndErrorOnKeyMissingValue(i)
case _ => ()
}
case i: Ident => detectAndErrorOnKeyMissingValue(i)
case _ => ()
}
}
traverseTrees(stmts)
traverse(expr)
case _ => super.traverse(tree)
}
}
@ -138,69 +174,11 @@ object TaskLinterDSLFeedback {
| 2. Otherwise, make the static evaluation explicit by evaluating `$task` outside the if expression.
""".stripMargin
/* If(
Ident(TermName("condition")),
Typed(
Typed(Apply(TypeApply(Select(Ident(sbt.std.InputWrapper),
TermName("wrapInitTask_$u2603$u2603")),
List(TypeTree())),
List(Ident(TermName("foo")))),
TypeTree()),
TypeTree().setOriginal(
Annotated(
Apply(Select(New(Ident(TypeName("unchecked"))), termNames.CONSTRUCTOR), List()),
Typed(Apply(TypeApply(Select(Ident(sbt.std.InputWrapper),
TermName("wrapInitTask_$u2603$u2603")),
List(TypeTree())),
List(Ident(TermName("foo")))),
TypeTree())
))
),
Typed(
Typed(Apply(TypeApply(Select(Ident(sbt.std.InputWrapper),
TermName("wrapInitTask_$u2603$u2603")),
List(TypeTree())),
List(Ident(TermName("bar")))),
TypeTree()),
TypeTree().setOriginal(
Annotated(
Apply(Select(New(Ident(TypeName("unchecked"))), termNames.CONSTRUCTOR), List()),
Typed(Apply(TypeApply(Select(Ident(sbt.std.InputWrapper),
TermName("wrapInitTask_$u2603$u2603")),
List(TypeTree())),
List(Ident(TermName("bar")))),
TypeTree())
))
)
)*/
/* Block(
List(
ValDef(
Modifiers(),
TermName("anon"),
TypeTree(),
Function(
List(),
Apply(
Select(
Typed(
Apply(TypeApply(Select(Ident(sbt.std.InputWrapper),
TermName("wrapInitTask_$u2603$u2603")),
List(TypeTree())),
List(Ident(TermName("fooNeg")))),
TypeTree()
),
TermName("$plus")
),
List(Literal(Constant("")))
)
)
)),
If(
Ident(TermName("condition")),
Apply(Select(Ident(TermName("anon")), TermName("apply")), List()),
Apply(Select(Ident(TermName("anon")), TermName("apply")), List())
)
)*/
def missingValueForKey(key: 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.
""".stripMargin
}

84
main-settings/src/test/scala/sbt/std/TaskPosSpec.scala
View File

@ -27,6 +27,30 @@ class TaskPosSpec {
}
}
locally {
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
var condition = true
val baz = Def.task[String] {
val fooAnon = () => foo.value: @sbtUnchecked
if (condition) fooAnon()
else fooAnon()
}
}
locally {
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
var condition = true
val baz = Def.task[String] {
val fooAnon = () => (foo.value: @sbtUnchecked) + ""
if (condition) fooAnon()
else fooAnon()
}
}
locally {
import sbt._
import sbt.Def._
@ -66,4 +90,64 @@ class TaskPosSpec {
} else Def.task("")
}
}
locally {
// missing .value error should not happen inside task dyn
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
val baz = Def.taskDyn[String] {
foo
}
}
locally {
// missing .value error should not happen inside task dyn
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
val avoidDCE = ""
val baz = Def.task[String] {
foo: @sbtUnchecked
avoidDCE
}
}
locally {
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
val baz = Def.task[String] {
def inner(s: KeyedInitialize[_]) = println(s)
inner(foo)
""
}
}
locally {
// In theory, this should be reported, but missing .value analysis is dumb at the cost of speed
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
def avoidDCE = { println(""); "" }
val baz = Def.task[String] {
val (_, _) = "" match {
case _ => (foo, 1 + 2)
}
avoidDCE
}
}
locally {
import sbt._
import sbt.Def._
val foo = taskKey[String]("")
def avoidDCE = { println(""); "" }
val baz = Def.task[String] {
val hehe = foo
// We do not detect `hehe` because guessing that the user did the wrong thing would require
// us to run the unused name traverser defined in Typer (and hence proxy it from context util)
avoidDCE
}
}
}

187
main-settings/src/test/scala/sbt/std/neg/TaskNegSpec.scala
View File

@ -7,19 +7,17 @@ import sbt.std.TestUtil._
class TaskNegSpec extends FunSuite {
import tools.reflect.ToolBoxError
def expectError(errorSnippet: String,
compileOptions: String = "-Xfatal-warnings",
compileOptions: String = "",
baseCompileOptions: String = s"-cp $toolboxClasspath")(code: String) = {
val errorMessage = intercept[ToolBoxError] {
eval(code, s"$compileOptions $baseCompileOptions")
println("SUCCESS")
println(s"Test failed -- compilation was successful! Expected:\n$errorSnippet")
}.getMessage
println(errorMessage)
val userMessage =
s"""
|FOUND: $errorMessage
|EXPECTED: $errorSnippet
""".stripMargin
println(userMessage)
assert(errorMessage.contains(errorSnippet), userMessage)
}
@ -64,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._
@ -87,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")) {
"""
@ -129,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) {
@ -168,4 +218,131 @@ class TaskNegSpec extends FunSuite {
""".stripMargin
}
}
test("Detect a missing `.value` inside a task") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg = taskKey[String]("")
|
|def avoidDCE = {println(""); ""}
|val bazNeg = Def.task[String] {
| fooNeg
| avoidDCE
|}
""".stripMargin
}
}
test("Detect a missing `.value` inside a val definition of a task") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg2")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg2 = taskKey[String]("")
|
|def avoidDCE = {println(""); ""}
|val bazNeg = Def.task[String] {
| val _ = fooNeg2
| avoidDCE
|}
""".stripMargin
}
}
test("Detect a missing `.value` inside a val definition of an inner method of a task") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg2")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg2 = taskKey[String]("")
|
|def avoidDCE = {println(""); ""}
|val bazNeg = Def.task[String] {
| def inner = {
| val _ = fooNeg2
| avoidDCE
| }
| inner
|}
""".stripMargin
}
}
test("Detect a missing `.value` inside an inner method of a task") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg3")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg3 = taskKey[String]("")
|def avoidDCE = {println(""); ""}
|val bazNeg = Def.task[String] {
| def inner: String = {
| fooNeg3
| avoidDCE
| }
| inner
|}
""".stripMargin
}
}
test("Detect a missing `.value` inside a task whose return type is Unit") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg4")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg4 = taskKey[String]("")
|
|val bazNeg = Def.task[Unit] {
| fooNeg4
|}
""".stripMargin
}
}
// Enable these tests when https://github.com/scala/bug/issues/10340 is fixed
/*
test("Detect a missing `.value` inside a val of an inner method of a task returning a literal") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg3")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg3 = taskKey[String]("")
|
|val bazNeg = Def.task[String] {
| def inner: String = {
| val _ = fooNeg3
| ""
| }
| inner
|}
""".stripMargin
}
}
test("Detect a missing `.value` inside a val of a task returning a literal") {
expectError(TaskLinterDSLFeedback.missingValueForKey("fooNeg3")) {
"""
|import sbt._
|import sbt.Def._
|
|val fooNeg3 = taskKey[String]("")
|
|val bazNeg = Def.task[String] {
| val _ = fooNeg3
| ""
|}
""".stripMargin
}
}
*/
}

3
main/src/main/scala/sbt/Defaults.scala
View File

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