The user should be able to configure whether or not the task linting is
strictly enforced. In my opinion, the linter is generally pretty good
about warning users that a particular definition may not behave the way
the user expects. Unfortunately, it is fairly common that the user
explicitly wants this behavior and making the linter abort compilation
is a frustrating user experience.
To fix this, I add the LinterLevel trait to a new sbt.dsl package in the
core-macros project. The user can configure the linter to:
1) abort when an issue is detected. This is the current default behavior.
2) print a warning when an issues is detected
3) skip linting all together
Linter configuration is managed by importing the corresponding implicit
object from the LinterLevel companion object.
Fixes#3266
There are many cases where one would want to force evaluation of the
task even when contained in a lambda (see
https://github.com/sbt/sbt/issues/3266). The @sbtUnchecked annotation is
one way to disable the linter that prevents this, but it is obscure. If
the annotation is to exist, I think it should be presented as a
solution.
I found it somewhat difficult to reason about the state of the tree
Traverser because of the usage of mutable variables and data structures.
For example, I determined that the uncheckedWrappers were never used
non-locally so a Set didn't seem to be the right data structure. It was
reasonably straightforward to switch to a more functional style by
parameterizing the method local traverser class.
Bonus:
- remove unused variable disableNoValueReport
- add scaladoc to document the input parameters of the traverser class
so that it's a bit easier to understand for future maintainers.
This cleans up all of the yellow intellij warnings for me. It still
complains about some typos, but those manifest as green squiggled lines
which are less annoying.
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
^
```
previously I was using separate thread in a forked test to test the server, but that is not enough isolation to run multiple server tests.
This adds `RunFromSourceMain.fork(workingDirectory: File)`, which allows us to run a fresh sbt on the given working directory. Next, I've refactored the stateful client-side buffer to a class `TestServer`.
* 1.1.x:
Use Java's redirectInput rather than sys.process's connectInput
Re-write toolboxClasspath to use sbt-buildinfo
Cleanup generateToolboxClasspath
Upgrade to sbt-buildinfo 0.8.0
Fix how fullClasspath is defined in TestBuildInfo
delete buildinfo.BuildInfo from sbt main
Conflicts:
project/plugins.sbt
Ref #3606, #3611, and #3613
This removes unnecessary thunk for slash syntax.
The semantics using this approach is strictly better than the previous `in (ref, config, task)`. By removing the thunk, we retain `(a / b) / c == a / b / c`.
See the following example:
```scala
scala> import sbt._, Keys._
scala> val t: TaskKey[Unit] = (test in Test)
t: sbt.TaskKey[Unit] = TaskKey(This / Select(ConfigKey(test)) / This / test)
scala> ThisBuild / t
ThisBuild / t
res1: sbt.TaskKey[Unit] = TaskKey(Select(ThisBuild) / Select(ConfigKey(test)) / This / test)
scala> ThisBuild / t / name
ThisBuild / t / name
res2: sbt.SettingKey[String] = SettingKey(Select(ThisBuild) / Select(ConfigKey(test)) / Select(test) / name)
```
so far so good? Now look at this:
```
scala> scala> name in (ThisBuild, t)
name in (ThisBuild, t)
res3: sbt.SettingKey[String] = SettingKey(Select(ThisBuild) / This / Select(test) / name)
```
`Test` configuration knowledge is lost! For `in (..)` maybe it was ok because mostly we don't use unscoped keys, but that's the difference between `in (..)` and `/`.
Fixes#3605
In ca71b4b902 I went about fixing the
inexhaustive matching in Scope's resolveProjectBuild and
resolveProjectRef. Looking back the change was wrong.
For resolveProjectBuild the new implementation is less wrong, but still
not great, seeing as it doesn't actually do any build resolving.
For resolveProjectRef the new implementation now blows up instead of
lies. Which means it's less leneant, more "fail-fast".
isProjectThis is unused; remnant of the pre-AutoPlugin days when build
settings where defined in Plugin.settings.
toString added for REPL testing:
```
scala> Zero / Zero / Zero / name
res0: sbt.SlashSyntax.ScopeAndKey[sbt.SettingKey[String]] = Zero / Zero / Zero / name
```
```
Provided by:
ProjectRef(uri("...."), "root") / Test / test
Dependencies:
Test / executeTests
Test / test / streams
Test / state
Test / test / testResultLogger
```
Fixessbt/sbt#1812
This adds unified slash syntax for both sbt shell and the build.sbt DSL.
Instead of the current `<project-id>/config:intask::key`,
this adds `<project-id>/<config-ident>/intask/key` where <config-ident> is the Scala identifier notation for the configurations like `Compile` and `Test`.
This also adds a series of implicits called `SlashSyntax` that adds `/` operators to project refererences, configuration, and keys such that the same syntax works in build.sbt.
These examples work for both from the shell and in build.sbt.
Global / cancelable
ThisBuild / scalaVersion
Test / test
root / Compile / compile / scalacOptions
ProjectRef(uri("file:/xxx/helloworld/"),"root")/Compile/scalacOptions
Zero / Zero / name
The inspect command now outputs something that can be copy-pasted:
> inspect compile
[info] Task: sbt.inc.Analysis
[info] Description:
[info] Compiles sources.
[info] Provided by:
[info] ProjectRef(uri("file:/xxx/helloworld/"),"root")/Compile/compile
[info] Defined at:
[info] (sbt.Defaults) Defaults.scala:326
[info] Dependencies:
[info] Compile/manipulateBytecode
[info] Compile/incCompileSetup
[info] Reverse dependencies:
[info] Compile/printWarnings
[info] Compile/products
[info] Compile/discoveredSbtPlugins
[info] Compile/discoveredMainClasses
[info] Delegates:
[info] Compile/compile
[info] compile
[info] ThisBuild/Compile/compile
[info] ThisBuild/compile
[info] Zero/Compile/compile
[info] Global/compile
[info] Related:
[info] Test/compile
Calling `.value` on a SettingKey doesn't trigger any execution and
doesn't have any side effect, so we can safely allow calls to `.value`
inside conditionals and lambdas.
Fixes#3299
This undeprecates the syntax, but at the same times moves it out of
implicit scope, therefore requiring a 'import TupleSyntax._' to opt-in
to the old syntax.
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.
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 val definitions with `_` as name and idents in statement position
inside blocks.
In the future, we can cover all val definitions no matter what their
name is. Unfortunately, doing so will come at the cost of speed: we have
to run the unused name analysis in `TypeDiagnostics` and expose it from
the power context in `ContextUtil`.
This is good enough for now. If users express interest in having a
smarter analysis, we can always consider trying the unused name
analysis. I am not sure how slow it will be -- hopefully it won't be
that much.
This commit does the following things:
* Removes the boolean from the instance context passes to the linter.
* Prohibits the use of value inside anonymous functions.
* Improves the previous check of `value` inside if.
The improvements have occurred thanks to the fix of an oversight in the
traverser. As a result, several implementation of tasks have been
rewritten because of new compilation failures by both checks.
Note that the new check that prohibits the use of value inside anonymous
functions ignores all the functions whose parameters have been
synthesized by scalac (that can happen in a number of different
scenarios, like for comprehensions). Other scripted tests have also been
fixed.
Running `.value` inside an anonymous function yields the following
error:
```
[error] /data/rw/code/scala/sbt/main-settings/src/test/scala/sbt/std/TaskPosSpec.scala:50:24: The evaluation of `foo` inside an anonymous function is prohibited.
[error]
[error] Problem: Task invocations inside anonymous functions are evaluated independently of whether the anonymous function is invoked or not.
[error]
[error] Solution:
[error] 1. Make `foo` evaluation explicit outside of the function body if you don't care about its evaluation.
[error] 2. Use a dynamic task to evaluate `foo` and pass that value as a parameter to an anonymous function.
[error]
[error] val anon = () => foo.value + " "
[error] ^
```
`.value` inside the if of a regular task is unsafe. The wrapping task
will always execute the value, no matter what the if predicate yields.
This commit adds the infrastructure to lint code for every sbt DSL
macro. It also adds example of neg tests that check that the DSL checks
are in place.
The sbt checks yield error for this specific case because we may want to
explore changing this behaviour in the future. The solutions to this are
straightforward and explained in the error message, that looks like
this:
```
EXPECTED: The evaluation of `fooNeg` happens always inside a regular task.
PROBLEM: `fooNeg` is inside the if expression of a regular task.
Regular tasks always evaluate task inside the bodies of if expressions.
SOLUTION:
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 `fooNeg` outside the if expression.
```
Aside from those solutions, this commit also adds a way to disable any
DSL check by using the new `sbt.unchecked` annotation. This annotation,
similar to `scala.annotation.unchecked` disables compiler output. In our
case, it will disable any task dsl check, making it silent.
Examples of positive checks have also been added.
There have been only two places in `Defaults.scala` where this check has
made compilation fail.
The first one is inside `allDependencies`. To ensure that we still have
static dependencies for `allDependencies`, I have hoisted up the value
invocation outside the if expression. We may want to explore adding a
dynamic task in the future, though. We are doing unnecessary work there.
The second one is inside `update` and is not important because it's not
exposed to the user. We use a `taskDyn`.
Ethan Atkins
Eugene Yokota
xuwei-k
Dale Wijnand
Dale Wijnand
Colin Dean
Simon Schäfer
Răzvan Flavius Panda
Martin Duhem
jvican