Refactor: Separate modelling of candidate newerVersions from Updates

_Addresses https://github.com/scala-steward-org/scala-steward/issues/3781_

The Scala Steward codebase deals with the concept of 'updates' in two areas:

* **Multiple `newerVersions`**: Initially, evaluating _candidate_ updates (in `UpdateAlg` & `FilterAlg`), where for any given artifact, there can be many possible versions we might want to upgrade to. The list of versions is reduced down with filtering (based on config & other rules), and eventually a single next version is selected.
* **Single `nextVersion`**: Subsequently, retrieving information on the updates represented by the pull requests Scala Steward created - each artifact being updated to a specific _single_ new version.

...Scala Steward used the `Update` type for _both_ areas (specifically, `Update.ForArtifactId` for the first phase, _requiring_ it to support multiple `newerVersions`).

Looking at a single `Update.ForArtifactId` in code, it's not always clear to the newcomer if the update contains many newer versions or not - ie where we are in the pipeline.

## Changes

* `Update.ForArtifactId` → `ArtifactUpdateCandidates`🆕  within `UpdateAlg` & `FilterAlg` where candidate versions are being evaluated
* `Update.ForArtifactId` & `ArtifactUpdateCandidates` both extend `ArtifactUpdateVersions`🆕 to allow `UpdatePattern.findMatch()` to filter lists of either of them:
  * Filtering the candidate versions of `ArtifactUpdateCandidates` in `UpdatesConfig.isAllowed`, etc
  * Filtering the `Update`s of existing PRs in `PruningAlg.newPullRequestsAllowed()` & `RetractedArtifact.isRetracted()`
* The fields from `Update.ForArtifactId` also required by `ArtifactUpdateCandidates` have been placed in the `ArtifactForUpdate`🆕 case class
* Restrict `Update.Single` types to just `nextVersion`, not `newerVersions`

Remaining unchanged:

* `PullRequestRepository` still persists instances of `Update`
* The JSON serialisation format of `Update` remains unchanged for now (the encoder & decoders have been adapted so that they still output the same format, despite the case classes having changed), so there's currently no need to introduce a new fallback 'legacy' decoder (several of which were recently deleted with #3785). This also means the JSON metadata added to PR descriptions (originally introduced in #3466) also remains the same for now, though I think it will be reasonable to adjust the format for `Update.ForArtifactId` to something more logical in future.

Author: rtyley

modules/benchmark/src/main/scala/org/scalasteward/benchmark/UpdatesConfigBenchmark.scala

