From 190dc23f7063787e145b1cf1c061259829681504 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 11 Apr 2018 01:31:38 -0400 Subject: [PATCH 1/2] Fixes linter that detects missing .value Fixes #4079 #3216 introduced a linter that checks against missing `.value`, but the tree only checked for `Ident`. This doesn't work because in reality the symbols of build.sbt are transformed to `$somehash.npmInstallTask` where `somehash` is the wrapper object we create. Similarly for the built-in keys, they are presented as `sbt.Keys.compile`. With this change unused task will fail to load the build with the following message: ``` /sbt-4079/build.sbt:26: error: The key `compile` is not being invoked inside the task definition. Problem: Keys missing `.value` are not initialized and their dependency is not registered. Solution: Replace `compile` by `compile.value` or remove it if unused. compile ^ /sbt-4079/build.sbt:27: error: The key `npmInstallTask` is not being invoked inside the task definition. Problem: Keys missing `.value` are not initialized and their dependency is not registered. Solution: Replace `npmInstallTask` by `npmInstallTask.value` or remove it if unused. npmInstallTask ^ ``` --- .../main/scala/sbt/std/TaskLinterDSL.scala | 20 ++++++++++++++----- 1 file changed, 15 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 a328cee70..1c47216fa 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -27,6 +27,7 @@ abstract class BaseTaskLinterDSL extends LinterDSL { private val taskKeyType = typeOf[sbt.TaskKey[_]] private val settingKeyType = typeOf[sbt.SettingKey[_]] private val inputKeyType = typeOf[sbt.InputKey[_]] + private val initializeType = typeOf[sbt.Def.Initialize[_]] private val uncheckedWrappers = MutableSet.empty[Tree] var insideIf: Boolean = false var insideAnon: Boolean = false @@ -57,7 +58,7 @@ abstract class BaseTaskLinterDSL extends LinterDSL { } @inline def isKey(tpe: Type): Boolean = - tpe <:< taskKeyType || tpe <:< settingKeyType || tpe <:< inputKeyType + tpe <:< initializeType || tpe <:< taskKeyType || tpe <:< settingKeyType || tpe <:< inputKeyType def detectAndErrorOnKeyMissingValue(i: Ident): Unit = { if (isKey(i.tpe)) { @@ -66,6 +67,13 @@ abstract class BaseTaskLinterDSL extends LinterDSL { } else () } + def detectAndErrorOnKeyMissingValue(s: Select): Unit = { + if (isKey(s.tpe)) { + val keyName = s.name.decodedName.toString + ctx.error(s.pos, TaskLinterDSLFeedback.missingValueForKey(keyName)) + } else () + } + override def traverse(tree: ctx.universe.Tree): Unit = { tree match { case ap @ Apply(TypeApply(Select(_, nme), tpe :: Nil), qual :: Nil) => @@ -118,11 +126,13 @@ abstract class BaseTaskLinterDSL extends LinterDSL { // 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 s: Select => detectAndErrorOnKeyMissingValue(s) + case _ => () } - case i: Ident => detectAndErrorOnKeyMissingValue(i) - case _ => () + case i: Ident => detectAndErrorOnKeyMissingValue(i) + case s: Select => detectAndErrorOnKeyMissingValue(s) + case t => () } } traverseTrees(stmts) From 06d7be0365cc087a079f47fc236f77ea6eaab00c Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Thu, 26 Apr 2018 04:44:12 -0400 Subject: [PATCH 2/2] handle X / y --- .../main/scala/sbt/std/TaskLinterDSL.scala | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala index 1c47216fa..98e26d4fe 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -24,9 +24,6 @@ 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 initializeType = typeOf[sbt.Def.Initialize[_]] private val uncheckedWrappers = MutableSet.empty[Tree] var insideIf: Boolean = false @@ -57,8 +54,8 @@ abstract class BaseTaskLinterDSL extends LinterDSL { } } - @inline def isKey(tpe: Type): Boolean = - tpe <:< initializeType || tpe <:< taskKeyType || tpe <:< settingKeyType || tpe <:< inputKeyType + @inline def isKey(tpe: Type): Boolean = isInitialize(tpe) + @inline def isInitialize(tpe: Type): Boolean = tpe <:< initializeType def detectAndErrorOnKeyMissingValue(i: Ident): Unit = { if (isKey(i.tpe)) { @@ -74,6 +71,13 @@ abstract class BaseTaskLinterDSL extends LinterDSL { } else () } + def detectAndErrorOnKeyMissingValue(a: Apply): Unit = { + if (isInitialize(a.tpe)) { + val expr = "X / y" + ctx.error(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) => @@ -128,11 +132,13 @@ abstract class BaseTaskLinterDSL extends LinterDSL { rhs match { case i: Ident => detectAndErrorOnKeyMissingValue(i) case s: Select => detectAndErrorOnKeyMissingValue(s) + case a: Apply => detectAndErrorOnKeyMissingValue(a) case _ => () } case i: Ident => detectAndErrorOnKeyMissingValue(i) case s: Select => detectAndErrorOnKeyMissingValue(s) - case t => () + case a: Apply => detectAndErrorOnKeyMissingValue(a) + case _ => () } } traverseTrees(stmts) @@ -171,14 +177,13 @@ 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) = 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}: | 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. @@ -189,7 +194,6 @@ object TaskLinterDSLFeedback { | |${ProblemHeader}: `$task` is inside the if expression of a regular task. | Regular tasks always evaluate task inside the bodies of if expressions. - | |${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. @@ -198,8 +202,14 @@ object TaskLinterDSLFeedback { 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. - | + |${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 + + def missingValueForInitialize(expr: 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. + """.stripMargin }