diff --git a/build.sbt b/build.sbt index fbe3b8a3..23ce6dc3 100644 --- a/build.sbt +++ b/build.sbt @@ -56,7 +56,7 @@ scriptedSbt := { if (jdk >= 21) "1.9.0" // first release that supports JDK21 else - (pluginCrossBuild / sbtVersion).value + "1.3.3" // get https://github.com/sbt/sbt/issues/1673 to avoid race conditions } libraryDependencies += compilerPlugin(scalafixSemanticdb) diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index 720a2d4d..1f4cb6d0 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -102,9 +102,20 @@ object ScalafixPlugin extends AutoPlugin { val oldCompile = compile.value // evaluated first, before the potential scalafix evaluation if (scalafixOnCompile.value) - scalafix - .toTask(" --triggered") - .map(_ => oldCompile) + Def.taskDyn { + // evaluate scalafixInvoked only when scalafixOnCompile is set + // to reduce risk of side effects on the build + if (scalafixInvoked.value) + // don't trigger scalafix if it was explicitly invoked, + // as it would be not only inefficient, but incorrect since + // scalafix might run with different CLI args, potentially + // rewriting files while `--check` was used + Def.task(oldCompile) + else + scalafix + .toTask(" --triggered") + .map(_ => oldCompile) + } else Def.task(oldCompile) }.value, // In some cases (I haven't been able to understand when/why, but this also happens for bgRunMain while @@ -505,8 +516,13 @@ 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 (circular dependency) trigger compilation + // if this is an non-explicit invocation - we use the presence of a + // flag rather than checking !scalafixInvoked, in order to trigger + // compile in case scalafix is wrapped in another task that + // scalafixInvoked does not know about + if (shellArgs.extra.contains("--triggered")) task + else task.dependsOn(config / compile) } else { Def.task { if (errors.length == 1) { @@ -583,7 +599,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 @@ -743,16 +759,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) - } } ) diff --git a/src/sbt-test/sbt-scalafix/relax-scalacOptions/build.sbt b/src/sbt-test/sbt-scalafix/relax-scalacOptions/build.sbt index cc0547a4..1bfde9f8 100644 --- a/src/sbt-test/sbt-scalafix/relax-scalacOptions/build.sbt +++ b/src/sbt-test/sbt-scalafix/relax-scalacOptions/build.sbt @@ -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", @@ -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) diff --git a/src/sbt-test/sbt-scalafix/relax-scalacOptions/test b/src/sbt-test/sbt-scalafix/relax-scalacOptions/test index 0b61661c..678f60ff 100644 --- a/src/sbt-test/sbt-scalafix/relax-scalacOptions/test +++ b/src/sbt-test/sbt-scalafix/relax-scalacOptions/test @@ -1,22 +1,45 @@ -# we have 2 fatal warnings preventing compilation +# 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 -> scalafixAll RemoveUnused +> scalafix RemoveUnused -# but that the zinc analysis file has not been persisted, otherwise the run with relax scalacOptions would -# cause the next compile to be full (non-incremental) no matter if there was a previous successful compilation --> checkZincAnalysisPresent +# 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 compilation succeeds after fixing the last warning +# confirm that regular compilation succeeds after fixing the last warning > scalafix ProcedureSyntax > compile -> checkZincAnalysisPresent -# check that there is no recompilation when running a semantic rule after a successful compilation --> checkLastCompilationCached -> scalafixAll RemoveUnused +# 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 \ No newline at end of file