Introduce scalafixAll InputTask #126
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbt/sbt#3572 shows that the macro behind inputTaskDyn is too restrictive, so this is bypassing it, to use directly the helpers that the macro would generate if it was less permissive.
bjaglin
commented
Jun 13, 2020
| private var scalafixInterface: () => ScalafixInterface = | ||
| () => throw new UninitializedError | ||
|
|
||
| private val scalafixInterface: Def.Initialize[() => ScalafixInterface] = |
There was a problem hiding this comment.
By being a Setting, the function wrapped here is instantiated at sbt load time (capturing other global scalafix* settings) as it's referenced by scalafix[All]InputTask through parser, but evaluated at task execution time, and since it's backed by a LazyValue, it's memoized.
* explicitly scope all value lookups to the current configuration in order to prevent scalafixAll from lookuping up in the Zero config * make sure scalafix and scalafixAll share the same cache for a given configuration
olafurpg
approved these changes
Jun 14, 2020
| @@ -158,20 +170,49 @@ object ScalafixPlugin extends AutoPlugin { | |||
|
|
|||
| } | |||
|
|
|||
| private def scalafixAllInputTask(): Def.Initialize[InputTask[Unit]] = | |||
| // workaround https://github.com/sbt/sbt/issues/3572 by invoking directly what Def.inputTaskDyn would via macro | |||
There was a problem hiding this comment.
Awesome to see a workaround for this issue, nice job tracking down the issue!
| @@ -205,6 +205,24 @@ $ exec chmod 000 src/test/scala/Valid.scala | |||
| $ delete src/main/scala | |||
| $ delete src/test/scala | |||
|
|
|||
| # scalafixAll and scalafix should share the same cache per configuration | |||
This was referenced Jul 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Docs companion: scalacenter/scalafix#1160
Similarly to
scalafmtAll(except that the implementation here is more tricky as we deal with anInputTaskfor which.alldoes not work), this allows a user to easily and efficiently run Scalafix across configurations for a given project (or of course for all projects/configurations with root project aggregation)I saw there were previous discussions/opinions about that (I should have opened an issue to discuss that beforehand). From my side I see no reason not to do it, especially because implementation is hard(er) downstream but simple here (or at least readable & compact), and because it allows
scalafix --checkto run in parallel over all configurations (rather than in sequential as documented in https://scalacenter.github.io/scalafix/docs/users/installation.html#enforce-in-ci), so it's not just sugar.There has already been work downstream to bring that to users through a command (like https://github.com/alejandrohdezma/sbt-fix#running-scalafix-in-all-configurations), but refactoring the code here allows us to expose an
InputTask, with the same auto-completion as the existingscalafix, which can't be done downstream at the moment.The first commit is preliminary, separating parsing & core logic from
scalafixso that the output of the parser is reused across configurations during ascalafixAllinvocation, and getting rid of a workaround.