Merge pull request #2982 from daddykotex/dfrancoeur/2942-gitignore

fthomas · web-flow · commit d6b5b904d0bf · 2023-02-26T17:50:04.000+01:00
git check-ignore before adding
diff --git a/modules/core/src/main/scala/org/scalasteward/core/edit/hooks/HookExecutor.scala b/modules/core/src/main/scala/org/scalasteward/core/edit/hooks/HookExecutor.scala
@@ -95,9 +95,21 @@ final class HookExecutor[F[_]](implicit
         oldContent <- fileAlg.readFile(file)
         newContent = oldContent.fold(newLines)(_ + "\n" + newLines)
         _ <- fileAlg.writeFile(file, newContent)
-        _ <- gitAlg.add(repo, file.pathAsString)
-        blameIgnoreCommitMsg = CommitMsg(s"Add '${commitMsg.title}' to $gitBlameIgnoreRevsName")
-        maybeBlameIgnoreCommit <- gitAlg.commitAllIfDirty(repo, blameIgnoreCommitMsg)
+        pathAsString = file.pathAsString
+
+        addAndCommit = gitAlg.add(repo, pathAsString).flatMap { _ =>
+          val blameIgnoreCommitMsg =
+            CommitMsg(s"Add '${commitMsg.title}' to $gitBlameIgnoreRevsName")
+          gitAlg.commitAllIfDirty(repo, blameIgnoreCommitMsg)
+        }
+        maybeBlameIgnoreCommit <- gitAlg
+          .checkIgnore(repo, pathAsString)
+          .ifM(
+            logger
+              .warn(s"Impossible to add '$pathAsString' because it is git ignored.")
+              .as(Option.empty[Commit]),
+            addAndCommit
+          )
       } yield maybeBlameIgnoreCommit
     } else F.pure(None)
 }
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
@@ -22,7 +22,7 @@ import cats.syntax.all._
 import org.http4s.Uri
 import org.scalasteward.core.application.Config.GitCfg
 import org.scalasteward.core.git.FileGitAlg.{dotdot, gitCmd}
-import org.scalasteward.core.io.process.SlurpOptions
+import org.scalasteward.core.io.process.{ProcessFailedException, SlurpOptions}
 import org.scalasteward.core.io.{FileAlg, ProcessAlg, WorkspaceAlg}
 import org.scalasteward.core.util.Nel
 
