Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add --triggered support when scalafixOnCompile is enabled #169

Merged
merged 3 commits into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/main/scala/scalafix/sbt/ScalafixPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ 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. " +
"Run Scalafix rule(s) declared in .scalafix.conf on compilation and fail on lint errors." +
"Default rules & rule configuration can be overridden using the `triggered` section." +
"Off by default. When enabled, caching will be automatically activated, " +
"but can be disabled with `scalafixCaching := false`."
)
Expand Down Expand Up @@ -107,7 +108,7 @@ object ScalafixPlugin extends AutoPlugin {
compile.value // evaluated first, before the potential scalafix evaluation
if (scalafixOnCompile.value)
scalafix
.toTask("")
.toTask(" --triggered")
.map(_ => oldCompile)
else Def.task(oldCompile)
}.value,
Expand Down Expand Up @@ -529,13 +530,29 @@ object ScalafixPlugin extends AutoPlugin {
// 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
case "--triggered" =>
// If there is a triggered section in the config file, --triggered flag should be accounted.
// If not, --triggered can share the same cache.
checkIfTriggeredSectionExists
case _ => true
}
write(cacheKeys)
case Arg.NoCache =>
throw StampingImpossible
}

private lazy val checkIfTriggeredSectionExists: Boolean = {
val confInArgs = interface.args
.collect { case Arg.Config(conf) => conf }
.flatten
.lastOption
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

val configFile = confInArgs.fold(file(".scalafix.conf"))(_.toFile())
if (configFile.exists()) {
val lines = IO.readLines(configFile)
lines.exists(_.trim.startsWith("triggered"))
} else false
}

def stampFile(file: File): String = {
// ensure the file exists and is not a directory
if (file.isFile)
Expand Down
9 changes: 8 additions & 1 deletion src/sbt-test/sbt-scalafix/scalafixOnCompile/.scalafix.conf
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
rules = [DisableSyntax, RemoveUnused]
DisableSyntax.noNulls = true
DisableSyntax.noNulls = true

DisableSyntax.noVars = true
RemoveUnused.privates = false
triggered = {
DisableSyntax.noVars = false
RemoveUnused.privates = true
}
7 changes: 6 additions & 1 deletion src/sbt-test/sbt-scalafix/scalafixOnCompile/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ inThisBuild(
scalaVersion := Versions.scala212,
scalacOptions ++= List(
"-Yrangepos",
"-Ywarn-unused-import"
"-Ywarn-unused"
)
)
)
Expand All @@ -21,3 +21,8 @@ lazy val rewrite = project
addCompilerPlugin(scalafixSemanticdb)
)
.settings(scalafixConfigSettings(IntegrationTest): _*)

lazy val triggered = project
.settings(
addCompilerPlugin(scalafixSemanticdb)
)
9 changes: 8 additions & 1 deletion src/sbt-test/sbt-scalafix/scalafixOnCompile/test
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,11 @@

# check configuration granularity for scalafixOnCompile
> set scalafixOnCompile.in(lint, Test) := false
> lint/test:compile
> lint/test:compile

# check that triggered rules are respected on compilation, and ignored on explicit invocation.
# `private var` is detected by `DisableSyntax.noVars = true` on explicit invocation,
# and removed by `triggered.RemoveUnused.privates = true` on compilation.
-> triggered/test:scalafix
> triggered/test:compile
> triggered/test:scalafix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Variables {
private var a = 1
def b: Int = return 2
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
rules = [DisableSyntax]
DisableSyntax.noVars = true

# Whitespaces are intentional for testing.
triggered.DisableSyntax.noNulls = true
bjaglin marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 34 additions & 0 deletions src/sbt-test/skip-windows/caching/test
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,40 @@ $ exec chmod 000 src/main/scala/InitiallyValid.scala
-> scalafix --check ProcedureSyntax
$ delete src/main/scala

# files should be re-checked in scalafix run without --triggered even after scalafix runs onCompile, if a triggered configuration is defined
> set scalafixConfig := Some(file("files/Triggered.scalafix.conf"))
> set scalafixOnCompile := true
$ mkdir src/main/scala
$ copy-file files/Valid.scala src/main/scala/Valid.scala
> compile
$ exec chmod 000 src/main/scala/Valid.scala
> scalafix --triggered --check
-> scalafix --check
> set scalafixOnCompile := false
$ delete src/main/scala

# files should be re-checked in scalafix --triggered run even after scalafix run without --triggered, if a triggered configuration is defined
> set scalafixConfig := Some(file("files/Triggered.scalafix.conf"))
$ mkdir src/main/scala
$ copy-file files/Valid.scala src/main/scala/Valid.scala
> scalafix
$ exec chmod 000 src/main/scala/Valid.scala
> scalafix --check
-> scalafix --triggered --check
$ delete src/main/scala

# files should not be re-checked in scalafix explicit invocation after scalafix runs onCompile if no triggered configuration is defined
> set scalafixConfig := Some(file("files/DisableSyntaxVar.scalafix.conf"))
> set scalafixOnCompile := true
$ mkdir src/main/scala
$ copy-file files/Valid.scala src/main/scala/Valid.scala
> compile
$ exec chmod 000 src/main/scala/Valid.scala
> scalafix --triggered --check
> scalafix --check
> set scalafixOnCompile := false
$ delete src/main/scala

# files should not be re-checked if all requested rules have previously been run across invocations
> set scalafixConfig := Some(file("files/DisableSyntaxVar.scalafix.conf"))
$ mkdir src/test/scala
Expand Down