Skip to content

Commit d8e6ddf

Browse files
committed
Refactor check of Coursier ordering onto Version.Update
This change just refactors the logic of the check to live on `Version.Update` (introduced with 13380eb in PR #3145 in August 2023), which feels like a good place for it to belong. The check for whether our version-updates comply with Coursier version-ordering was added with this PR in January 2020: * #1210 ...the check was supposed to be temporary, until Scala Steward's own `Order[Version]` was considered battle-tested, but I don't know when we'll decide that's happened! ## `current == next` is ok? Since commit 75c08e6, committed directly to `main` a day later, `VersionOrderingConflict` has only been raised if `current > next` - so `current == next` is fine. There's no further info on _why_ in the commit tho'! 75c08e6 ## New `coursier-versions` library Our Scala Steward check uses `coursier.core.Version`, which is deprecated since Coursier 2.1.25: https://github.com/coursier/coursier/blame/0f3ceb65e9e46826967d2e4cb6f87cf7fb3e5c9d/modules/core/shared/src/main/scala/coursier/core/Version.scala#L14 ...the recommended class is now `coursier.version.Version`, which is in the newer https://github.com/coursier/versions library created in May 2020: https://github.com/coursier/versions/blob/main/versions/shared/src/coursier/version/Version.scala
1 parent cfc35f4 commit d8e6ddf

3 files changed

Lines changed: 16 additions & 7 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ final case class Version(value: String) {
122122
}
123123

124124
object Version {
125-
case class Update(currentVersion: Version, nextVersion: Version)
125+
case class Update(currentVersion: Version, nextVersion: Version) {
126+
def obeysCoursierOrdering: Boolean =
127+
coursier.core.Version(nextVersion.value) >= coursier.core.Version(currentVersion.value)
128+
}
126129

127130
def show(versions: Version*): String = {
128131
val vs0 = versions.map(_.value)

modules/core/src/main/scala/org/scalasteward/core/update/FilterAlg.scala

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,7 @@ object FilterAlg {
129129
}
130130

131131
private def checkVersionOrdering(
132-
update: Update.ForArtifactId
133-
): Either[RejectionReason, Update.ForArtifactId] = {
134-
val current = coursier.core.Version(update.currentVersion.value)
135-
val next = coursier.core.Version(update.nextVersion.value)
136-
if (current > next) Left(VersionOrderingConflict(update)) else Right(update)
137-
}
132+
u: Update.ForArtifactId
133+
): Either[RejectionReason, Update.ForArtifactId] =
134+
Either.cond(u.versionUpdate.obeysCoursierOrdering, u, VersionOrderingConflict(u))
138135
}

modules/core/src/test/scala/org/scalasteward/core/data/VersionTest.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,21 @@ import cats.kernel.laws.discipline.OrderTests
55
import munit.DisciplineSuite
66
import org.scalacheck.Prop.*
77
import org.scalasteward.core.TestInstances.*
8+
import org.scalasteward.core.TestSyntax.StringOps
89
import org.scalasteward.core.data.Version.Component
10+
911
import scala.util.Random
1012

1113
class VersionTest extends DisciplineSuite {
1214
checkAll("Order[Version]", OrderTests[Version].order)
1315

16+
test("Note that currently, we treat Snapshot vs Milestone differently to Coursier") {
17+
val snapshot = "1.0.0-SNAP8"
18+
val milestone = "1.0.0-M2"
19+
assert(snapshot.v < milestone.v)
20+
assert(coursier.core.Version(snapshot) > coursier.core.Version(milestone)) // Surprising to me
21+
}
22+
1423
test("issue 1615: broken transitivity") {
1524
val res = OrderTests[Version].laws.transitivity(Version(""), Version("0"), Version("X"))
1625
assertEquals(res.lhs, res.rhs)

0 commit comments

Comments
 (0)