From 2ca68e4f38cc39d5a7cdd32658e69bcad9d5b6ca Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Fri, 12 Jun 2020 14:32:10 +0200 Subject: [PATCH 1/2] remove settings initialization workaround https://github.com/sbt/sbt/issues/3572 shows that the macro behind inputTaskDyn is too restrictive, so this is bypassing it, to use directly the helpers that the macro would generate if it was less permissive. --- .../internal/sbt/ScalafixCompletions.scala | 8 +-- .../scala/scalafix/sbt/ScalafixPlugin.scala | 65 ++++++++++--------- .../internal/sbt/SbtCompletionsSuite.scala | 2 +- 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/main/scala/scalafix/internal/sbt/ScalafixCompletions.scala b/src/main/scala/scalafix/internal/sbt/ScalafixCompletions.scala index e1cded04..f8498720 100644 --- a/src/main/scala/scalafix/internal/sbt/ScalafixCompletions.scala +++ b/src/main/scala/scalafix/internal/sbt/ScalafixCompletions.scala @@ -9,7 +9,7 @@ import sbt.complete._ import sbt.complete.DefaultParsers._ class ScalafixCompletions( - workingDirectory: () => Path, + workingDirectory: Path, loadedRules: () => Seq[ScalafixRule], terminalWidth: Option[Int] ) { @@ -66,9 +66,9 @@ class ScalafixCompletions( } string - .examples(new AbsolutePathExamples(workingDirectory())) + .examples(new AbsolutePathExamples(workingDirectory)) .map { f => - toAbsolutePath(Paths.get(f), workingDirectory()) + toAbsolutePath(Paths.get(f), workingDirectory) } .filter(f => Files.exists(f), x => x) } @@ -96,7 +96,7 @@ class ScalafixCompletions( private val namedRule2: Parser[ShellArgs.Rule] = namedRule.map(s => ShellArgs.Rule(s)) private lazy val gitDiffParser: P = { - val jgitCompletion = new JGitCompletion(workingDirectory()) + val jgitCompletion = new JGitCompletion(workingDirectory) token( NotQuoted, TokenCompletions.fixed((seen, _) => { diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index 7478d0a5..78bc3db3 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -7,9 +7,10 @@ import com.geirsson.coursiersmall.Repository import sbt.KeyRanks.Invisible import sbt.Keys._ import sbt._ +import sbt.complete._ +import sbt.internal.sbtscalafix.Caching._ import sbt.internal.sbtscalafix.{Compat, JLineAccess} import sbt.plugins.JvmPlugin -import sbt.internal.sbtscalafix.Caching._ import scalafix.interfaces.ScalafixError import scalafix.internal.sbt._ @@ -103,19 +104,6 @@ object ScalafixPlugin extends AutoPlugin { Seq(ivyConfigurations += ScalafixConfig) override lazy val globalSettings: Seq[Def.Setting[_]] = Seq( - initialize := { - val _ = initialize.value - // Ideally, we would not resort to storing mutable state in `initialize`. - // The optimal solution would be to run `scalafixDependencies.value` - // inside `scalafixInputTask`. - // However, we can't do that due to an sbt bug: - // https://github.com/sbt/sbt/issues/3572#issuecomment-417582703 - workingDirectory = baseDirectory.in(ThisBuild).value.toPath - scalafixInterface = ScalafixInterface.fromToolClasspath( - scalafixDependencies = scalafixDependencies.in(ThisBuild).value, - scalafixCustomResolvers = scalafixResolvers.in(ThisBuild).value - ) - }, scalafixConfig := None, // let scalafix-cli try to infer $CWD/.scalafix.conf scalafixCaching := false, scalafixResolvers := ScalafixCoursier.defaultResolvers, @@ -124,12 +112,28 @@ object ScalafixPlugin extends AutoPlugin { ) lazy val stdoutLogger = Compat.ConsoleLogger(System.out) - private var workingDirectory = file("").getAbsoluteFile.toPath - private var scalafixInterface: () => ScalafixInterface = - () => throw new UninitializedError + + private val scalafixInterface: Def.Initialize[() => ScalafixInterface] = + Def.setting { + ScalafixInterface.fromToolClasspath( + scalafixDependencies = scalafixDependencies.in(ThisBuild).value, + scalafixCustomResolvers = scalafixResolvers.in(ThisBuild).value + ) + } + + private val parser: Def.Initialize[Parser[ShellArgs]] = Def.setting { + new ScalafixCompletions( + workingDirectory = baseDirectory.in(ThisBuild).value.toPath, + // Unfortunately, local rules will not show up as completions in the parser, as that parser can only + // depend on settings, while local rules classpath must be looked up via tasks + loadedRules = () => scalafixInterface.value().availableRules(), + terminalWidth = Some(JLineAccess.terminalWidth) + ).parser + } private def scalafixArgsFromShell( shell: ShellArgs, + scalafixInterface: () => ScalafixInterface, projectDepsExternal: Seq[ModuleID], baseResolvers: Seq[Repository], projectDepsInternal: Seq[File] @@ -161,15 +165,17 @@ object ScalafixPlugin extends AutoPlugin { private def scalafixInputTask( config: Configuration ): Def.Initialize[InputTask[Unit]] = - Def.inputTaskDyn { - val shell0 = new ScalafixCompletions( - workingDirectory = () => workingDirectory, - // Unfortunately, local rules will not show up as completions in the parser, as that parser can only depend - // on global settings (see note in initialize), while local rules classpath must - // be looked up via tasks on projects - loadedRules = () => scalafixInterface().availableRules(), - terminalWidth = Some(JLineAccess.terminalWidth) - ).parser.parsed + // workaround https://github.com/sbt/sbt/issues/3572 by invoking directly what Def.inputTaskDyn would via macro + InputTask + .createDyn(InputTask.initParserAsInput(parser))( + Def.task(shellArgs => scalafixTask(shellArgs, config)) + ) + + private def scalafixTask( + shellArgs: ShellArgs, + config: Configuration + ): Def.Initialize[Task[Unit]] = + Def.taskDyn { val errorLogger = new PrintStream(LoggingOutputStream(streams.value.log, Level.Error)) val projectDepsInternal = products.in(ScalafixConfig).value ++ @@ -180,12 +186,13 @@ object ScalafixPlugin extends AutoPlugin { .value .flatMap(_.get(moduleID.key)) - if (shell0.rules.isEmpty && shell0.extra == List("--help")) { + if (shellArgs.rules.isEmpty && shellArgs.extra == List("--help")) { scalafixHelp } else { val scalafixConf = scalafixConfig.value.map(_.toPath) val (shell, mainInterface0) = scalafixArgsFromShell( - shell0, + shellArgs, + scalafixInterface.value, projectDepsExternal, scalafixResolvers.in(ThisBuild).value, projectDepsInternal @@ -212,7 +219,7 @@ object ScalafixPlugin extends AutoPlugin { } private def scalafixHelp: Def.Initialize[Task[Unit]] = Def.task { - scalafixInterface().withArgs(Arg.ParsedArgs(List("--help"))).run() + scalafixInterface.value().withArgs(Arg.ParsedArgs(List("--help"))).run() () } diff --git a/src/test/scala/scalafix/internal/sbt/SbtCompletionsSuite.scala b/src/test/scala/scalafix/internal/sbt/SbtCompletionsSuite.scala index a9cc2a41..590505dd 100644 --- a/src/test/scala/scalafix/internal/sbt/SbtCompletionsSuite.scala +++ b/src/test/scala/scalafix/internal/sbt/SbtCompletionsSuite.scala @@ -35,7 +35,7 @@ class SbtCompletionsSuite extends AnyFunSuite { val loadedRules = mainArgs.availableRules.toList val parser = new ScalafixCompletions( - workingDirectory = () => fs.workingDirectory.toAbsolutePath, + workingDirectory = fs.workingDirectory.toAbsolutePath, loadedRules = () => loadedRules, terminalWidth = None ).parser From 7de73f0fa335b08f4eebaca748f416e3cccb5abc Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Sat, 13 Jun 2020 00:55:13 +0200 Subject: [PATCH 2/2] introduce scalafixAll to run scalafix across configurations * explicitly scope all value lookups to the current configuration in order to prevent scalafixAll from lookuping up in the Zero config * make sure scalafix and scalafixAll share the same cache for a given configuration --- .../scala/scalafix/sbt/ScalafixPlugin.scala | 75 +++++++++++++++---- src/sbt-test/sbt-scalafix/cross-build/test | 3 +- src/sbt-test/sbt-scalafix/inconfig/test | 4 + src/sbt-test/skip-windows/caching/test | 18 +++++ 4 files changed, 83 insertions(+), 17 deletions(-) diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala index 78bc3db3..f73e81ab 100644 --- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala +++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala @@ -30,9 +30,14 @@ object ScalafixPlugin extends AutoPlugin { val scalafix: InputKey[Unit] = inputKey[Unit]( - "Run scalafix rule in this project and configuration. " + + "Run scalafix rule(s) in this project and configuration. " + "For example: scalafix RemoveUnusedImports. " + - "To run on test sources use test:scalafix." + "To run on test sources use test:scalafix or scalafixAll." + ) + 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." ) val scalafixCaching: SettingKey[Boolean] = settingKey[Boolean]( @@ -101,7 +106,10 @@ object ScalafixPlugin extends AutoPlugin { override lazy val projectSettings: Seq[Def.Setting[_]] = Seq(Compile, Test).flatMap(c => inConfig(c)(scalafixConfigSettings(c))) ++ inConfig(ScalafixConfig)(Defaults.configSettings) ++ - Seq(ivyConfigurations += ScalafixConfig) + Seq( + ivyConfigurations += ScalafixConfig, + scalafixAll := scalafixAllInputTask.evaluated + ) override lazy val globalSettings: Seq[Def.Setting[_]] = Seq( scalafixConfig := None, // let scalafix-cli try to infer $CWD/.scalafix.conf @@ -162,6 +170,28 @@ object ScalafixPlugin extends AutoPlugin { } + private def scalafixAllInputTask(): Def.Initialize[InputTask[Unit]] = + // workaround https://github.com/sbt/sbt/issues/3572 by invoking directly what Def.inputTaskDyn would via macro + InputTask + .createDyn(InputTask.initParserAsInput(parser))( + Def.task(shellArgs => scalafixAllTask(shellArgs, thisProject.value)) + ) + + private def scalafixAllTask( + shellArgs: ShellArgs, + project: ResolvedProject + ): Def.Initialize[Task[Unit]] = + Def.taskDyn { + val configsWithScalafixInputKey = project.settings + .map(_.key) + .filter(_.key == scalafix.key) + .flatMap(_.scope.config.toOption) + + configsWithScalafixInputKey + .map(config => scalafixTask(shellArgs, config)) + .joinWith(_.join.map(_ => ())) + } + private def scalafixInputTask( config: Configuration ): Def.Initialize[InputTask[Unit]] = @@ -173,11 +203,16 @@ object ScalafixPlugin extends AutoPlugin { private def scalafixTask( shellArgs: ShellArgs, - config: Configuration + config: ConfigKey ): Def.Initialize[Task[Unit]] = Def.taskDyn { val errorLogger = - new PrintStream(LoggingOutputStream(streams.value.log, Level.Error)) + new PrintStream( + LoggingOutputStream( + streams.in(config, scalafix).value.log, + Level.Error + ) + ) val projectDepsInternal = products.in(ScalafixConfig).value ++ internalDependencyClasspath.in(ScalafixConfig).value.map(_.data) val projectDepsExternal = @@ -189,7 +224,7 @@ object ScalafixPlugin extends AutoPlugin { if (shellArgs.rules.isEmpty && shellArgs.extra == List("--help")) { scalafixHelp } else { - val scalafixConf = scalafixConfig.value.map(_.toPath) + val scalafixConf = scalafixConfig.in(config).value.map(_.toPath) val (shell, mainInterface0) = scalafixArgsFromShell( shellArgs, scalafixInterface.value, @@ -198,7 +233,9 @@ object ScalafixPlugin extends AutoPlugin { projectDepsInternal ) val maybeNoCache = - if (shell.noCache || !scalafixCaching.value) Seq(Arg.NoCache) else Nil + if (shell.noCache || !scalafixCaching.in(config).value) + Seq(Arg.NoCache) + else Nil val mainInterface = mainInterface0 .withArgs(maybeNoCache: _*) .withArgs( @@ -226,25 +263,28 @@ object ScalafixPlugin extends AutoPlugin { private def scalafixSyntactic( mainInterface: ScalafixInterface, shellArgs: ShellArgs, - config: Configuration + config: ConfigKey ): Def.Initialize[Task[Unit]] = Def.task { val files = filesToFix(shellArgs, config).value - runArgs(mainInterface.withArgs(Arg.Paths(files)), streams.value) + runArgs( + mainInterface.withArgs(Arg.Paths(files)), + streams.in(config, scalafix).value + ) } private def scalafixSemantic( ruleNames: Seq[String], mainArgs: ScalafixInterface, shellArgs: ShellArgs, - config: Configuration + config: ConfigKey ): Def.Initialize[Task[Unit]] = Def.taskDyn { - val dependencies = allDependencies.value + val dependencies = allDependencies.in(config).value val files = filesToFix(shellArgs, config).value val withScalaInterface = mainArgs.withArgs( Arg.ScalaVersion(scalaVersion.value), - Arg.ScalacOptions(scalacOptions.value) + Arg.ScalacOptions(scalacOptions.in(config).value) ) val errors = new SemanticRuleValidator( new SemanticdbNotFound(ruleNames, scalaVersion.value, sbtVersion.value) @@ -253,9 +293,12 @@ object ScalafixPlugin extends AutoPlugin { Def.task { val semanticInterface = withScalaInterface.withArgs( Arg.Paths(files), - Arg.Classpath(fullClasspath.value.map(_.data.toPath)) + Arg.Classpath(fullClasspath.in(config).value.map(_.data.toPath)) + ) + runArgs( + semanticInterface, + streams.in(config, scalafix).value ) - runArgs(semanticInterface, streams.value) } } else { Def.task { @@ -396,7 +439,7 @@ object ScalafixPlugin extends AutoPlugin { } private def filesToFix( shellArgs: ShellArgs, - config: Configuration + config: ConfigKey ): Def.Initialize[Task[Seq[Path]]] = Def.taskDyn { // Dynamic task to avoid redundantly computing `unmanagedSources.value` @@ -407,7 +450,7 @@ object ScalafixPlugin extends AutoPlugin { } else { Def.task { for { - source <- unmanagedSources.in(config).in(scalafix).value + source <- unmanagedSources.in(config, scalafix).value if source.exists() if isScalaFile(source) } yield source.toPath diff --git a/src/sbt-test/sbt-scalafix/cross-build/test b/src/sbt-test/sbt-scalafix/cross-build/test index 97f5ffe9..67ee9790 100644 --- a/src/sbt-test/sbt-scalafix/cross-build/test +++ b/src/sbt-test/sbt-scalafix/cross-build/test @@ -1,6 +1,7 @@ # compile should not affect scalafix > compile +-> scalafixAll --test ProcedureSyntax -> scalafix --test ProcedureSyntax -> compile:scalafix --test ProcedureSyntax -> test:scalafix --test ProcedureSyntax @@ -17,7 +18,7 @@ > test:scalafix --test ProcedureSyntax -> it:scalafix --test ProcedureSyntax -> it:scalafix ProcedureSyntax +> scalafixAll ProcedureSyntax > it:scalafix --test ProcedureSyntax > javaProject/compile:scalafix ProcedureSyntax diff --git a/src/sbt-test/sbt-scalafix/inconfig/test b/src/sbt-test/sbt-scalafix/inconfig/test index 00e15e71..278899a0 100644 --- a/src/sbt-test/sbt-scalafix/inconfig/test +++ b/src/sbt-test/sbt-scalafix/inconfig/test @@ -1,5 +1,9 @@ -> ; example/scalafix --test ; example/it:scalafix --test +-> example/scalafixAll --test > example/it:scalafix > example/it:scalafix --test -> example/scalafix --test > tests/test +> example/scalafixAll +> example/scalafixAll --test +> example/scalafix --test diff --git a/src/sbt-test/skip-windows/caching/test b/src/sbt-test/skip-windows/caching/test index ed9e14f7..f10b99f4 100644 --- a/src/sbt-test/skip-windows/caching/test +++ b/src/sbt-test/skip-windows/caching/test @@ -205,6 +205,24 @@ $ exec chmod 000 src/test/scala/Valid.scala $ delete src/main/scala $ delete src/test/scala +# scalafixAll and scalafix should share the same cache per configuration +> set scalafixConfig := None +$ mkdir src/main/scala +$ mkdir src/test/scala +$ copy-file files/UnusedImports.scala src/main/scala/Valid.scala +$ copy-file files/ProcedureSyntax.scala src/test/scala/InitiallyInvalid.scala +-> scalafixAll --check ProcedureSyntax +$ exec chmod 000 src/main/scala/Valid.scala +> scalafix --check ProcedureSyntax +-> test:scalafix --check ProcedureSyntax +> test:scalafix ProcedureSyntax +$ exec chmod 000 src/test/scala/InitiallyInvalid.scala +> scalafix --check ProcedureSyntax +> test:scalafix --check ProcedureSyntax +> scalafixAll --check ProcedureSyntax +$ delete src/main/scala +$ delete src/test/scala + # make sure the cache is disabled when using `--stdout` > set scalafixConfig := None $ mkdir src/main/scala