document scalafixOnCompileConfig key & scalafixOnCompile shortcomings #1211
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
|
/cc @taisukeoe |
bjaglin
force-pushed
the
scalafixOnCompileConfig
branch
from
93f7f6d to
86cae56
Compare
bjaglin
changed the title
bjaglin
force-pushed
the
scalafixOnCompileConfig
branch
2 times, most recently
from
b26c567 to
6b576c8
Compare
bjaglin
commented
Jul 13, 2020
Comment on lines
+202
to
+226
| Although this looks like an easy way to use Scalafix as a linter, use this | ||
| feature with care as it has several shortcomings, for example: | ||
|
|
||
| 1. Some rules such as `RemoveUnused` can be counter-productive if applied too | ||
| often/early, as the work-in-progress code that was just added might disappear | ||
| after a simple `test`. | ||
| 1. If you run many semantic rules by default, the last one(s) to run might see | ||
| stale information and fail the invocation, which needs to be re-run manually. | ||
| This is [not specific to `scalafixOnCompile`](https://github.com/scalacenter/scalafix/issues/1204), | ||
| but the problem becomes much more visible with it. | ||
| 1. To keep the overhad minimal, `scalafixCaching` is automatically enabled when | ||
| `scalafixOnCompile` is, which can cause unexpected behaviors if you run into | ||
| false positive cache hits. `scalafixCaching` can explicitly be set to | ||
| `false` in that case. | ||
| 1. Non-idempotent rewrite rules might get you in an infinite loop where sources | ||
| never converge - not specific to `scalafixOnCompile` either, but rather | ||
| confusing when triggered automatically. | ||
| 1. Bugs in rule implementations can prevent you from getting a successul | ||
| `compile`, blocking testing or publishing for example | ||
|
|
||
| Some of these shortcomings can be mitigated by the `scalafixOnCompileConfig` | ||
| setting key, which allows to use a separate configuration file whenever | ||
| Scalafix is triggered by `compile`. This way, Scalafix can execute a | ||
| different, safer set of rules compared to the regular `scalafixConfig` that | ||
| remains used for explicit `scalafix` & `scalafixAll` invocations. |
There was a problem hiding this comment.
@taisukeoe I have (shamelessly) included your own concern in this list. Feedback very much welcomed!
There was a problem hiding this comment.
@bjaglin Great capture 👍 Thank you very much!
In the future, it would be nicer to describe what kind of rules are recommended for on-compile, although I haven't found the adequate words yet.
The safest way is to recommend only diagnostic rules such as DisableSyntax, while there are few rules only for diagnostics. I feel it is a bit too conservative and might miss some useful combinations.
So at this moment, I'd agree with your explanation.
bjaglin
changed the title
bjaglin
changed the title
bjaglin
force-pushed
the
scalafixOnCompileConfig
branch
from
6b576c8 to
d9898fa
Compare
bjaglin
force-pushed
the
scalafixOnCompileConfig
branch
from
d9898fa to
dc13b92
Compare
bjaglin
force-pushed
the
scalafixOnCompileConfig
branch
from
dc13b92 to
561f777
Compare
|
Considering scalacenter/sbt-scalafix#148 (review), I have spinned-off pure |
|
Closing this as obsolete, since #1213 was merged and the strategy for scalacenter/sbt-scalafix#148 is revisited in #1217 |
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.
Companion PR of scalacenter/sbt-scalafix#148. It is still in discussion but drafting the public doc helps weighting-in pros & cons I believe.