Skip to content

Commit

Permalink
prevent recompilation after cold scalafix invocations on sbt 1.4+
Browse files Browse the repository at this point in the history
On all sbt versions, the Zinc analysis is now saved on compilations triggered
by scalafix invocations.

On sbt 1.4+, this allows subsequent scalafix invocations not to re-trigger
compilations, and subsequent regular compilations to be either incremental (in
case the build contains `-Xfatal-warnings` or alike) or a full cache hit
otherwise.

On sbt 1.3.x, this does not help for any subsequent compilations. That's
because before sbt/sbt@e81d459, compile*FileInputs
had to be explicitly triggeredBy any task that stores a Zinc analysis for the
cache key to be effective. Even though it would have been possible to register
`scalafix / compile` to trigger them and thus get the same benefits as on
sbt 1.4+, the complexity is just not worth it.
  • Loading branch information
bjaglin committed Apr 23, 2024
1 parent 9ceb2c5 commit 0e74275
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 47 deletions.
18 changes: 5 additions & 13 deletions src/main/scala/scalafix/sbt/ScalafixPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,10 @@ object ScalafixPlugin extends AutoPlugin {
(config / scalafix / streams).value
)
}
// sub-task of compile after which bytecode should not be modified
task.dependsOn(config / manipulateBytecode)
// we should not and cannot trigger compilation if this is an
// invocation triggerd by scalafixOnCompile
if (shellArgs.extra.contains("--triggered")) task
else task.dependsOn(config / compile)
} else {
Def.task {
if (errors.length == 1) {
Expand Down Expand Up @@ -583,7 +585,7 @@ object ScalafixPlugin extends AutoPlugin {
case "--stdout" =>
// --stdout cannot be cached as we don't capture the output to replay it
throw StampingImpossible
case "--tool-classpath" =>
case tcp if tcp.startsWith("--tool-classpath") =>
// custom tool classpaths might contain directories for which we would need to stamp all files, so
// just disable caching for now to keep it simple and to be safe
throw StampingImpossible
Expand Down Expand Up @@ -743,16 +745,6 @@ object ScalafixPlugin extends AutoPlugin {
options.ignoredScalacOptions() ++
scalacOptionsToRelax.map(_.pattern())
)
},
manipulateBytecode := {
val analysis = manipulateBytecode.value
if (!scalafixInvoked.value) analysis
else {
// prevent storage of the analysis with relaxed scalacOptions - despite not depending explicitly on compile,
// it is being triggered for parent configs/projects through evaluation of dependencyClasspath (TrackAlways)
// in the scope where scalafix is invoked
analysis.withHasModified(false)
}
}
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
val V = _root_.scalafix.sbt.BuildInfo

addCompilerPlugin(scalafixSemanticdb)
scalacOptions ++= Seq("-Yrangepos")

scalacOptions ++= Seq("-Yrangepos", "-Ywarn-unused")
scalaVersion := V.scala212
scalacOptions ++= Seq(
// generate errors on unused imports
"-Xfatal-warnings",
"-Ywarn-unused"
)
Compile / compile / scalacOptions ++= Seq(
// generate errors on procedure syntax
"-Wconf:cat=deprecation:e",
Expand Down Expand Up @@ -38,11 +32,6 @@ TaskKey[Unit]("checkLastCompilationCached") := {
}
}

TaskKey[Unit]("checkZincAnalysisPresent") := {
val isPresent = (Compile / previousCompile).value.analysis.isPresent()
assert(isPresent, "zinc analysis not found")
}

// https://github.com/sbt/sbt/commit/dbb47b3ce822ff7ec25881dadd71a3b29e202273
// must be outside a macro to workaround "Illegal dynamic reference: Def"
def streamScopedKey(scope: Scope) = Def.ScopedKey(scope, Keys.streams.key)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sbt.version=1.4.9
45 changes: 45 additions & 0 deletions src/sbt-test/sbt-1.4/relax-scalacOptions/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# when the build does not contain any flag relaxed on scalafix invocation
# check that regular compilation benefits froma prior direct scalafix invocation ...
-> scalafix --check RemoveUnused
-> checkLastCompilationCached
> compile
> checkLastCompilationCached

# ... and indirect scalafix invocation
> clean
> test:scalafix --check RemoveUnused
-> checkLastCompilationCached
> compile
> checkLastCompilationCached

# we now have 2 fatal warnings preventing compilation
> set scalacOptions ++= Seq("-Xfatal-warnings")
-> compile

# ensure that a semantic rule can be ran despite warnings, to fix one of the warning
> scalafix RemoveUnused

# confirm that the rule was applied and that triggered compilation is cached once the updated file recompiled
> scalafix --check RemoveUnused
-> checkLastCompilationCached
> scalafix --check RemoveUnused
> checkLastCompilationCached

# and that -Xfatal-warnings remains honored for regular compilation
-> compile

# confirm that regular compilation succeeds after fixing the last warning
> scalafix ProcedureSyntax
> compile

# check that regular compilation is cached as usual
> compile
> checkLastCompilationCached

# check that there is no recompilation when running a semantic rule after a regular compilation ...
> scalafix RemoveUnused
> checkLastCompilationCached

# nor when compiling after running a semantic rule
> compile
> checkLastCompilationCached
22 changes: 0 additions & 22 deletions src/sbt-test/sbt-scalafix/relax-scalacOptions/test

This file was deleted.

0 comments on commit 0e74275

Please sign in to comment.