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.
It was becoming a pain to work on these files in intellij because the
auto-import feature would implicitly optimize all of the imports in
these files, leading to a large diff. I'd then have to go and manually
add the import that I care about. This change does add some wildcard
imports, which I don't always love, but these files are so unwieldy
already that I think it's worth it to have the imports follow the format
preferred by intellij.
This file was littered with intellij warnings due to public members and
fields not having their types annotated. Although in this case it didn't
really matter, it is good practice to always annotate public methods and
fields so that they can evolve in a binary compatible way.
It has long been a frustration of mine that it is necessary to prepend
multiple commands with a ';'. In this commit, I relax that restriction.
I had to reorder the command definitions so that multi comes before act.
This was because if the multi command did not have a leading semicolon,
then it would be handled by the action parser before the multi command
parser had a shot at it. Sadness ensued.
Presently the multi command parser doesn't work correctly if one of the
commands includes a string literal. For example, suppose that there is
an input task defined name "bash" that shells out and runs the input.
Then the following does not work with the current multi command parser:
; bash "rm target/classes/Foo.class; touch src/main/scala/Foo.scala"; comple
Note that this is a real use case that has caused me issues in the past.
The problem is that the semicolon inside of the quote gets interpreted
as a command separator token. To fix this, I rework the parser so that
it consumes string literals and doesn't modify them. By using
StringEscapable, I allow the string to contain quotation marks itself.
I couldn't write a scripted test for this because in a command like
`; foo "bar"; baz`, the quotes around bar seem to get stripped. This
could be fixed by adding an alternative to StringEscapable that matches
an escaped string, but that is more work than I'm willing to do right
now.
I discovered that when I ran multi-commands with '~' that if there was a
space between the ';' and the command, then the parsing of the command
would fail and the watch would abort. To fix this, I refactor
Watched.watch to use the multi command parser and, if that parser fails,
we fallback on a single command.
Prior to this commit, there was no unit testing of the parser for
multiple commands. I wanted to make some improvements to the parser, so
I reworked the implementation to be testable. This change also allows
the multiParserImpl method to be shared with Watched.watch, which I will
also update in a subsequent commit.
There also were no explicit scripted tests for multiple commands, so I
added one that I will augment in later commits.
In #4446, @japgolly reported that in some projects, if a parent project
was broken, then '~' would immediately exit upon startup. I tracked it
down to this managed sources filter. The idea of this filter is to avoid
getting stuck in a build loop if managedSources writes into an unmanaged
source directory. If the (managedSources in ThisScope).value line
failed, however, it would cause the watchSources, and by delegation,
watchTransitiveSources task to fail. The fix is to only create this
filter if the managedSources task succeeds.
I'm not 100% sure if we shouldn't just get rid of this filter entirely
and just document that '~' will probably loop if a build writes the
result of managedSources into an unmanaged source directory.
In #4446, @japgolly reported that in some projects, if a parent project
was broken, then '~' would immediately exit upon startup. I tracked it
down to this managed sources filter. The idea of this filter is to avoid
getting stuck in a build loop if managedSources writes into an unmanaged
source directory. If the (managedSources in ThisScope).value line
failed, however, it would cause the watchSources, and by delegation,
watchTransitiveSources task to fail. The fix is to only create this
filter if the managedSources task succeeds.
I'm not 100% sure if we shouldn't just get rid of this filter entirely
and just document that '~' will probably loop if a build writes the
result of managedSources into an unmanaged source directory.
Fixes#4437
Until now, sbt was resolved twice once by the launcher, and the second time by the metabuild.
This excludes sbt from the metabuild graph, and instead uses app classpath from the launcher.