Repository: scala-steward-org/scala-steward
Pull request #2470 (2022-01-22 18:58:12)

Refactor UpdateAlg
Some refactorings in `UpdateAlg` to reduce the line count.


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/update/UpdateAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/update/UpdateAlg.scala
index 91c4d92a..26c7f7de 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/update/UpdateAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/update/UpdateAlg.scala
@@ -17,14 +17,14 @@
 package org.scalasteward.core.update
 
 import cats.Monad
+import cats.data.OptionT
 import cats.syntax.all._
 import org.scalasteward.core.coursier.VersionsCache
 import org.scalasteward.core.data._
 import org.scalasteward.core.repoconfig.RepoConfig
-import org.scalasteward.core.update.UpdateAlg.migratedDependency
+import org.scalasteward.core.update.UpdateAlg.migrateDependency
 import org.scalasteward.core.update.artifact.{ArtifactChange, ArtifactMigrationsFinder}
 import org.scalasteward.core.util.Nel
-
 import scala.concurrent.duration.FiniteDuration
 
 final class UpdateAlg[F[_]](implicit
@@ -37,38 +37,9 @@ final class UpdateAlg[F[_]](implicit
       dependency: Scope[Dependency],
       maxAge: Option[FiniteDuration]
   ): F[Option[Update.Single]] =
-    for {
-      existingVersions <- versionsCache.getVersions(dependency, maxAge)
-      maybeUpdate = maybeNewerVersions(dependency, existingVersions)
-        .map(vs => Update.Single(CrossDependency(dependency.value), vs.map(_.value)))
-      maybeUpdateOrRename <- maybeUpdate match {
-        case Some(update) => F.pure(Some(update))
-        case None =>
-          artifactMigrationsFinder.findUpdateWithRenamedArtifact(dependency.value) match {
-            case Some(artifactChange) => verifyVersion(dependency, artifactChange, maxAge)
-            case None                 => F.pure(None)
-          }
-      }
-    } yield maybeUpdateOrRename
-
-  private def verifyVersion(
-      dependency: Scope[Dependency],
-      artifactChange: ArtifactChange,
-      maxAge: Option[FiniteDuration]
-  ): F[Option[Update.Single]] =
-    versionsCache
-      .getVersions(migratedDependency(dependency, artifactChange), maxAge)
-      .map { existingVersions =>
-        maybeNewerVersions(dependency, existingVersions)
-          .map(newerVersions =>
-            Update.Single(
-              CrossDependency(dependency.value),
-              newerVersions.map(_.value),
-              Some(artifactChange.groupIdAfter),
-              Some(artifactChange.artifactIdAfter)
-            )
-          )
-      }
+    findUpdateWithoutMigration(dependency, maxAge)
+      .orElse(findUpdateWithMigration(dependency, maxAge))
+      .value
 
   def findUpdates(
       dependencies: List[Scope.Dependency],
@@ -79,13 +50,39 @@ final class UpdateAlg[F[_]](implicit
     updates.flatMap(filterAlg.localFilterMany(repoConfig, _))
   }
 
-  private def maybeNewerVersions(
+  private def findUpdateWithoutMigration(
       dependency: Scope[Dependency],
-      existingVersions: List[Version]
-  ): Option[Nel[Version]] = {
-    val current = Version(dependency.value.version)
-    Nel.fromList(existingVersions.filter(_ > current))
-  }
+      maxAge: Option[FiniteDuration]
+  ): OptionT[F, Update.Single] =
+    findNewerVersions(dependency, maxAge).map { newerVersions =>
+      Update.Single(CrossDependency(dependency.value), newerVersions.map(_.value))
+    }
+
+  private def findUpdateWithMigration(
+      dependency: Scope[Dependency],
+      maxAge: Option[FiniteDuration]
+  ): OptionT[F, Update.Single] =
+    OptionT.fromOption(artifactMigrationsFinder.findArtifactChange(dependency.value)).flatMap {
+      artifactChange =>
+        findNewerVersions(dependency.map(migrateDependency(_, artifactChange)), maxAge).map {
+          newerVersions =>
+            Update.Single(
+              CrossDependency(dependency.value),
+              newerVersions.map(_.value),
+              Some(artifactChange.groupIdAfter),
+              Some(artifactChange.artifactIdAfter)
+            )
+        }
+    }
+
+  private def findNewerVersions(
+      dependency: Scope[Dependency],
+      maxAge: Option[FiniteDuration]
+  ): OptionT[F, Nel[Version]] =
+    OptionT(versionsCache.getVersions(dependency, maxAge).map { versions =>
+      val current = Version(dependency.value.version)
+      Nel.fromList(versions.filter(_ > current))
+    })
 }
 
 object UpdateAlg {
@@ -96,23 +93,12 @@ object UpdateAlg {
       update.artifactIds.contains_(dependency.artifactId)
     }
 
-  def migratedDependency(
-      dependency: Scope[Dependency],
-      artifactChange: ArtifactChange
-  ): Scope[Dependency] = {
-    val oldArtifactId = dependency.value.artifactId
-    val newGroupId = artifactChange.groupIdAfter
-    val newArtifactId = ArtifactId(
-      artifactChange.artifactIdAfter,
-      oldArtifactId.maybeCrossName.map(
-        _.replace(oldArtifactId.name, artifactChange.artifactIdAfter)
-      )
-    )
-    dependency.copy(value =
-      dependency.value.copy(
-        groupId = newGroupId,
-        artifactId = newArtifactId
-      )
+  def migrateArtifactId(artifactId: ArtifactId, newName: String): ArtifactId =
+    ArtifactId(newName, artifactId.maybeCrossName.map(_.replace(artifactId.name, newName)))
+
+  def migrateDependency(dependency: Dependency, change: ArtifactChange): Dependency =
+    dependency.copy(
+      groupId = change.groupIdAfter,
+      artifactId = migrateArtifactId(dependency.artifactId, change.artifactIdAfter)
     )
-  }
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinder.scala b/modules/core/src/main/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinder.scala
index 15e0266c..4290f9ae 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinder.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinder.scala
@@ -20,7 +20,7 @@ import cats.syntax.all._
 import org.scalasteward.core.data.Dependency
 
 final class ArtifactMigrationsFinder(migrations: List[ArtifactChange]) {
-  def findUpdateWithRenamedArtifact(dependency: Dependency): Option[ArtifactChange] =
+  def findArtifactChange(dependency: Dependency): Option[ArtifactChange] =
     migrations
       .find { migration =>
         (migration.groupIdBefore, migration.artifactIdBefore) match {


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinderTest.scala b/modules/core/src/test/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinderTest.scala
index 4b7bb075..fa92745d 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinderTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/update/artifact/ArtifactMigrationsFinderTest.scala
@@ -44,38 +44,34 @@ class ArtifactMigrationsFinderTest extends FunSuite {
     )
   )
 
-  test(
-    "findUpdateWithRenamedArtifact: for groupId, returns empty if artifactIdAfter is not listed"
-  ) {
+  test("findArtifactChange: for groupId, returns empty if artifactIdAfter is not listed") {
     val original = "org.spire-math".g % ("UNKNOWN", "UNKNOWN_2.12").a % "1.0.0"
-    val obtained = standardGroupMigrations.findUpdateWithRenamedArtifact(original)
+    val obtained = standardGroupMigrations.findArtifactChange(original)
     assertEquals(obtained, None)
   }
 
-  test(
-    "findUpdateWithRenamedArtifact: for artifactId, returns empty if groupIdAfter is not listed"
-  ) {
+  test("findArtifactChange: for artifactId, returns empty if groupIdAfter is not listed") {
     val original = "UNKNOWN".g % ("artifact-before", "artifact-before_2.12").a % "1.0.0"
-    val obtained = standardArtifactMigrations.findUpdateWithRenamedArtifact(original)
+    val obtained = standardArtifactMigrations.findArtifactChange(original)
     assertEquals(obtained, None)
   }
 
   test(
-    "findUpdateWithRenamedArtifact: for both groupId and artifactId, returns empty if either of groupIdBefore or " +
+    "findArtifactChange: for both groupId and artifactId, returns empty if either of groupIdBefore or " +
       "artifactIdBefore are not listed"
   ) {
     val original1 = "UNKNOWN".g % ("artifact-before", "artifact-before_2.12").a % "1.0.0"
-    val obtained1 = standardBothMigrations.findUpdateWithRenamedArtifact(original1)
+    val obtained1 = standardBothMigrations.findArtifactChange(original1)
     assertEquals(obtained1, None)
 
     val original2 = "org.before".g % ("UNKNOWN", "UNKNOWN_2.12").a % "1.0.0"
-    val obtained2 = standardBothMigrations.findUpdateWithRenamedArtifact(original2)
+    val obtained2 = standardBothMigrations.findArtifactChange(original2)
     assertEquals(obtained2, None)
   }
 
-  test("findUpdateWithRenamedArtifact: for groupId, returns Update.Single for updating groupId") {
+  test("findArtifactChange: for groupId, returns Update.Single for updating groupId") {
     val original = "org.spire-math".g % ("kind-projector", "kind-projector_2.12").a % "0.9.0"
-    val obtained = standardGroupMigrations.findUpdateWithRenamedArtifact(original)
+    val obtained = standardGroupMigrations.findArtifactChange(original)
     val expected = ArtifactChange(
       Some("org.spire-math".g),
       "org.typelevel".g,
@@ -85,11 +81,9 @@ class ArtifactMigrationsFinderTest extends FunSuite {
     assertEquals(obtained, Some(expected))
   }
 
-  test(
-    "findUpdateWithRenamedArtifact: for artifactId, returns Update.Single for updating artifactId"
-  ) {
+  test("findArtifactChange: for artifactId, returns Update.Single for updating artifactId") {
     val original = "com.nodifferent".g % ("artifact-before", "artifact-before_2.12").a % "0.9.0"
-    val obtained = standardArtifactMigrations.findUpdateWithRenamedArtifact(original)
+    val obtained = standardArtifactMigrations.findArtifactChange(original)
     val expected = ArtifactChange(
       None,
       "com.nodifferent".g,
@@ -100,11 +94,11 @@ class ArtifactMigrationsFinderTest extends FunSuite {
   }
 
   test(
-    "findUpdateWithRenamedArtifact: for both groupId and artifactId, returns Update.Single for updating both groupId " +
+    "findArtifactChange: for both groupId and artifactId, returns Update.Single for updating both groupId " +
       "and artifactId"
   ) {
     val original = "org.before".g % ("artifact-before", "artifact-before_2.12").a % "0.9.0"
-    val obtained = standardBothMigrations.findUpdateWithRenamedArtifact(original)
+    val obtained = standardBothMigrations.findArtifactChange(original)
     val expected = ArtifactChange(
       Some("org.before".g),
       "org.after".g,
@@ -126,7 +120,7 @@ class ArtifactMigrationsFinderTest extends FunSuite {
     assertEquals(obtained, Some(expected))
   }
 
-  test("migratedDependency: newer groupId") {
+  test("migrateDependency: newer groupId") {
     val dependency = "org.spire-math".g % ("kind-projector", "kind-projector_2.12").a % "0.9.10"
     val artifactChange = ArtifactChange(
       Some("org.spire-math".g),
@@ -135,11 +129,11 @@ class ArtifactMigrationsFinderTest extends FunSuite {
       "kind-projector"
     )
     val expected = "org.typelevel".g % ("kind-projector", "kind-projector_2.12").a % "0.9.10"
-    val obtained = UpdateAlg.migratedDependency(dependency.withMavenCentral, artifactChange)
-    assertEquals(obtained, expected.withMavenCentral)
+    val obtained = UpdateAlg.migrateDependency(dependency, artifactChange)
+    assertEquals(obtained, expected)
   }
 
-  test("migratedDependency: newer ArtifactId") {
+  test("migrateDependency: newer ArtifactId") {
     val dependency = "org.spire-math".g % ("kind-projector", "kind-projector_2.12").a % "0.9.10"
     val artifactChange = ArtifactChange(
       Some("org.spire-math".g),
@@ -148,7 +142,7 @@ class ArtifactMigrationsFinderTest extends FunSuite {
       "new-projector"
     )
     val expected = "org.spire-math".g % ("new-projector", "new-projector_2.12").a % "0.9.10"
-    val obtained = UpdateAlg.migratedDependency(dependency.withMavenCentral, artifactChange)
-    assertEquals(obtained, expected.withMavenCentral)
+    val obtained = UpdateAlg.migrateDependency(dependency, artifactChange)
+    assertEquals(obtained, expected)
   }
 }