Repository: scala-steward-org/scala-steward
Pull request #1809 (2020-12-14 15:38:21)

Run Scalafix migrations only if version can be bumped
This does not run any Scalafix migration if the version cannot be
bumped. It prevents PRs that only contains changes from Scalafix
migrations.


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
index eca0d938..1c97576d 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
@@ -27,7 +27,7 @@ import org.scalasteward.core.git
 import org.scalasteward.core.git.{Commit, GitAlg}
 import org.scalasteward.core.io.{isSourceFile, FileAlg, WorkspaceAlg}
 import org.scalasteward.core.repoconfig.RepoConfig
-import org.scalasteward.core.scalafix.MigrationAlg
+import org.scalasteward.core.scalafix.{Migration, MigrationAlg}
 import org.scalasteward.core.util._
 import org.scalasteward.core.vcs.data.Repo
 
@@ -47,13 +47,21 @@ final class EditAlg[F[_]](implicit
       case None =>
         logger.warn("No files found that contain the current version").as(Nil)
       case Some(files) =>
-        for {
-          cs1 <- runScalafixMigrations(repo, update)
-          cs2 <- bumpVersion(repo, repoConfig, update, files)
-          cs3 <-
-            if (cs2.isEmpty) F.pure(Nil)
-            else hookExecutor.execPostUpdateHooks(repo, repoConfig, update)
-        } yield cs1 ++ cs2 ++ cs3
+        bumpVersion(update, files).flatMap {
+          case false => logger.warn("Unable to bump version").as(Nil)
+          case true =>
+            val migrations = migrationAlg.findMigrations(update)
+            for {
+              cs1 <-
+                if (migrations.isEmpty) F.pure(Nil)
+                else
+                  gitAlg.discardChanges(repo) *>
+                    runScalafixMigrations(repo, migrations) <*
+                    bumpVersion(update, files)
+              cs2 <- gitAlg.commitAllIfDirty(repo, git.commitMsgFor(update, repoConfig.commits))
+              cs3 <- hookExecutor.execPostUpdateHooks(repo, repoConfig, update)
+            } yield cs1 ++ cs2 ++ cs3
+        }
     }
 
   private def findFilesContainingCurrentVersion(
@@ -66,8 +74,8 @@ final class EditAlg[F[_]](implicit
       fileAlg.findFiles(repoDir, fileFilter, _.contains(update.currentVersion)).map(Nel.fromList)
     }
 
-  private def runScalafixMigrations(repo: Repo, update: Update): F[List[Commit]] =
-    migrationAlg.findMigrations(update).traverseFilter { migration =>
+  private def runScalafixMigrations(repo: Repo, migrations: List[Migration]): F[List[Commit]] =
+    migrations.traverseFilter { migration =>
       logger.info(s"Running migration $migration") >>
         buildToolDispatcher.runMigrations(repo, Nel.one(migration)).attempt.flatMap {
           case Right(_) =>
@@ -79,19 +87,11 @@ final class EditAlg[F[_]](implicit
         }
     }
 
-  private def bumpVersion(
-      repo: Repo,
-      repoConfig: RepoConfig,
-      update: Update,
-      files: Nel[File]
-  ): F[List[Commit]] = {
+  private def bumpVersion(update: Update, files: Nel[File]): F[Boolean] = {
     val actions = UpdateHeuristic.all.map { heuristic =>
       logger.info(s"Trying heuristic '${heuristic.name}'") >>
         fileAlg.editFiles(files, heuristic.replaceVersion(update))
     }
-    bindUntilTrue(actions).ifM(
-      gitAlg.commitAllIfDirty(repo, git.commitMsgFor(update, repoConfig.commits)).map(_.toList),
-      logger.warn("Unable to bump version").as(Nil)
-    )
+    bindUntilTrue(actions)
   }
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/git/FileGitAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/git/FileGitAlg.scala
index 2243bcd8..f3e1e067 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/git/FileGitAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/git/FileGitAlg.scala
@@ -66,6 +66,9 @@ final class FileGitAlg[F[_]](config: Config)(implicit
     git("rev-parse", "--abbrev-ref", Branch.head.name)(repo)
       .map(lines => Branch(lines.mkString.trim))
 
+  override def discardChanges(repo: File): F[Unit] =
+    git("checkout", "--", ".")(repo).void
+
   override def findFilesContaining(repo: File, string: String): F[List[String]] =
     git("grep", "-I", "--fixed-strings", "--files-with-matches", string)(repo)
       .handleError(_ => List.empty[String])
diff --git a/modules/core/src/main/scala/org/scalasteward/core/git/GenGitAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/git/GenGitAlg.scala
index e3cea4da..6ef9aca7 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/git/GenGitAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/git/GenGitAlg.scala
@@ -42,6 +42,9 @@ trait GenGitAlg[F[_], Repo] {
 
   def currentBranch(repo: Repo): F[Branch]
 
+  /** Discards unstaged changes. */
+  def discardChanges(repo: Repo): F[Unit]
+
   def findFilesContaining(repo: Repo, string: String): F[List[String]]
 
   /** Returns `true` if merging `branch` into `base` results in merge conflicts. */
@@ -99,6 +102,9 @@ trait GenGitAlg[F[_], Repo] {
       override def currentBranch(repo: A): F[Branch] =
         f(repo).flatMap(self.currentBranch)
 
+      override def discardChanges(repo: A): F[Unit] =
+        f(repo).flatMap(self.discardChanges)
+
       override def findFilesContaining(repo: A, string: String): F[List[String]] =
         f(repo).flatMap(self.findFilesContaining(_, string))
 
diff --git a/project/build.properties b/project/build.properties
index c06db1bb..7de0a938 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,1 +1,1 @@
-sbt.version=1.4.5
+sbt.version=1.4.4


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
index 23847486..3c7a95a5 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
@@ -6,6 +6,7 @@ import org.scalasteward.core.data.{GroupId, Update}
 import org.scalasteward.core.mock.MockContext.{config, editAlg}
 import org.scalasteward.core.mock.MockState
 import org.scalasteward.core.repoconfig.RepoConfig
+import org.scalasteward.core.scalafmt.scalafmtBinary
 import org.scalasteward.core.util.Nel
 import org.scalasteward.core.vcs.data.Repo
 import org.scalatest.funsuite.AnyFunSuite
@@ -79,6 +80,8 @@ class EditAlgTest extends AnyFunSuite with Matchers {
         List("read", scalafmtConf.pathAsString),
         List("read", scalafmtConf.pathAsString),
         List("write", scalafmtConf.pathAsString),
+        envVars ++ (repoDir.toString :: gitStatus),
+        List("VAR1=val1", "VAR2=val2", repoDir.toString, scalafmtBinary, "--non-interactive"),
         envVars ++ (repoDir.toString :: gitStatus)
       ),
       logs = Vector(
@@ -89,7 +92,8 @@ class EditAlgTest extends AnyFunSuite with Matchers {
         (None, "Trying heuristic 'sliding'"),
         (None, "Trying heuristic 'completeGroupId'"),
         (None, "Trying heuristic 'groupId'"),
-        (None, "Trying heuristic 'specific'")
+        (None, "Trying heuristic 'specific'"),
+        (None, "Executing post-update hook for org.scalameta:scalafmt-core")
       ),
       files = Map(
         scalafmtConf ->
diff --git a/modules/core/src/test/scala/org/scalasteward/core/git/FileGitAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/git/FileGitAlgTest.scala
index 41ad8ae9..a8911a83 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/git/FileGitAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/git/FileGitAlgTest.scala
@@ -70,6 +70,24 @@ class FileGitAlgTest extends AnyFunSuite with Matchers {
     p.unsafeRunSync() shouldBe master
   }
 
+  test("discardChanges") {
+    val repo = rootDir / "discardChanges"
+    val p = for {
+      _ <- supplement.createRepo(repo)
+      file = repo / "test.txt"
+      _ <- ioFileAlg.writeFile(file, "hello")
+      _ <- supplement.git("add", "test.txt")(repo)
+      _ <- ioGitAlg.commitAll(repo, "Add test.txt")
+      _ <- ioFileAlg.writeFile(file, "world")
+      before <- ioFileAlg.readFile(file)
+      _ <- ioGitAlg.discardChanges(repo)
+      after <- ioFileAlg.readFile(file)
+    } yield (before, after)
+    val (before, after) = p.unsafeRunSync()
+    before shouldBe Some("world")
+    after shouldBe Some("hello")
+  }
+
   test("findFilesContaining") {
     val repo = rootDir / "findFilesContaining"
     val p = for {