scala-steward
scala-steward copied to clipboard
Run Scalafix build migrations using sbt
Tries to fix an issue encountered in #3126 whereby Scalafix build migrations failed because they require SemanticDB to be enabled.
This PR changes the SbtAlg to make use of sbt to run Scalafix migrations.
It does this by writing the required sbt configuration files recursively up to the deepest meta-build found in the project before running the scalafix command in each meta-build, similar to getDependencies.
The Scalafix CLI is still used to run syntactic migrations, because sbt-scalafix cannot run migrations on *.sbt files.
Thanks to my employer @opencastsoftware for sponsoring this work.
@fthomas is it possible to dry run Scala Steward (i.e. not open any PRs)? I'd like to test this out but I don't really want to open PRs against other folks' projects.
This test no longer makes a lot of sense in light of this change, as it's testing for the usage of the Scalafix CLI - can I simply remove that test or would you prefer to maintain some kind of testing of build migrations at the level of EditAlg?
is it possible to dry run Scala Steward (i.e. not open any PRs)?
There is some kind of dry run (https://github.com/scala-steward-org/scala-steward/pull/1828) but that would also not run any Scalafix migrations. You could temporarily comment this line to not push any commits and not creating PRs.
What I also use sometimes for testing is a test repo (like https://github.com/scala-steward-org/test-repo-2) with multiple branches. If Scala Steward creates a PR for a specific branch and I want to test my changes again, I just push another branch (without Scala Steward's change) to the repo and point Scala Steward to that new branch.
OK, I've tested this now on a few projects with some mixed results:
- In spotify/scio the sbt-tpolecat migration ran successfully, although it looks like I have some more work to do on the migration rules as wildcard imports from some of the symbols are not being migrated. Gosh how I hate wildcard imports! They make understanding code difficult and it turns out they make migrating it difficult too.
- In some builds with older (<1.5.x) sbt, I was expecting
into be migrated to/in the mainbuild.sbt, but it didn't seem to work in at least uclid-org/uclid and sdkman/sdkman-candidates. ~~I wonder if this implies that build migrations need to run at the root level to touchbuild.sbt, in which case there would actually be no difference betweensourcesandbuildmigration logic except thatsourcesmigrations would only operate at buildDepth = 0.~~ EDIT: Absolutely not, this ends with scalafix attempting to translateinfrom ScalaTest too!
it looks like I have some more work to do on the migration rules
I have done some work on this in sbt-tpolecat now and will send a PR to update the migration SHA in scala-steward
In some builds with older (<1.5.x) sbt, I was expecting
into be migrated to/in the mainbuild.sbt, but it didn't seem to work
I have asked the Scalafix experts in the Scalameta Discord about this 🤞 that they have some idea how to fix it
OK, so the word is that sbt-scalafix can't currently update *.sbt files because sbt doesn't return those as part of the build's unmanagedSources, and semantic migrations that update *.sbt files aren't supported and have never worked as it's so hard to generate SemanticDB for *.sbt files.
That means we need to continue running syntactic migrations on *.sbt files with the CLI, perhaps also disabling semantic migrations in the CLI invocations as they won't work in any case.
OK, so the word is that
sbt-scalafixcan't currently update*.sbtfiles because sbt doesn't return those as part of the build'sunmanagedSources, and semantic migrations that update*.sbtfiles aren't supported and have never worked as it's so hard to generate SemanticDB for*.sbtfiles.
:-(
That means we need to continue running syntactic migrations on
*.sbtfiles with the CLI, perhaps also disabling semantic migrations in the CLI invocations as they won't work in any case.
You mean via the --syntactic option? Would that have made a difference for the sbt-tpolecat migration or would it just have not run the migration?
You mean via the
--syntacticoption? Would that have made a difference for the sbt-tpolecat migration or would it just have not run the migration?
I think it would just have not run the migration, avoiding the errors due to missing semanticdb
Codecov Report
Patch coverage: 100.00% and project coverage change: +0.04% :tada:
Comparison is base (
ea94b76) 91.04% compared to head (be3c89a) 91.08%. Report is 4 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3133 +/- ##
==========================================
+ Coverage 91.04% 91.08% +0.04%
==========================================
Files 162 162
Lines 3406 3423 +17
Branches 315 312 -3
==========================================
+ Hits 3101 3118 +17
Misses 305 305
| Files Changed | Coverage Δ | |
|---|---|---|
| ...a/org/scalasteward/core/buildtool/sbt/SbtAlg.scala | 98.83% <100.00%> (+0.26%) |
:arrow_up: |
| .../scalasteward/core/edit/scalafix/ScalafixCli.scala | 100.00% <100.00%> (ø) |
|
| .../main/scala/org/scalasteward/core/io/FileAlg.scala | 100.00% <100.00%> (ø) |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.