@@ -33,7 +33,7 @@ class UpdatesConfigBenchmark {
     val newerVersions = Nel
       .of("2.0.0", "2.1.0", "2.1.1", "2.2.0", "3.0.0", "3.1.0", "3.2.1", "3.3.3", "4.0", "5.0")
       .map(Version.apply)
-    val update = Update.ForArtifactId(dependency, newerVersions)
+    val update = ArtifactUpdateCandidates(ArtifactForUpdate(dependency), newerVersions)
 
     UpdatesConfig().keep(update)
     UpdatesConfig(allow = Some(List(UpdatePattern(groupId, None, None)))).keep(update)

modules/core/src/main/scala/org/scalasteward/core/data/Update.scala

@@ -23,6 +23,43 @@ import org.scalasteward.core.repoconfig.PullRequestGroup
 import org.scalasteward.core.util
 import org.scalasteward.core.util.Nel
 
+case class ArtifactForUpdate(
+    crossDependency: CrossDependency,
+    newerGroupId: Option[GroupId] = None,
+    newerArtifactId: Option[String] = None
+) {
+  private val headDependency: Dependency = crossDependency.head
+  def groupId: GroupId = headDependency.groupId
+  def artifactId: ArtifactId = headDependency.artifactId
+  def currentVersion: Version = headDependency.version
+}
+
+trait ArtifactUpdateVersions {
+  val artifactForUpdate: ArtifactForUpdate
+
+  val refersToUpdateVersions: Nel[Version]
+
+  def show: String
+}
+
+/** Captures possible ''candidate'' newer versions that we may update an artifact to.
+  *
+  * Compare with the other subclass of [[ArtifactUpdateVersions]], [[Update.ForArtifactId]], which
+  * denotes the specific ''single'' next version ultimately used in a PR.
+  */
+case class ArtifactUpdateCandidates(
+    artifactForUpdate: ArtifactForUpdate,
+    newerVersions: Nel[Version]
+) extends ArtifactUpdateVersions {
+  override val refersToUpdateVersions: Nel[Version] = newerVersions
+
+  def asSpecificUpdate(nextVersion: Version): Update.ForArtifactId =
+    Update.ForArtifactId(artifactForUpdate, nextVersion)
+
+  override def show: String =
+    s"${artifactForUpdate.groupId}:${artifactForUpdate.crossDependency.showArtifactNames} : ${Version.show((artifactForUpdate.currentVersion +: refersToUpdateVersions.toList)*)}"
+}
+
 sealed trait Update {
 
   def on[A](update: Update.Single => A, grouped: Update.Grouped => A): A = this match {
@@ -37,6 +74,11 @@ sealed trait Update {
 
 object Update {
 
+  /** Denotes the update of one or more artifacts, which have all matched the same
+    * `pullRequests.grouping` config rule.
+    *
+    * An `Update.Grouped` PR looks like this: [[https://github.com/guardian/etag-caching/pull/62]]
+    */
   final case class Grouped(
       name: String,
       title: Option[String],
@@ -49,47 +91,54 @@ object Update {
 
   sealed trait Single extends Product with Serializable with Update {
     override val asSingleUpdates: List[Update.Single] = List(this)
+    def artifactsForUpdate: Nel[ArtifactForUpdate]
     def forArtifactIds: Nel[ForArtifactId]
     def crossDependencies: Nel[CrossDependency]
     def dependencies: Nel[Dependency]
     def groupId: GroupId
     def artifactIds: Nel[ArtifactId]
     def mainArtifactId: String
+    def showArtifacts: String
     def groupAndMainArtifactId: (GroupId, String) = (groupId, mainArtifactId)
     def currentVersion: Version
-    def newerVersions: Nel[Version]
+    def nextVersion: Version
 
     final def name: String = Update.nameOf(groupId, mainArtifactId)
 
-    final def nextVersion: Version = newerVersions.head
-
-    final override def show: String = {
-      val artifacts = this match {
-        case s: ForArtifactId => s.crossDependency.showArtifactNames
-        case g: ForGroupId => g.crossDependencies.map(_.showArtifactNames).mkString_("{", ", ", "}")
-      }
-      val versions = {
-        val vs0 = (currentVersion :: newerVersions).toList
-        val vs1 = if (vs0.size > 6) vs0.take(3) ++ ("..." :: vs0.takeRight(3)) else vs0
-        vs1.mkString("", " -> ", "")
-      }
-      s"$groupId:$artifacts : $versions"
-    }
+    final override def show: String =
+      s"$groupId:$showArtifacts : ${Version.show(currentVersion, nextVersion)}"
 
-    def withNewerVersions(versions: Nel[Version]): Update.Single = this match {
-      case s @ ForArtifactId(_, _, _, _) =>
-        s.copy(newerVersions = versions)
-      case ForGroupId(forArtifactIds) =>
-        ForGroupId(forArtifactIds.map(_.copy(newerVersions = versions)))
+    def withNextVersion(nextVersion: Version): Update.Single = this match {
+      case s: ForArtifactId =>
+        s.copy(nextVersion = nextVersion)
+      case g: ForGroupId =>
+        g.copy(nextVersion = nextVersion)
     }
   }
 
+  /** Denotes the update of a specific single artifact to some particular chosen next version.
+    *
+    * An update PR with a single `Update.ForArtifactId` looks like this:
+    * [[https://github.com/guardian/etag-caching/pull/125]]
+    *
+    * In the other subclass of [[ArtifactUpdateVersions]], [[ArtifactUpdateCandidates]], _multiple_
+    * possible candidate newer versions are stored.
+    */
   final case class ForArtifactId(
-      crossDependency: CrossDependency,
-      newerVersions: Nel[Version],
-      newerGroupId: Option[GroupId] = None,
-      newerArtifactId: Option[String] = None
-  ) extends Single {
+      artifactForUpdate: ArtifactForUpdate,
+      nextVersion: Version
+  ) extends Single
+      with ArtifactUpdateVersions {
+    val crossDependency: CrossDependency = artifactForUpdate.crossDependency
+
+    override val refersToUpdateVersions: Nel[Version] = Nel.one(nextVersion)
+
+    override def artifactsForUpdate: Nel[ArtifactForUpdate] = Nel.one(artifactForUpdate)
+
+    override def showArtifacts: String = crossDependency.showArtifactNames
+
+    lazy val versionUpdate: Version.Update = Version.Update(currentVersion, nextVersion)
+
     override def forArtifactIds: Nel[ForArtifactId] =
       Nel.one(this)
 
@@ -100,7 +149,7 @@ object Update {
       crossDependency.dependencies
 
     override def groupId: GroupId =
-      crossDependency.head.groupId
+      artifactForUpdate.groupId
 
     override def artifactIds: Nel[ArtifactId] =
       dependencies.map(_.artifactId)
@@ -109,17 +158,28 @@ object Update {
       artifactId.name
 
     override def currentVersion: Version =
-      crossDependency.head.version
+      artifactForUpdate.currentVersion
 
     def artifactId: ArtifactId =
-      crossDependency.head.artifactId
+      artifactForUpdate.artifactId
   }
 
+  /** Denotes the update of several artifacts which all have the same Maven group-id, and also are
+    * all are being updated ''from'' the same version, and updated ''to'' the same `nextVersion`.
+    *
+    * An `Update.ForGroupId` PR looks like this:
+    * [[https://github.com/guardian/etag-caching/pull/128]]
+    */
   final case class ForGroupId(
-      forArtifactIds: Nel[ForArtifactId]
+      artifactsForUpdate: Nel[ArtifactForUpdate],
+      nextVersion: Version
   ) extends Single {
+
+    override def forArtifactIds: Nel[ForArtifactId] =
+      artifactsForUpdate.map(ForArtifactId(_, nextVersion))
+
     override def crossDependencies: Nel[CrossDependency] =
-      forArtifactIds.map(_.crossDependency)
+      artifactsForUpdate.map(_.crossDependency)
 
     override def dependencies: Nel[Dependency] =
       crossDependencies.flatMap(_.dependencies)
@@ -142,12 +202,12 @@ object Update {
         .getOrElse(artifactIds.head.name)
     }
 
+    override def showArtifacts: String =
+      crossDependencies.map(_.showArtifactNames).mkString_("{", ", ", "}")
+
     override def currentVersion: Version =
       dependencies.head.version
 
-    override def newerVersions: Nel[Version] =
-      forArtifactIds.head.newerVersions
-
     def artifactIdsPrefix: Option[String] =
       util.string.longestCommonPrefixGteq(artifactIds.map(_.name), 3)
   }
@@ -163,19 +223,23 @@ object Update {
 
   def groupByArtifactIdName(updates: List[ForArtifactId]): List[ForArtifactId] = {
     val groups0 =
-      updates.groupByNel(s => (s.groupId, s.artifactId.name, s.currentVersion, s.newerVersions))
+      updates.groupByNel(s => (s.groupId, s.artifactId.name, s.currentVersion, s.nextVersion))
     val groups1 = groups0.values.map { group =>
-      val dependencies = group.flatMap(_.crossDependency.dependencies).distinct.sorted
-      group.head.copy(crossDependency = CrossDependency(dependencies))
+      val dependencies = group.flatMap(_.dependencies).distinct.sorted
+      val update: Update.ForArtifactId = group.head
+      update.copy(artifactForUpdate =
+        update.artifactForUpdate.copy(crossDependency = CrossDependency(dependencies))
+      )
     }
     groups1.toList.distinct.sortBy(u => u: Update.Single)
   }
 
   def groupByGroupId(updates: List[ForArtifactId]): List[Single] = {
     val groups0 =
-      updates.groupByNel(s => (s.groupId, s.currentVersion, s.newerVersions))
-    val groups1 = groups0.values.map { group =>
-      if (group.tail.isEmpty) group.head else ForGroupId(group)
+      updates.groupByNel(s => (s.groupId, s.versionUpdate))
+    val groups1 = groups0.map { case ((_, versionUpdate), group) =>
+      if (group.tail.isEmpty) group.head
+      else ForGroupId(group.map(_.artifactForUpdate), versionUpdate.nextVersion)
     }
     groups1.toList.distinct.sorted
   }
@@ -197,7 +261,7 @@ object Update {
     }
 
   implicit val SingleOrder: Order[Single] =
-    Order.by((u: Single) => (u.crossDependencies, u.newerVersions))
+    Order.by((u: Single) => (u.crossDependencies, u.nextVersion))
 
   // Encoder and Decoder instances
 
@@ -206,19 +270,28 @@ object Update {
   implicit private val forArtifactIdEncoder: Encoder[ForArtifactId] =
     Encoder.forProduct1("ForArtifactId")(identity[ForArtifactId]) {
       Encoder.forProduct4("crossDependency", "newerVersions", "newerGroupId", "newerArtifactId") {
-        s => (s.crossDependency, s.newerVersions, s.newerGroupId, s.newerArtifactId)
+        s =>
+          (
+            s.crossDependency,
+            Seq(s.nextVersion),
+            s.artifactForUpdate.newerGroupId,
+            s.artifactForUpdate.newerArtifactId
+          )
       }
     }
 
   private val unwrappedForArtifactIdDecoder: Decoder[ForArtifactId] =
     Decoder.forProduct4("crossDependency", "newerVersions", "newerGroupId", "newerArtifactId") {
       (
           crossDependency: CrossDependency,
-          newerVersions: Nel[Version],
+          newerVersions: List[Version],
           newerGroupId: Option[GroupId],
           newerArtifactId: Option[String]
       ) =>
-        ForArtifactId(crossDependency, newerVersions, newerGroupId, newerArtifactId)
+        ForArtifactId(
+          ArtifactForUpdate(crossDependency, newerGroupId, newerArtifactId),
+          newerVersions.head
+        )
     }
 
   private val forArtifactIdDecoderV2 =
@@ -236,7 +309,7 @@ object Update {
 
   private val unwrappedForGroupIdDecoderV3: Decoder[ForGroupId] =
     Decoder.forProduct1("forArtifactIds") { (forArtifactIds: Nel[ForArtifactId]) =>
-      ForGroupId(forArtifactIds)
+      ForGroupId(forArtifactIds.map(_.artifactForUpdate), forArtifactIds.head.nextVersion)
     }
 
   private val forGroupIdDecoderV3: Decoder[ForGroupId] =

modules/core/src/main/scala/org/scalasteward/core/data/Version.scala

@@ -124,6 +124,12 @@ final case class Version(value: String) {
 object Version {
   case class Update(currentVersion: Version, nextVersion: Version)
 
+  def show(versions: Version*): String = {
+    val vs0 = versions.map(_.value)
+    val vs1: Seq[String] = if (vs0.size > 6) (vs0.take(3) :+ "...") ++ vs0.takeRight(3) else vs0
+    vs1.mkString(" -> ")
+  }
+
   val tagNames: List[Version => String] = List("v" + _, _.value, "release-" + _)
 
   implicit val versionDecoder: Decoder[Version] =
@@ -138,6 +144,9 @@ object Version {
       c1.compare(c2)
     }
 
+  implicit val versionUpdateOrder: Order[Version.Update] =
+    Order.by((vu: Version.Update) => (vu.currentVersion, vu.nextVersion))
+
   private def padToSameLength[A](l1: List[A], l2: List[A], elem: A): (List[A], List[A]) = {
     val maxLength = math.max(l1.length, l2.length)
     (l1.padTo(maxLength, elem), l2.padTo(maxLength, elem))

modules/core/src/main/scala/org/scalasteward/core/edit/update/Selector.scala

@@ -204,21 +204,19 @@ object Selector {
       update: Update.Single,
       modulePositions: List[ModulePosition]
   ): List[Substring.Replacement] =
-    update.forArtifactIds.toList.flatMap { forArtifactId =>
-      val newerGroupId = forArtifactId.newerGroupId
-      val newerArtifactId = forArtifactId.newerArtifactId
+    update.artifactsForUpdate.toList.flatMap { artifactForUpdate =>
+      val newerGroupId = artifactForUpdate.newerGroupId
+      val newerArtifactId = artifactForUpdate.newerArtifactId
       if (newerGroupId.isEmpty && newerArtifactId.isEmpty) List.empty
       else {
-        val currentGroupId = forArtifactId.groupId
-        val currentArtifactId = forArtifactId.artifactIds.head
         modulePositions
           .filter { p =>
-            p.groupId.value === currentGroupId.value &&
-            currentArtifactId.names.contains_(p.artifactId.value)
+            p.groupId.value === artifactForUpdate.groupId.value &&
+            artifactForUpdate.artifactId.names.contains_(p.artifactId.value)
           }
           .flatMap { p =>
-            newerGroupId.map(g => p.groupId.replaceWith(g.value)).toList ++
-              newerArtifactId.map(a => p.artifactId.replaceWith(a)).toList
+            newerGroupId.map(g => p.groupId.replaceWith(g.value)) ++
+              newerArtifactId.map(a => p.artifactId.replaceWith(a))
           }
       }
     }

modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala

@@ -363,7 +363,7 @@ object NewPullRequestData {
 
     val artifactMigrationsLabel = Option.when {
       update.asSingleUpdates
-        .flatMap(_.forArtifactIds.toList)
+        .flatMap(_.artifactsForUpdate.toList)
         .exists(u => u.newerGroupId.nonEmpty || u.newerArtifactId.nonEmpty)
     }("artifact-migrations")
     val scalafixLabel = edits.collectFirst { case _: ScalafixEdit => "scalafix-migrations" }

modules/core/src/main/scala/org/scalasteward/core/nurture/PullRequestRepository.scala

@@ -72,7 +72,7 @@ final class PullRequestRepository[F[_]](kvStore: KeyValueStore[F, Repo, Map[Uri,
       _.collect {
         case (url, Entry(baseSha1, u: Update.Single, state, _, number, updateBranch))
             if state === PullRequestState.Open &&
-              u.withNewerVersions(update.newerVersions) === update &&
+              u.withNextVersion(update.nextVersion) === update &&
               u.nextVersion < update.nextVersion =>
           for {
             number <- number

modules/core/src/main/scala/org/scalasteward/core/repoconfig/UpdatePattern.scala

@@ -19,7 +19,7 @@ package org.scalasteward.core.repoconfig
 import cats.syntax.all.*
 import io.circe.Codec
 import io.circe.generic.semiauto.*
-import org.scalasteward.core.data.{GroupId, Update, Version}
+import org.scalasteward.core.data.{ArtifactUpdateVersions, GroupId, Version}
 
 final case class UpdatePattern(
     groupId: GroupId,
@@ -37,12 +37,14 @@ object UpdatePattern {
 
   def findMatch(
       patterns: List[UpdatePattern],
-      update: Update.ForArtifactId,
+      update: ArtifactUpdateVersions,
       include: Boolean
   ): MatchResult = {
-    val byGroupId = patterns.filter(_.groupId === update.groupId)
-    val byArtifactId = byGroupId.filter(_.artifactId.forall(_ === update.artifactId.name))
-    val filteredVersions = update.newerVersions.filter(newVersion =>
+    val artifactForUpdate = update.artifactForUpdate
+    val byGroupId = patterns.filter(_.groupId === artifactForUpdate.groupId)
+    val byArtifactId =
+      byGroupId.filter(_.artifactId.forall(_ === artifactForUpdate.artifactId.name))
+    val filteredVersions = update.refersToUpdateVersions.filter(newVersion =>
       byArtifactId.exists(_.version.forall(_.matches(newVersion.value))) === include
     )
     MatchResult(byArtifactId, filteredVersions)