From 0cf550111b2f48bae55c20081eb7644a63ad8bed Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Sun, 5 Jul 2020 14:19:00 +0200 Subject: [PATCH 1/3] introduce scalafixOnCompile Users can control whether scalafix (running the default rules declared in .scalafix.conf) should be run as part of `compile` at the project & configuration level. The value of scalafixOnCompile is ignored when invoking directly scalafix or scalafixAll, as the CLI arguments (if there are any) take precedence over the default rules. That means for example that `scalafix --check` is safe to run even though scalafixOnCompile := true, as there will not be any rewrite triggered by the implicit compilation. --- .../scala/scalafix/sbt/ScalafixPlugin.scala | 45 +++++++++++++++++-- .../scalafixOnCompile/.scalafix.conf | 2 + .../sbt-scalafix/scalafixOnCompile/build.sbt | 23 ++++++++++ .../lint/src/test/scala/Null.scala | 3 ++ .../scalafixOnCompile/project/plugins.sbt | 2 + .../rewrite/src/it/scala/UnusedImports.scala | 3 ++ .../src/main/scala/UnusedImports.scala | 3 ++ .../sbt-scalafix/scalafixOnCompile/test | 26 +++++++++++ 8 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/.scalafix.conf create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/build.sbt create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/lint/src/test/scala/Null.scala create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/project/plugins.sbt create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/it/scala/UnusedImports.scala create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/main/scala/UnusedImports.scala create mode 100644 src/sbt-test/sbt-scalafix/scalafixOnCompile/test diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index 2b2fc60c..cc93704e 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -33,13 +33,22 @@ object ScalafixPlugin extends AutoPlugin { inputKey[Unit]( "Run scalafix rule(s) in this project and configuration. " + "For example: scalafix RemoveUnusedImports. " + - "To run on test sources use test:scalafix or scalafixAll." + "To run on test sources use test:scalafix or scalafixAll. " + + "When invoked directly, prior compilation will be triggered for semantic rules." ) val scalafixAll: InputKey[Unit] = inputKey[Unit]( "Run scalafix rule(s) in this project, for all configurations where scalafix is enabled. " + - "Compile and Test are enabled by default, other configurations can be enabled via scalafixConfigSettings." + "Compile and Test are enabled by default, other configurations can be enabled via scalafixConfigSettings. " + + "When invoked directly, prior compilation will be triggered for semantic rules." ) + + val scalafixOnCompile: SettingKey[Boolean] = + settingKey[Boolean]( + "Run Scalafix rule(s) declared in .scalafix.conf on compilation and fail on lint errors. " + + "Off by default." + ) + val scalafixCaching: SettingKey[Boolean] = settingKey[Boolean]( "Cache scalafix invocations (off by default, still experimental)." @@ -87,6 +96,17 @@ object ScalafixPlugin extends AutoPlugin { def scalafixConfigSettings(config: Configuration): Seq[Def.Setting[_]] = Seq( scalafix := scalafixInputTask(config).evaluated, + compile := Def.taskDyn { + val oldCompile = + compile.value // evaluated first, before the potential scalafix evaluation + val runScalafixAfterCompile = + scalafixOnCompile.value && !scalafixRunExplicitly.value + if (runScalafixAfterCompile) + scalafix + .toTask("") + .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 // fgRunMain is fine), there is no specific streams attached to InputTasks, so we they end up sharing the // global streams, causing issues for cache storage. This does not happen for Tasks, so we define a dummy one @@ -160,6 +180,7 @@ object ScalafixPlugin extends AutoPlugin { override lazy val globalSettings: Seq[Def.Setting[_]] = Seq( scalafixConfig := None, // let scalafix-cli try to infer $CWD/.scalafix.conf scalafixCaching := false, + scalafixOnCompile := false, scalafixResolvers := Seq( Repository.ivy2Local(), Repository.central(), @@ -394,16 +415,22 @@ object ScalafixPlugin extends AutoPlugin { new SemanticdbNotFound(ruleNames, scalaVersion.value, sbtVersion.value) ).findErrors(files, dependencies, withScalaInterface) if (errors.isEmpty) { - Def.task { + val task = Def.task { + // passively consume compilation output without triggering compile as it can result in a cyclic dependency + val classpath = + dependencyClasspath.in(config).value.map(_.data.toPath) :+ + classDirectory.in(config).value.toPath val semanticInterface = withScalaInterface.withArgs( Arg.Paths(files), - Arg.Classpath(fullClasspath.in(config).value.map(_.data.toPath)) + Arg.Classpath(classpath) ) runArgs( semanticInterface, streams.in(config, scalafix).value ) } + if (scalafixRunExplicitly.value) task.dependsOn(compile.in(config)) + else task } else { Def.task { if (errors.length == 1) { @@ -537,6 +564,16 @@ object ScalafixPlugin extends AutoPlugin { } } + // Controls whether scalafix should depend on compile (true) & whether compile may depend on + // scalafix (false), to avoid cyclic dependencies causing deadlocks during executions (as + // dependencies come from dynamic tasks). + private val scalafixRunExplicitly: Def.Initialize[Task[Boolean]] = + Def.task { + executionRoots.value.exists { root => + Seq(scalafix.key, scalafixAll.key).contains(root.key) + } + } + private def isScalaFile(file: File): Boolean = { val path = file.getPath path.endsWith(".scala") || diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/.scalafix.conf b/src/sbt-test/sbt-scalafix/scalafixOnCompile/.scalafix.conf new file mode 100644 index 00000000..8af1d8a7 --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/.scalafix.conf @@ -0,0 +1,2 @@ +rules = [DisableSyntax, RemoveUnused] +DisableSyntax.noNulls = true \ No newline at end of file diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/build.sbt b/src/sbt-test/sbt-scalafix/scalafixOnCompile/build.sbt new file mode 100644 index 00000000..83ea22a8 --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/build.sbt @@ -0,0 +1,23 @@ +import _root_.scalafix.sbt.{BuildInfo => Versions} + +inThisBuild( + Seq( + scalaVersion := Versions.scala212, + scalacOptions ++= List( + "-Yrangepos", + "-Ywarn-unused-import" + ) + ) +) +lazy val lint = project + .settings( + addCompilerPlugin(scalafixSemanticdb) + ) + +lazy val rewrite = project + .configs(IntegrationTest) + .settings( + Defaults.itSettings, + inConfig(IntegrationTest)(scalafixConfigSettings(IntegrationTest)), + addCompilerPlugin(scalafixSemanticdb) + ) diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/lint/src/test/scala/Null.scala b/src/sbt-test/sbt-scalafix/scalafixOnCompile/lint/src/test/scala/Null.scala new file mode 100644 index 00000000..cbfa633d --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/lint/src/test/scala/Null.scala @@ -0,0 +1,3 @@ +object Null { + println(null) +} diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/project/plugins.sbt b/src/sbt-test/sbt-scalafix/scalafixOnCompile/project/plugins.sbt new file mode 100644 index 00000000..2d3b4d3b --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/project/plugins.sbt @@ -0,0 +1,2 @@ +resolvers += Resolver.sonatypeRepo("public") +addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % sys.props("plugin.version")) diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/it/scala/UnusedImports.scala b/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/it/scala/UnusedImports.scala new file mode 100644 index 00000000..ad9eeac9 --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/it/scala/UnusedImports.scala @@ -0,0 +1,3 @@ +import java.time.Instant + +object UnusedImports diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/main/scala/UnusedImports.scala b/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/main/scala/UnusedImports.scala new file mode 100644 index 00000000..ad9eeac9 --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/rewrite/src/main/scala/UnusedImports.scala @@ -0,0 +1,3 @@ +import java.time.Instant + +object UnusedImports diff --git a/src/sbt-test/sbt-scalafix/scalafixOnCompile/test b/src/sbt-test/sbt-scalafix/scalafixOnCompile/test new file mode 100644 index 00000000..5939078d --- /dev/null +++ b/src/sbt-test/sbt-scalafix/scalafixOnCompile/test @@ -0,0 +1,26 @@ +# check implicit rewrite of rewrite/src/main/scala/UnusedImports.scala via `compile` +> compile +-> scalafix --check +> set scalafixOnCompile.in(ThisBuild) := true +-> scalafix --check +> compile +> scalafix --check + +# check explicit rewrite of rewrite/src/it/scala/UnusedImports.scala via `scalafix` +-> it:scalafix --check +> it:scalafix +> it:scalafix --check + +# check lint for lint/src/test/scala/Null.scala +-> lint/test:scalafix --check +-> lint/test:scalafix +-> lint/test:compile + +# check that default rules are ignored when rules are passed explicitly +-> lint/test:scalafix --check +> lint/test:scalafix --check RemoveUnused +> lint/test:scalafix RemoveUnused + +# check configuration granularity for scalafixOnCompile +> set scalafixOnCompile.in(lint, Test) := false +> lint/test:compile \ No newline at end of file From a4ba4964420fac47d8633a6b68d4779e73b39708 Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Wed, 8 Jul 2020 22:42:57 +0200 Subject: [PATCH 2/3] enable caching when `scalafixOnCompile := true` --- src/main/scala/scalafix/sbt/ScalafixPlugin.scala | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index cc93704e..5ee10188 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -46,12 +46,13 @@ object ScalafixPlugin extends AutoPlugin { val scalafixOnCompile: SettingKey[Boolean] = settingKey[Boolean]( "Run Scalafix rule(s) declared in .scalafix.conf on compilation and fail on lint errors. " + - "Off by default." + "Off by default. When enabled, caching will be automatically activated, " + + "but can be disabled with `scalafixCaching := false`." ) val scalafixCaching: SettingKey[Boolean] = settingKey[Boolean]( - "Cache scalafix invocations (off by default, still experimental)." + "Cache scalafix invocations (off by default, on if scalafixOnCompile := true)." ) import scala.language.implicitConversions @@ -179,7 +180,6 @@ object ScalafixPlugin extends AutoPlugin { override lazy val globalSettings: Seq[Def.Setting[_]] = Seq( scalafixConfig := None, // let scalafix-cli try to infer $CWD/.scalafix.conf - scalafixCaching := false, scalafixOnCompile := false, scalafixResolvers := Seq( Repository.ivy2Local(), @@ -354,10 +354,9 @@ object ScalafixPlugin extends AutoPlugin { scalafixResolvers.in(ThisBuild).value, projectDepsInternal ) + val cachingRequested = scalafixCaching.or(scalafixOnCompile).value val maybeNoCache = - if (shell.noCache || !scalafixCaching.in(config).value) - Seq(Arg.NoCache) - else Nil + if (shell.noCache || !cachingRequested) Seq(Arg.NoCache) else Nil val mainInterface = mainInterface0 .withArgs(maybeNoCache: _*) .withArgs( From 7562e24d8cf08ddde5b1897c7bad152bdf2c2afe Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Wed, 8 Jul 2020 23:14:40 +0200 Subject: [PATCH 3/3] deprecated built-in rule name --- src/main/scala/scalafix/sbt/ScalafixPlugin.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index 5ee10188..9977d98c 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -32,7 +32,7 @@ object ScalafixPlugin extends AutoPlugin { val scalafix: InputKey[Unit] = inputKey[Unit]( "Run scalafix rule(s) in this project and configuration. " + - "For example: scalafix RemoveUnusedImports. " + + "For example: scalafix RemoveUnused. " + "To run on test sources use test:scalafix or scalafixAll. " + "When invoked directly, prior compilation will be triggered for semantic rules." )