scala-steward icon indicating copy to clipboard operation
scala-steward copied to clipboard

Run Scalafix build migrations using sbt

Open DavidGregory084 opened this issue 2 years ago • 10 comments

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.

DavidGregory084 avatar Jul 31 '23 13:07 DavidGregory084

@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.

DavidGregory084 avatar Jul 31 '23 13:07 DavidGregory084

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?

DavidGregory084 avatar Jul 31 '23 14:07 DavidGregory084

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.

fthomas avatar Jul 31 '23 14:07 fthomas

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.

fthomas avatar Jul 31 '23 14:07 fthomas

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 in to be migrated to / in the main build.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 touch build.sbt, in which case there would actually be no difference between sources and build migration logic except that sources migrations would only operate at buildDepth = 0.~~ EDIT: Absolutely not, this ends with scalafix attempting to translate in from ScalaTest too!

DavidGregory084 avatar Aug 01 '23 10:08 DavidGregory084

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 in to be migrated to / in the main build.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

DavidGregory084 avatar Aug 01 '23 14:08 DavidGregory084

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.

DavidGregory084 avatar Aug 02 '23 09:08 DavidGregory084

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.

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?

fthomas avatar Aug 02 '23 11:08 fthomas

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?

I think it would just have not run the migration, avoiding the errors due to missing semanticdb

DavidGregory084 avatar Aug 02 '23 11:08 DavidGregory084

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.

codecov[bot] avatar Aug 02 '23 12:08 codecov[bot]