From 0cee45d1d2776027e1f44f16e31655e998382923 Mon Sep 17 00:00:00 2001
From: Brice Jaglin <bjaglin@teads.tv>
Date: Fri, 10 Jul 2020 11:45:51 +0200
Subject: [PATCH] caching: remove optimization bringing false positive cache
 hits

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.
---
 src/main/scala/scalafix/sbt/ScalafixPlugin.scala | 12 ++++++------
 src/sbt-test/skip-windows/caching/test           |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala
index 2b2fc60c..84c37df5 100644
--- a/src/main/scala/scalafix/sbt/ScalafixPlugin.scala
+++ b/src/main/scala/scalafix/sbt/ScalafixPlugin.scala
@@ -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 {
diff --git a/src/sbt-test/skip-windows/caching/test b/src/sbt-test/skip-windows/caching/test
index 6a476ab0..0ea7b658 100644
--- a/src/sbt-test/skip-windows/caching/test
+++ b/src/sbt-test/skip-windows/caching/test
@@ -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
@@ -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