Use GitAlg.findFilesContaining in ScannerAlg

This implements the idea mentioned in #2994 (comment) `ScannerAlg` now uses `GitAlg.findFilesContaining` instead of our own `FileAlg.findFiles` to find files that contain versions and groupIds. This simplifies `findPathsContaining` because we do not need the gitignore dependency anymore to skip files that are ignored by Git since `git grep`, which is used by `GitAlg.findFilesContaining`, does this automatically for us. The biggest impact of this change in the code base is in the tests. `EditAlgTest` and `RewriteTest` now need to actually execute `git grep` to find files that should be edited. For this, the `execCommands` flag is used (#3005) and Git repositories are created for these tests. One advantage of this change is that we mock less and the `MockState` traces are closer to what Steward is actually doing in production.
scala-steward-org · Nov 17, 2023 · c5589ff · c5589ff
1 parent 107f04a
commit c5589ff
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 167 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -67,6 +67,10 @@ jobs:
       - name: Check that workflows are up to date
         run: sbt githubWorkflowCheck
 
+      - uses: coursier/setup-action@v1
+        with:
+          apps: scalafmt
+
       - name: Build project
         run: sbt '++ ${{ matrix.scala }}' validate
 

diff --git a/build.sbt b/build.sbt
@@ -53,6 +53,8 @@ ThisBuild / githubWorkflowPublish := Seq(
 ThisBuild / githubWorkflowJavaVersions := Seq(JavaSpec(Temurin, "17"), JavaSpec(Temurin, "11"))
 ThisBuild / githubWorkflowBuild :=
   Seq(
+    WorkflowStep
+      .Use(UseRef.Public("coursier", "setup-action", "v1"), params = Map("apps" -> "scalafmt")),
     WorkflowStep.Sbt(List("validate"), name = Some("Build project")),
     WorkflowStep.Use(
       UseRef.Public("codecov", "codecov-action", "v3"),
@@ -122,7 +124,6 @@ lazy val core = myCrossProject("core")
       Dependencies.decline,
       Dependencies.fs2Core,
       Dependencies.fs2Io,
-      Dependencies.gitignore,
       Dependencies.http4sCirce,
       Dependencies.http4sClient,
       Dependencies.http4sCore,

diff --git a/modules/core/src/main/scala/org/scalasteward/core/edit/update/ScannerAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/edit/update/ScannerAlg.scala
@@ -16,20 +16,20 @@
 
 package org.scalasteward.core.edit.update
 
-import better.files.File
 import cats.effect.Concurrent
-import cats.syntax.functor._
-import com.github.arturopala.gitignore.GitIgnore
+import cats.syntax.all._
 import fs2.Stream
 import org.scalasteward.core.data.{Dependency, Repo, Version}
 import org.scalasteward.core.edit.update.data.{ModulePosition, VersionPosition}
+import org.scalasteward.core.git.GitAlg
 import org.scalasteward.core.io.{FileAlg, FileData, WorkspaceAlg}
 import org.scalasteward.core.repoconfig.RepoConfig
 import org.scalasteward.core.util.Nel
 
 /** Scans all files that Scala Steward is allowed to edit for version and module positions. */
 final class ScannerAlg[F[_]](implicit
     fileAlg: FileAlg[F],
+    gitAlg: GitAlg[F],
     workspaceAlg: WorkspaceAlg[F],
     F: Concurrent[F]
 ) {
@@ -58,35 +58,15 @@ final class ScannerAlg[F[_]](implicit
       .compile
       .foldMonoid
 
-  private def getGitIgnore(repoDir: File): F[Option[GitIgnore]] = {
-    val gitIgnore = repoDir / ".gitignore"
-    fileAlg.readFile(gitIgnore).map(_.map(GitIgnore.parse))
-  }
-
   private def findPathsContaining(
       repo: Repo,
       config: RepoConfig,
       string: String
-  ): Stream[F, FileData] = {
-    def fileFilter(repoDir: File, maybeGitIgnore: Option[GitIgnore]) = (file: File) => {
-      val path = repoDir.relativize(file).toString
-      val notDotGit = !path.startsWith(".git/")
-      val onlyKeepConfiguredExtensions =
-        config.updates.fileExtensionsOrDefault.exists(path.endsWith)
-      val allowedByGitIgnore = maybeGitIgnore.map(_.isAllowed(path)).getOrElse(true)
-      val cond = notDotGit &&
-        onlyKeepConfiguredExtensions &&
-        allowedByGitIgnore
-      Option.when(cond)(path)
+  ): Stream[F, FileData] =
+    Stream.eval(workspaceAlg.repoDir(repo)).flatMap { repoDir =>
+      Stream
+        .evalSeq(gitAlg.findFilesContaining(repo, string))
+        .filter(path => config.updates.fileExtensionsOrDefault.exists(path.endsWith))
+        .evalMapFilter(path => fileAlg.readFile(repoDir / path).map(_.map(FileData(path, _))))
     }
-    val contentFilter = (content: String) => Some(content).filter(_.contains(string))
-
-    for {
-      repoDir <- Stream.eval(workspaceAlg.repoDir(repo))
-      maybeRootGitIgnore <- Stream.eval(getGitIgnore(repoDir))
-      files <- fileAlg
-        .findFiles(repoDir, fileFilter(repoDir, maybeRootGitIgnore), contentFilter)
-        .map(FileData.tupled)
-    } yield files
-  }
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/io/FileAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/io/FileAlg.scala
@@ -19,7 +19,7 @@ package org.scalasteward.core.io
 import better.files.File
 import cats.effect.{Resource, Sync}
 import cats.syntax.all._
-import cats.{ApplicativeError, Monad, MonadThrow}
+import cats.{ApplicativeError, MonadThrow}
 import fs2.Stream
 import org.apache.commons.io.FileUtils
 import org.http4s.Uri
@@ -67,22 +67,6 @@ trait FileAlg[F[_]] {
     readFile(file)
       .flatMap(_.fold(F.unit)(edit(_).flatMap(writeFile(file, _))))
       .adaptError { case t => new Throwable(s"failed to edit $file", t) }
-
-  final def findFiles[A, B](
-      dir: File,
-      fileFilter: File => Option[A],
-      contentFilter: String => Option[B]
-  )(implicit F: Monad[F]): Stream[F, (A, B)] = {
-    val none = Option.empty[(A, B)].pure[F]
-    walk(dir).evalMapFilter { file =>
-      isRegularFile(file).ifM(
-        fileFilter(file).fold(none) { a =>
-          readFile(file).map(_.flatMap(contentFilter).tupleLeft(a))
-        },
-        none
-      )
-    }
-  }
 }
 
 object FileAlg {

diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala
@@ -33,25 +33,11 @@ class BuildToolDispatcherTest extends FunSuite {
         Cmd("test", "-f", s"$repoDir/pom.xml"),
         Cmd("test", "-f", s"$repoDir/build.sc"),
         Cmd("test", "-f", s"$repoDir/build.sbt"),
-        Cmd.git(
-          repoDir,
-          "grep",
-          "-I",
-          "--fixed-strings",
-          "--files-with-matches",
-          "//> using lib "
-        ),
+        Cmd.gitGrep(repoDir, "//> using lib "),
         Cmd("test", "-f", s"$repoDir/mvn-build/pom.xml"),
         Cmd("test", "-f", s"$repoDir/mvn-build/build.sc"),
         Cmd("test", "-f", s"$repoDir/mvn-build/build.sbt"),
-        Cmd.git(
-          repoDir,
-          "grep",
-          "-I",
-          "--fixed-strings",
-          "--files-with-matches",
-          "//> using lib "
-        ),
+        Cmd.gitGrep(repoDir, "//> using lib "),
         Log("Get dependencies in . from sbt"),
         Cmd("read", s"$repoDir/project/build.properties"),
         Cmd("test", "-d", s"$repoDir/project"),

diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/scalacli/ScalaCliAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/scalacli/ScalaCliAlgTest.scala
@@ -16,8 +16,7 @@ class ScalaCliAlgTest extends CatsEffectSuite {
     val buildRoot = BuildRoot(repo, ".")
     val repoDir = workspaceAlg.repoDir(repo).unsafeRunSync()
     val fileWithUsingLib = "test.md" // this test fails if the extension is .scala or .sc
-    val grepCmd =
-      Cmd.git(repoDir, "grep", "-I", "--fixed-strings", "--files-with-matches", "//> using lib ")
+    val grepCmd = Cmd.gitGrep(repoDir, "//> using lib ")
     val initial =
       MockState.empty.copy(commandOutputs = Map(grepCmd -> Right(List(fileWithUsingLib))))
     val obtained = scalaCliAlg.containsBuild(buildRoot).runA(initial)

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
@@ -1,6 +1,5 @@
 package org.scalasteward.core.edit
 
-import better.files.File
 import cats.effect.unsafe.implicits.global
 import munit.FunSuite
 import org.scalasteward.core.TestInstances.dummyRepoCache
@@ -16,46 +15,39 @@ import org.scalasteward.core.scalafmt.ScalafmtAlg.opts
 import org.scalasteward.core.scalafmt.{scalafmtBinary, scalafmtConfName, scalafmtDependency}
 
 class EditAlgTest extends FunSuite {
-  private def gitStatus(repoDir: File): Cmd =
-    Cmd.git(repoDir, "status", "--porcelain", "--untracked-files=no", "--ignore-submodules")
-
   test("applyUpdate") {
     val repo = Repo("edit-alg", "test-1")
     val data = RepoData(repo, dummyRepoCache, RepoConfig.empty)
     val repoDir = workspaceAlg.repoDir(repo).unsafeRunSync()
     val update = ("org.typelevel".g % "cats-core".a % "1.2.0" %> "1.3.0").single
-    val file1 = repoDir / "build.sbt"
-    val file2 = repoDir / "project/Dependencies.scala"
+    val buildSbt = repoDir / "build.sbt"
+    val scalaFile = repoDir / "project/Dependencies.scala"
     val gitignore = repoDir / ".gitignore"
 
     val state = MockState.empty
-      .addFiles(file1 -> """val catsVersion = "1.2.0"""", file2 -> "", gitignore -> "")
+      .copy(execCommands = true)
+      .initGitRepo(
+        repoDir,
+        buildSbt -> """val catsVersion = "1.2.0"""",
+        scalaFile -> "",
+        gitignore -> ""
+      )
       .flatMap(editAlg.applyUpdate(data, update).runS)
       .unsafeRunSync()
 
     val expected = MockState.empty.copy(
+      execCommands = true,
       trace = Vector(
-        Cmd("read", gitignore.pathAsString),
-        Cmd("test", "-f", repoDir.pathAsString),
-        Cmd("test", "-f", gitignore.pathAsString),
-        Cmd("test", "-f", file1.pathAsString),
-        Cmd("read", file1.pathAsString),
-        Cmd("test", "-f", (repoDir / "project").pathAsString),
-        Cmd("test", "-f", file2.pathAsString),
-        Cmd("read", file2.pathAsString),
-        Cmd("read", gitignore.pathAsString),
-        Cmd("test", "-f", repoDir.pathAsString),
-        Cmd("test", "-f", gitignore.pathAsString),
-        Cmd("test", "-f", file1.pathAsString),
-        Cmd("read", file1.pathAsString),
-        Cmd("test", "-f", (repoDir / "project").pathAsString),
-        Cmd("test", "-f", file2.pathAsString),
-        Cmd("read", file2.pathAsString),
-        Cmd("read", file1.pathAsString),
-        Cmd("write", file1.pathAsString),
-        gitStatus(repoDir)
+        Cmd.gitGrep(repoDir, update.currentVersion.value),
+        Cmd("read", buildSbt.pathAsString),
+        Cmd.gitGrep(repoDir, update.groupId.value),
+        Cmd("read", buildSbt.pathAsString),
+        Cmd("write", buildSbt.pathAsString),
+        Cmd.gitStatus(repoDir),
+        Cmd.gitCommit(repoDir, "Update cats-core to 1.3.0"),
+        Cmd.gitLatestSha1(repoDir)
       ),
-      files = Map(file1 -> """val catsVersion = "1.3.0"""", file2 -> "", gitignore -> "")
+      files = Map(buildSbt -> """val catsVersion = "1.3.0"""", scalaFile -> "", gitignore -> "")
     )
 
     assertEquals(state, expected)
@@ -69,80 +61,67 @@ class EditAlgTest extends FunSuite {
     val data = RepoData(repo, cache, RepoConfig.empty)
     val repoDir = workspaceAlg.repoDir(repo).unsafeRunSync()
     val update = ("org.scalameta".g % "scalafmt-core".a % "2.0.0" %> "2.1.0").single
-    val gitignore = repoDir / ".gitignore"
+    val gitignore = (repoDir / ".gitignore") -> "target/"
     val scalafmtConf = repoDir / scalafmtConfName
     val scalafmtConfContent = """maxColumn = 100
                                 |version = 2.0.0
                                 |align.openParenCallSite = false
                                 |""".stripMargin
-    val buildSbt = repoDir / "build.sbt"
+    val buildSbt = (repoDir / "build.sbt") -> "\n"
     val target = repoDir / "target"
     // this file should not be read because it's under target which is git ignored
-    val targetScalaFile = target / "SomeFile.scala"
+    val targetScalaFile = (target / "SomeFile.scala") -> s"""object Test {"2.0.0"}"""
 
     val state = MockState.empty
-      .addFiles(
-        scalafmtConf -> scalafmtConfContent,
-        buildSbt -> "",
-        gitignore -> "target/",
-        targetScalaFile -> ""
-      )
+      .copy(execCommands = true)
+      .initGitRepo(repoDir, scalafmtConf -> scalafmtConfContent, buildSbt, gitignore)
+      .flatMap(_.addFiles(targetScalaFile))
       .flatMap(editAlg.applyUpdate(data, update).runS)
       .unsafeRunSync()
 
     val expected = MockState.empty.copy(
+      execCommands = true,
       trace = Vector(
-        Cmd("read", gitignore.pathAsString),
-        Cmd("test", "-f", repoDir.pathAsString),
-        Cmd("test", "-f", gitignore.pathAsString),
-        Cmd("test", "-f", scalafmtConf.pathAsString),
+        Cmd.gitGrep(repoDir, update.currentVersion.value),
         Cmd("read", scalafmtConf.pathAsString),
-        Cmd("test", "-f", buildSbt.pathAsString),
-        Cmd("read", buildSbt.pathAsString),
-        Cmd("test", "-f", target.pathAsString),
-        Cmd("test", "-f", targetScalaFile.pathAsString),
-        Cmd("read", gitignore.pathAsString),
-        Cmd("test", "-f", repoDir.pathAsString),
-        Cmd("test", "-f", gitignore.pathAsString),
-        Cmd("test", "-f", scalafmtConf.pathAsString),
-        Cmd("read", scalafmtConf.pathAsString),
-        Cmd("test", "-f", buildSbt.pathAsString),
-        Cmd("read", buildSbt.pathAsString),
-        Cmd("test", "-f", target.pathAsString),
-        Cmd("test", "-f", targetScalaFile.pathAsString),
+        Cmd.gitGrep(repoDir, update.groupId.value),
         Cmd("read", scalafmtConf.pathAsString),
         Cmd("write", scalafmtConf.pathAsString),
         Cmd.exec(repoDir, scalafmtBinary :: opts.nonInteractive :: opts.modeChanged: _*),
-        gitStatus(repoDir),
+        Cmd.gitStatus(repoDir),
+        Cmd.gitCommit(repoDir, "Update scalafmt-core to 2.1.0"),
+        Cmd.gitLatestSha1(repoDir),
         Log(
           "Executing post-update hook for org.scalameta:scalafmt-core with command 'scalafmt --non-interactive'"
         ),
         Cmd.exec(repoDir, scalafmtBinary, opts.nonInteractive),
-        gitStatus(repoDir)
+        Cmd.gitStatus(repoDir)
       ),
       files = Map(
         scalafmtConf ->
           """maxColumn = 100
             |version = 2.1.0
             |align.openParenCallSite = false
             |""".stripMargin,
-        buildSbt -> "",
-        gitignore -> "target/",
-        targetScalaFile -> ""
+        buildSbt,
+        gitignore,
+        targetScalaFile
       )
     )
 
     assertEquals(state, expected)
   }
 
   test("applyUpdate with build Scalafix") {
-    val repo = Repo("edit-alg", "test-3-1")
+    val repo = Repo("edit-alg", "test-3")
     val data = RepoData(repo, dummyRepoCache, RepoConfig.empty)
     val repoDir = workspaceAlg.repoDir(repo).unsafeRunSync()
     val update = (sbtGroupId % sbtArtifactId % "1.4.9" %> "1.5.5").single
 
     val state = MockState.empty
-      .addFiles(
+      .copy(execCommands = true)
+      .initGitRepo(
+        repoDir,
         repoDir / "build.sbt" -> "",
         repoDir / "project" / "build.properties" -> """sbt.version=1.4.9"""
       )

diff --git a/modules/core/src/test/scala/org/scalasteward/core/edit/RewriteTest.scala b/modules/core/src/test/scala/org/scalasteward/core/edit/RewriteTest.scala
@@ -957,7 +957,8 @@ class RewriteTest extends FunSuite {
     val repoDir = workspaceAlg.repoDir(repo).unsafeRunSync()
     val filesInRepoDir = files.map { case (file, content) => repoDir / file -> content }
     val state = MockState.empty
-      .addFiles(filesInRepoDir.toSeq: _*)
+      .copy(execCommands = true)
+      .initGitRepo(repoDir, filesInRepoDir.toSeq: _*)
       .flatMap(editAlg.applyUpdate(data, update).runS)
       .unsafeRunSync()
     val obtained = state.files