Skip to content

Commit

Permalink
caching: remove optimization bringing false positive cache hits
Browse files Browse the repository at this point in the history
If a file is patched during a scalafix run, it's not safe to assume
that all rules observed the updated content of that file and thus can
be skipped next time. It is if we have only one rule (which we could
check) or if the patch is applied after the first rule (which we can't
tell). For correctness, stamping should happen before rule execution.
  • Loading branch information
github-brice-jaglin committed Jul 10, 2020
1 parent 3800f16 commit 0cee45d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
12 changes: 6 additions & 6 deletions src/main/scala/scalafix/sbt/ScalafixPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -491,15 +491,15 @@ object ScalafixPlugin extends AutoPlugin {
}

def diffWithPreviousRun[T](f: (Boolean, ChangeReport[File]) => T): T = {
val tracker = Tracked.inputChanged(streams.cacheDirectory / "inputs") {
(inputChanged: Boolean, _: Seq[Arg.CacheKey]) =>
val diffOutputs: Difference = Tracked.diffOutputs(
streams.cacheDirectory / "outputs",
val tracker = Tracked.inputChanged(streams.cacheDirectory / "args") {
(argsChanged: Boolean, _: Seq[Arg.CacheKey]) =>
val diffInputs: Difference = Tracked.diffInputs(
streams.cacheDirectory / "targets",
lastModifiedStyle
)
diffOutputs(paths.map(_.toFile).toSet) {
diffInputs(paths.map(_.toFile).toSet) {
diffTargets: ChangeReport[File] =>
f(inputChanged, diffTargets)
f(argsChanged, diffTargets)
}
}
Try(tracker(cacheKeyArgs)).recover {
Expand Down
8 changes: 4 additions & 4 deletions src/sbt-test/skip-windows/caching/test
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,17 @@ $ exec chmod 000 src/main/scala/Valid.scala
> scalafix --check
$ delete src/main/scala

# a file patched during the last run should be cached
# a file patched during the last run should not be cached as some rules might not have observed the latest content
> set scalafixConfig := None
$ mkdir src/test/scala
$ copy-file files/ProcedureSyntax.scala src/test/scala/ToPatch.scala
$ copy-file files/Valid.scala src/test/scala/Valid.scala
# avoid rounding in mtime that could cause false negatives in `newer`
$ sleep 1000
> test:scalafix ProcedureSyntax
> test:scalafix ProcedureSyntax DisableSyntax
$ newer src/test/scala/ToPatch.scala src/test/scala/Valid.scala
$ exec chmod 000 src/test/scala/ToPatch.scala
$ exec chmod 000 src/test/scala/Valid.scala
> test:scalafix ProcedureSyntax
-> test:scalafix ProcedureSyntax DisableSyntax
$ delete src/test/scala

# an added file after a successful run should be checked
Expand Down Expand Up @@ -216,6 +215,7 @@ $ exec chmod 000 src/main/scala/Valid.scala
> scalafix --check ProcedureSyntax
-> test:scalafix --check ProcedureSyntax
> test:scalafix ProcedureSyntax
> test:scalafix ProcedureSyntax
$ exec chmod 000 src/test/scala/InitiallyInvalid.scala
> scalafix --check ProcedureSyntax
> test:scalafix --check ProcedureSyntax
Expand Down

0 comments on commit 0cee45d

Please sign in to comment.