scalafix icon indicating copy to clipboard operation
scalafix copied to clipboard

Scalafix sbt task hangs when invoked as part of compile task

Open bpholt opened this issue 1 year ago • 2 comments
trafficstars

Prior to Scalafix 0.12.1, we had many repos where we apply Scalafix rules on every compile by chaining the scalafix task as part of the compile task. As of 0.12.1, applying any semantic rules in this way causes sbt to hang in such a way that it can only be killed from the outside (e.g. using kill -9 ${sbt pid}).

As a minimal example, use this build.sbt:

Compile / compile := Def.taskDyn {
  val output = (Compile / compile).value
  Def.task {
    (Compile / scalafix).toTask(" ExplicitResultTypes").value
    output
  }
}.value

project/plugins.sbt:

addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.12.1")

and project/build.properties:

sbt.version=1.10.0

Example output:

[info] welcome to sbt 1.10.0 (Eclipse Adoptium Java 11.0.18)
[info] loading settings for project global-plugins from global.sbt ...
[info] loading global plugins from ~/.sbt/1.0/plugins
[info] loading settings for project sbt-scalafix-issue-build from plugins.sbt ...
[info] loading project definition from ~/sbt-scalafix-issue/project
[info] loading settings for project sbt-scalafix-issue from build.sbt ...
[info] set current project to sbt-scalafix-issue (in build file:~/sbt-scalafix-issue/)
sbt:sbt-scalafix-issue> compile
^C
[warn] Canceling execution...
^Z
[1]+  Stopped                 sbt
~/sbt-scalafix-issue $ kill -9 %1
[1]+  Killed: 9               sbt

If we comment out the (Compile / scalafix).toTask(" ExplicitResultTypes").value line in build.sbt and run the tasks separately, they work fine:

[info] welcome to sbt 1.10.0 (Eclipse Adoptium Java 11.0.18)
[info] loading settings for project global-plugins from global.sbt ...
[info] loading global plugins from ~/.sbt/1.0/plugins
[info] loading settings for project sbt-scalafix-issue-build from plugins.sbt ...
[info] loading project definition from ~/sbt-scalafix-issue/project
[info] loading settings for project sbt-scalafix-issue from build.sbt ...
[info] set current project to sbt-scalafix-issue (in build file:~/sbt-scalafix-issue/)
sbt:sbt-scalafix-issue> compile
[success] Total time: 0 s, completed Jun 10, 2024, 4:56:53 PM
sbt:sbt-scalafix-issue> scalafix ExplicitResultTypes
[success] Total time: 1 s, completed Jun 10, 2024, 4:57:02 PM

Furthermore, using one of the built-in syntactic rules works fine as well.

Replacing Scalafix 0.12.1 with 0.12.0 also works fine.

bpholt avatar Jun 10 '24 22:06 bpholt

Thanks for the report @bphold!

This is clearly a regression of https://github.com/scalacenter/sbt-scalafix/pull/411. I can't think of an easy way to fix that without reverting that PR...

However, it looks like what you are trying to achieve can be done through a built-in feature.

# build.sbt
scalafixOnCompile := true
# .scalafix.conf
triggered.rules = [ ExplicitResultTypes ]

Would that work for you?

bjaglin avatar Jun 23 '24 16:06 bjaglin

Yes, I think that works! I'm going to try re-updating some of the projects I downgraded from 0.12.1 to 0.12.0 to make sure, but I'm cautiously optimistic.

For background, many of our projects use an AddCatsTaglessInstances scalafix, and are therefore typically structured like this:

lazy val `generated-sources` = (project in file("generated-sources"))
  .settings(
    Compile / compile := Def.taskDyn {
      val compileOutput = (Compile / compile).value

      Def.task {
        (Compile / scalafix).toTask(" AddCatsTaglessInstances").value
        compileOutput
      }
    }.value,
  )

lazy val `main-project` = project.in(file(".")).dependsOn(`generated-sources`)

The generated-sources project uses a third party code generator to generate a bunch of Scala files, which we then modify using scalafix. (Not an ideal structure, obviously, but it's where we are.)

Ideally we'd be able to run main-project/compile and have all of this happen in one step:

  1. generate the sources in the generated-sources project
  2. compile those generated sources so scalafix can make changes
  3. run the AddCatsTaglessInstances scalafix
  4. compile the output of the AddCatsTaglessInstances scalafix
  5. compile the sources in main-project, which depend on the modifications made by the scalafix

As it exists today (both with our previous Compile / compile override and with scalafixOnCompile := true), sbt appears to skip step 4, so when it starts step 5 and tries to compile the main-project sources, it fails because it doesn't recognize the changes made by the scalafix. Explicitly running generated-sources/compile before running main-project/compile works, but it's easy to forget to do. I wonder if the changes made to the sbt task graph by setting scalafixOnCompile := true could also take that into account? 🤔

bpholt avatar Jul 24 '24 21:07 bpholt

Yes, I think that works! I'm going to try re-updating some of the projects I downgraded from 0.12.1 to 0.12.0 to make sure, but I'm cautiously optimistic.

Glad to see you are back tracking the latest scalafix version 👍

As it exists today (both with our previous Compile / compile override and with scalafixOnCompile := true), sbt appears to skip step 4, so when it starts step 5 and tries to compile the main-project sources, it fails because it doesn't recognize the changes made by the scalafix. ... I wonder if the changes made to the sbt task graph by setting scalafixOnCompile := true could also take that into account? 🤔

Sorry it took me so long to answer, I wanted to have a deep look at it since my initial thought was that this was impossible... Unfortunately, my conclusion is that it's indeed impossible with the current sbt model, where a task dependency can only be evaluated once per task invocation.

To achieve what you are describing, my recommendation would be to follow the pattern used by https://github.com/hamnis/dataclass-scalafix. Namely, to handle scalafix additions via a source generator declared hooked in main-project, which does not mutate files in generated-sources but instead feeds patched versions of them to the main-project compiler. sbt-scalafix caching behavior is completely untested with that setup, so there might be false negatives or false positives, but sbt-coursier has been using it for 2 years, so it shouldn't be a blocker :)

In any case, I suggest we continue the discussion on https://github.com/scalacenter/scalafix/issues/1674 if that's OK for you.

bjaglin avatar Dec 29 '24 21:12 bjaglin