@@ -47,6 +47,11 @@ final class FileGitAlg[F[_]](config: GitCfg)(implicit
   override def checkoutBranch(repo: File, branch: Branch): F[Unit] =
     git_("checkout", branch.name)(repo).void
 
+  override def checkIgnore(repo: File, file: String): F[Boolean] =
+    git_("check-ignore", file)(repo)
+      .as(true)
+      .recover { case ex: ProcessFailedException if ex.exitValue === 1 => false }
+
   override def clone(repo: File, url: Uri): F[Unit] =
     for {
       rootDir <- workspaceAlg.rootDir
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
@@ -34,6 +34,8 @@ trait GenGitAlg[F[_], Repo] {
 
   def checkoutBranch(repo: Repo, branch: Branch): F[Unit]
 
+  def checkIgnore(repo: Repo, file: String): F[Boolean]
+
   def clone(repo: Repo, url: Uri): F[Unit]
 
   def cloneExists(repo: Repo): F[Boolean]
@@ -105,6 +107,9 @@ trait GenGitAlg[F[_], Repo] {
       override def checkoutBranch(repo: A, branch: Branch): F[Unit] =
         f(repo).flatMap(self.checkoutBranch(_, branch))
 
+      override def checkIgnore(repo: A, file: String): F[Boolean] =
+        f(repo).flatMap(self.checkIgnore(_, file))
+
       override def clone(repo: A, url: Uri): F[Unit] =
         f(repo).flatMap(self.clone(_, url))
 
diff --git a/modules/core/src/main/scala/org/scalasteward/core/io/process.scala b/modules/core/src/main/scala/org/scalasteward/core/io/process.scala
@@ -148,7 +148,7 @@ object process {
   final class ProcessFailedException(
       args: Args,
       buffer: ListBuffer[String],
-      exitValue: Int
+      val exitValue: Int
   ) extends IOException(makeMessage(args, buffer)(s"exited with code $exitValue."))
 
   final class ProcessTimedOutException(
diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/mill/MillAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/mill/MillAlgTest.scala
@@ -28,7 +28,8 @@ class MillAlgTest extends FunSuite {
       "show",
       extractDeps
     )
-    val initial = MockState.empty.copy(commandOutputs = Map(millCmd -> List("""{"modules":[]}""")))
+    val initial =
+      MockState.empty.copy(commandOutputs = Map(millCmd -> Right(List("""{"modules":[]}"""))))
     val state = millAlg.getDependencies(buildRoot).runS(initial).unsafeRunSync()
     val expected = initial.copy(
       trace = Vector(
diff --git a/modules/core/src/test/scala/org/scalasteward/core/edit/hooks/HookExecutorTest.scala b/modules/core/src/test/scala/org/scalasteward/core/edit/hooks/HookExecutorTest.scala
@@ -1,5 +1,6 @@
 package org.scalasteward.core.edit.hooks
 
+import cats.data.NonEmptyList
 import cats.syntax.all._
 import munit.CatsEffectSuite
 import org.scalasteward.core.TestInstances.{dummyRepoCache, dummySha1}
@@ -15,6 +16,9 @@ import org.scalasteward.core.repoconfig.{PostUpdateHookConfig, RepoConfig, Scala
 import org.scalasteward.core.scalafmt.ScalafmtAlg.opts
 import org.scalasteward.core.scalafmt.{scalafmtArtifactId, scalafmtBinary, scalafmtGroupId}
 import org.scalasteward.core.util.Nel
+import org.scalasteward.core.io.process.ProcessFailedException
+import org.scalasteward.core.io.process
+import scala.collection.mutable.ListBuffer
 
 class HookExecutorTest extends CatsEffectSuite {
   private val repo = Repo("scala-steward-org", "scala-steward")
@@ -27,23 +31,51 @@ class HookExecutorTest extends CatsEffectSuite {
     state.map(assertEquals(_, MockState.empty))
   }
 
-  test("scalafmt: enabled by config") {
-    val update = (scalafmtGroupId % scalafmtArtifactId % "2.7.4" %> "2.7.5").single
-    val initial = MockState.empty.copy(commandOutputs =
-      Map(
-        FileGitAlg.gitCmd.toList ++
-          List("status", "--porcelain", "--untracked-files=no", "--ignore-submodules") ->
-          List("build.sbt"),
-        FileGitAlg.gitCmd.toList ++ List("rev-parse", "--verify", "HEAD") ->
-          List(dummySha1.value.value)
+  Seq(true, false).foreach { case blameRevIgnored =>
+    val isIgnored = if (blameRevIgnored) "ignored" else "not ignored"
+
+    test(s"scalafmt: enabled by config / $gitBlameIgnoreRevsName $isIgnored") {
+      val gitBlameIgnoreRevs = repoDir / gitBlameIgnoreRevsName
+      val update = (scalafmtGroupId % scalafmtArtifactId % "2.7.4" %> "2.7.5").single
+      val initial = MockState.empty.copy(commandOutputs =
+        Map(
+          FileGitAlg.gitCmd.toList ++
+            List("status", "--porcelain", "--untracked-files=no", "--ignore-submodules") ->
+            Right(List("build.sbt")),
+          FileGitAlg.gitCmd.toList ++ List("rev-parse", "--verify", "HEAD") ->
+            Right(List(dummySha1.value.value)),
+          FileGitAlg.gitCmd.toList ++ List("check-ignore", gitBlameIgnoreRevs.pathAsString) ->
+            (if (blameRevIgnored) Right(List.empty) else Left(dummyProcessError))
+        )
       )
-    )
-    val gitBlameIgnoreRevs = repoDir / gitBlameIgnoreRevsName
-    val state = FileAlgTest.ioFileAlg.deleteForce(gitBlameIgnoreRevs) >>
-      hookExecutor.execPostUpdateHooks(data, update).runS(initial)
+      val state = FileAlgTest.ioFileAlg.deleteForce(gitBlameIgnoreRevs) >>
+        hookExecutor.execPostUpdateHooks(data, update).runS(initial)
 
-    val expected = initial.copy(
-      trace = Vector(
+      val logIfIgnored =
+        if (blameRevIgnored)
+          Vector(Log(s"Impossible to add '$gitBlameIgnoreRevs' because it is git ignored."))
+        else
+          Vector(
+            Cmd(gitCmd(repoDir), "add", gitBlameIgnoreRevs.pathAsString),
+            Cmd(
+              gitCmd(repoDir),
+              "status",
+              "--porcelain",
+              "--untracked-files=no",
+              "--ignore-submodules"
+            ),
+            Cmd(
+              gitCmd(repoDir),
+              "commit",
+              "--all",
+              "--no-gpg-sign",
+              "-m",
+              s"Add 'Reformat with scalafmt 2.7.5' to $gitBlameIgnoreRevsName"
+            ),
+            Cmd(gitCmd(repoDir), "rev-parse", "--verify", "HEAD")
+          )
+
+      val traces = Vector(
         Log(
           "Executing post-update hook for org.scalameta:scalafmt-core with command 'scalafmt --non-interactive'"
         ),
@@ -70,30 +102,19 @@ class HookExecutorTest extends CatsEffectSuite {
         Cmd(gitCmd(repoDir), "rev-parse", "--verify", "HEAD"),
         Cmd("read", gitBlameIgnoreRevs.pathAsString),
         Cmd("write", gitBlameIgnoreRevs.pathAsString),
-        Cmd(gitCmd(repoDir), "add", gitBlameIgnoreRevs.pathAsString),
-        Cmd(
-          gitCmd(repoDir),
-          "status",
-          "--porcelain",
-          "--untracked-files=no",
-          "--ignore-submodules"
-        ),
-        Cmd(
-          gitCmd(repoDir),
-          "commit",
-          "--all",
-          "--no-gpg-sign",
-          "-m",
-          s"Add 'Reformat with scalafmt 2.7.5' to $gitBlameIgnoreRevsName"
-        ),
-        Cmd(gitCmd(repoDir), "rev-parse", "--verify", "HEAD")
-      ),
-      files = Map(
-        gitBlameIgnoreRevs -> s"# Scala Steward: Reformat with scalafmt 2.7.5\n${dummySha1.value.value}\n"
+        Cmd(gitCmd(repoDir), "check-ignore", gitBlameIgnoreRevs.pathAsString)
+      ) ++
+        logIfIgnored ++ Vector(
+        )
+      val expected = initial.copy(
+        trace = traces,
+        files = Map(
+          gitBlameIgnoreRevs -> s"# Scala Steward: Reformat with scalafmt 2.7.5\n${dummySha1.value.value}\n"
+        )
       )
-    )
 
-    state.map(assertEquals(_, expected))
+      state.map(assertEquals(_, expected))
+    }
   }
 
   test("scalafmt: disabled by config") {
@@ -166,4 +187,7 @@ class HookExecutorTest extends CatsEffectSuite {
 
     state.map(assertEquals(_, expected))
   }
+
+  private val dummyProcessError =
+    new ProcessFailedException(process.Args(NonEmptyList.of("cmd")), ListBuffer.empty, 1)
 }
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
@@ -16,6 +16,32 @@ import org.scalasteward.core.util.Nel
 class FileGitAlgTest extends CatsEffectSuite {
   private val rootDir = mockRoot / "git-tests"
 
+  test("add with .gitignore") {
+    val repo = rootDir / "branchgitignpreAuthors"
+    for {
+      _ <- ioAuxGitAlg.createRepo(repo)
+
+      gitIgnoreFile = repo / ".gitignore"
+      _ <- ioFileAlg.writeFile(gitIgnoreFile, "ignored.txt")
+      _ <- ioAuxGitAlg.addFiles(repo, gitIgnoreFile)
+
+      ignoredFile = repo / "ignored.txt"
+      _ <- ioFileAlg.writeFile(ignoredFile, "irrelevant")
+      ignoredFileCheck <- ioGitAlg.checkIgnore(repo, ignoredFile.pathAsString)
+
+      notIgnoredFile = repo / "not-ignored.txt"
+      _ <- ioFileAlg.writeFile(notIgnoredFile, "irrelevant")
+      notIgnoredFileCheck <- ioGitAlg.checkIgnore(repo, notIgnoredFile.pathAsString)
+
+    } yield {
+      assert(ignoredFileCheck, "The file is in .gitignore, checkIgnore should return true.")
+      assert(
+        !notIgnoredFileCheck,
+        "The file is not in .gitignore, checkIgnore should return false."
+      )
+    }
+  }
+
   test("branchAuthors") {
     val repo = rootDir / "branchAuthors"
     for {
diff --git a/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala b/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala
@@ -1,19 +1,26 @@
 package org.scalasteward.core.io
 
 import cats.data.Kleisli
+import cats.effect.IO
 import org.scalasteward.core.application.Config.ProcessCfg
 import org.scalasteward.core.mock.MockEff
 
 object MockProcessAlg {
   def create(config: ProcessCfg): ProcessAlg[MockEff] =
     new ProcessAlg(config)({ args =>
-      Kleisli {
-        _.modify { s =>
-          val cmd = args.workingDirectory.map(_.toString).toList ++ args.command.toList
-          val s1 = s.exec(cmd, args.extraEnv: _*)
-          val a = s.commandOutputs.getOrElse(args.command.toList, List.empty)
-          (s1, a)
-        }
+      Kleisli { x =>
+        for {
+          state <- x.get
+          cmd = args.workingDirectory.map(_.toString).toList ++ args.command.toList
+          newState = state.exec(cmd, args.extraEnv: _*)
+          res <- x
+            .set(newState)
+            .flatMap { _ =>
+              state.commandOutputs
+                .getOrElse(args.command.toList, Right(List.empty))
+                .fold(err => IO.raiseError(err), a => IO.pure(a))
+            }
+        } yield res
       }
     })
 }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/mock/MockState.scala b/modules/core/src/test/scala/org/scalasteward/core/mock/MockState.scala
@@ -10,7 +10,7 @@ import org.scalasteward.core.mock.MockState.TraceEntry.{Cmd, Log}
 
 final case class MockState(
     trace: Vector[TraceEntry],
-    commandOutputs: Map[List[String], List[String]],
+    commandOutputs: Map[List[String], Either[Throwable, List[String]]],
     files: Map[File, String],
     uris: Map[Uri, String],
     clientResponses: HttpApp[MockEff]
