Introduce semantic equality, make (non-semantic) comparison total

README.md

@@ -61,7 +61,24 @@ The coursier documentation [details](https://get-coursier.io/docs/other-version-
 how versions are compared.
 
 `Version` implements `Ordered[Version]`, so that `Version` instances can be compared together,
-and a sequence of `Version`s can be sorted.
+and a sequence of `Version`s can be sorted. That order is total, and consistent with `equals`:
+`a.compare(b)` is `0` if and only if `a == b`. Versions that mean the same thing but are spelled
+differently, like `1.0` and `1.0.0`, or `1.2+foo` and `1.2+bar`, are ordered by their
+representation, so that sorted collections and hash-based ones agree on which versions are
+distinct.
+
+Use `compareSemantic` to compare versions up to that equivalence - it returns `0` for versions
+that only differ by padding, separators, build metadata, or a release qualifier like `Final` or
+`GA`. This is the order that decides whether a version sits in an interval, or whether two
+constraints can be reconciled:
+```scala
+Version("1.0").compare(Version("1.0.0"))
+// res: Int = -2
+Version("1.0").compareSemantic(Version("1.0.0"))
+// res: Int = 0
+Version("1.1.0").compareSemantic(Version("1.1.0.Final"))
+// res: Int = 0
+```
 
 ## `VersionConstraint`
 

README.template.md

@@ -54,7 +54,24 @@ The coursier documentation [details](https://get-coursier.io/docs/other-version-
 how versions are compared.
 
 `Version` implements `Ordered[Version]`, so that `Version` instances can be compared together,
-and a sequence of `Version`s can be sorted.
+and a sequence of `Version`s can be sorted. That order is total, and consistent with `equals`:
+`a.compare(b)` is `0` if and only if `a == b`. Versions that mean the same thing but are spelled
+differently, like `1.0` and `1.0.0`, or `1.2+foo` and `1.2+bar`, are ordered by their
+representation, so that sorted collections and hash-based ones agree on which versions are
+distinct.
+
+Use `compareSemantic` to compare versions up to that equivalence - it returns `0` for versions
+that only differ by padding, separators, build metadata, or a release qualifier like `Final` or
+`GA`. This is the order that decides whether a version sits in an interval, or whether two
+constraints can be reconciled:
+```scala
+Version("1.0").compare(Version("1.0.0"))
+// res: Int = -2
+Version("1.0").compareSemantic(Version("1.0.0"))
+// res: Int = 0
+Version("1.1.0").compareSemantic(Version("1.1.0.Final"))
+// res: Int = 0
+```
 
 ## `VersionConstraint`
 

versions/shared/src/coursier/version/Version.scala

@@ -18,9 +18,48 @@ case class Version(repr: String) extends Ordered[Version] {
       items0 = Version.items(repr)
     items0
   }
-  def compare(other: Version) = {
+  /**
+   * Total order on versions, consistent with `equals` and `hashCode`.
+   *
+   * `a.compare(b) == 0` if and only if `a == b`. Versions that are semantically
+   * equivalent but spelled differently (`1.0` and `1.0.0`, `1.2` and `1.2+foo`,
+   * `1.2+bar` and `1.2+foo`, …) are ordered by their representation, so that
+   * sorted collections and hash-based ones agree on which versions are distinct.
+   *
+   * Use [[compareSemantic]] to compare versions up to that equivalence.
+   */
+  def compare(other: Version): Int = {
+    if (repr == other.repr) 0 // fast path
+    else {
+      val cmp = Version.listCompare(items, other.items)
+      // Break ties so that the order is total: versions that only differ by
+      // padding, separators, or build metadata are ordered by representation.
+      if (cmp == 0) repr.compareTo(other.repr)
+      else cmp
+    }
+  }
+
+  /**
+   * Compares versions up to the equivalence induced by version parsing.
+   *
+   * Returns `0` for versions that have the same meaning but different
+   * representations, like `1.0` and `1.0.0`, `1.1.0` and `1.1.0.Final`, or
+   * `1.2+foo` and `1.2+bar` (Semver § 10: build metadata doesn't take part in
+   * precedence).
+   *
+   * This is *not* consistent with `equals` - it is the order to use to decide
+   * whether a version sits in an interval, or whether two versions can be
+   * reconciled. Use [[compare]] for sorting or for sorted collections.
+   */
+  def compareSemantic(other: Version): Int =
     if (repr == other.repr) 0 // fast path
-    else Version.listCompare(items, other.items)
+    else Version.listCompare(semanticItems, other.semanticItems)
+
+  private var semanticItems0: Vector[Version.Item] = null
+  private def semanticItems: Vector[Version.Item] = {
+    if (semanticItems0 == null)
+      semanticItems0 = Version.semanticItems(items)
+    semanticItems0
   }
   def isEmpty = items.forall(_.isEmpty)
 
@@ -121,6 +160,16 @@ object Version {
 
     override def compareToEmpty = level.compare(0)
     def isPreRelease: Boolean = level < Tag.emptyLevel
+
+    /**
+     * Whether this tag stands for the absence of a qualifier, like the `Final` of
+     * `3.10.1.Final` or the `GA` of `1.2.3.GA`.
+     *
+     * [[Version.compare]] orders those after the empty item, so that no two versions compare
+     * equal, while [[Version.compareSemantic]] treats them as the empty tag.
+     */
+    def isReleaseEquivalent: Boolean =
+      level == Tag.emptyLevel || level == Tag.gaLevel || level == Tag.finalLevel
     def compareTag(other: Tag): Int = {
       val levelComp = level.compare(other.level)
       if (levelComp == 0 && level == Tag.otherLevel) value.compareToIgnoreCase(other.value)
@@ -281,6 +330,26 @@ object Version {
     first +: tokens.toVector.map(_._2)
   }
 
+  private val emptyTag = Tag("")
+
+  /**
+   * Replaces the tags that stand for the absence of a qualifier - `ga` and `final` - by the
+   * empty tag, so that `1.1.0`, `1.1.0.GA` and `1.1.0.Final` compare equal.
+   *
+   * Those tags are ordered right after the empty item, and nothing else sits between them, so
+   * that collapsing them keeps the order of everything else unchanged.
+   */
+  private def semanticItems(items: Vector[Item]): Vector[Item] = {
+    def isReleaseEquivalent(item: Item): Boolean =
+      item match {
+        case t: Tag => t.isReleaseEquivalent
+        case _ => false
+      }
+    if (items.exists(isReleaseEquivalent))
+      items.map(item => if (isReleaseEquivalent(item)) emptyTag else item)
+    else items
+  }
+
   // before comparing two versions pad the number parts to the equal number of digits
   // for example, 1-ga, and 1.0.0 comparison will be adjusted first to 1.0.0-ga and 1.0.0.
   def listCompare(first0: Vector[Item], second0: Vector[Item]): Int = {

versions/shared/src/coursier/version/VersionCompatibility.scala

@@ -25,6 +25,52 @@ sealed abstract class VersionCompatibility {
 
 object VersionCompatibility {
 
+  /**
+    * Whether `version` carries a pre-release qualifier.
+    *
+    * Two kinds of items count as pre-releases:
+    *   - pre-release qualifiers, wherever they appear (`1.2.3-RC1`, `2.13.0-M3`, `1.2.3.RC1`),
+    *   - plain literals right after a `-`, which is where semantic versioning puts
+    *     pre-releases (`1.0.0-preview`).
+    *
+    * Release qualifiers and plain literals in the fourth, dot-separated segment of a
+    * Maven-style version are deliberately *not* pre-releases: `3.10.1.Final` and
+    * `9.4.25.v20191220` are releases.
+    */
+  private def hasPreReleaseQualifier(version: Version): Boolean = {
+    val (first, rest) = Version.Tokenizer(version.repr)
+    def preRelease(item: Version.Item, afterHyphen: Boolean): Boolean =
+      item match {
+        case t: Version.Tag =>
+          if (t.isQualifier) t.isPreRelease else afterHyphen
+        case _ => false
+      }
+    preRelease(first, afterHyphen = false) ||
+    rest.exists {
+      case (sep, item) => preRelease(item, sep == Version.Tokenizer.Hyphen)
+    }
+  }
+
+  /**
+    * Whether `version` can take part in a semantic versioning compatibility check as a constraint.
+    *
+    * Its significant part (the major number, or major and minor for 0.x versions) must be numeric,
+    * the rest must be made of numbers, qualifiers and build metadata, and it must not be a
+    * pre-release - per semantic versioning, a constraint like `1.2.3-RC1` only accepts itself.
+    */
+  private def isSemVerComparable(version: Version, significantPartLength: Int): Boolean = {
+    val items = version.items
+    items.lengthCompare(significantPartLength) >= 0 &&
+    items.take(significantPartLength).forall(_.isNumber) &&
+    items.drop(significantPartLength).forall {
+      case _: Version.Numeric => true
+      case _: Version.BuildMetadata => true
+      case _: Version.Tag => true
+      case _ => false // Min / Max
+    } &&
+    !hasPreReleaseQualifier(version)
+  }
+
   case object Default extends VersionCompatibility {
     def isCompatible(constraint: String, version: String): Boolean =
       PackVer.isCompatible(constraint, version)
@@ -85,7 +131,7 @@ object VersionCompatibility {
         if (c.interval == VersionInterval.zero)
           c.preferred.exists { wanted =>
             val toCompare = significativePartLength(v)
-            wanted.items.forall(_.isNumber) &&
+            isSemVerComparable(wanted, toCompare) &&
             wanted.items.take(toCompare) == v.items.take(toCompare) && {
               import Ordering.Implicits._
               wanted.items.drop(toCompare) <= v.items.drop(toCompare)
@@ -101,7 +147,7 @@ object VersionCompatibility {
         .filter(_.forall(_.isNumber))
         .map(_.collect { case n: Version.Numeric => n })
         .map(items => items.map(_.repr).mkString("."))
-        .filter(s => Version(s).compareTo(v) <= 0)
+        .filter(s => Version(s).compareSemantic(v) <= 0)
       candidateOpt.getOrElse(version)
     }
   }
@@ -118,7 +164,7 @@ object VersionCompatibility {
         val v = Version(version)
         if (c.interval == VersionInterval.zero)
           c.preferred.exists { wanted =>
-            wanted.items.forall(_.isNumber) &&
+            isSemVerComparable(wanted, 1) &&
             wanted.items.take(1) == v.items.take(1) &&
             v.items.take(1).exists(!_.isEmpty) && {
               import Ordering.Implicits._
@@ -134,7 +180,7 @@ object VersionCompatibility {
         .filter(items => items.nonEmpty && items.forall(_.isNumber) && items.forall(!_.isEmpty))
         .map(_.collect { case n: Version.Numeric => n })
         .map(items => items.map(_.repr).mkString("."))
-        .filter(s => Version(s).compareTo(v) <= 0)
+        .filter(s => Version(s).compareSemantic(v) <= 0)
       candidateOpt.getOrElse(version)
     }
   }
@@ -155,7 +201,7 @@ object VersionCompatibility {
         .filter(_.forall(_.isNumber))
         .map(_.collect { case n: Version.Numeric => n })
         .map(items => items.map(_.repr).mkString("."))
-        .filter(s => Version(s).compareTo(v) <= 0)
+        .filter(s => Version(s).compareSemantic(v) <= 0)
       candidateOpt.getOrElse(version)
     }
   }

versions/shared/src/coursier/version/VersionConstraint.scala

@@ -18,13 +18,13 @@ sealed abstract class VersionConstraint extends Product with Serializable with O
 
   private lazy val compareKey = preferred.headOption.orElse(interval.from).getOrElse(Version.zero)
   def compare(other: VersionConstraint): Int =
-    compareKey.compare(other.compareKey)
+    compareKey.compareSemantic(other.compareKey)
 
   def isValid: Boolean =
     interval.isValid && preferred.forall { v =>
       interval.contains(v) ||
         interval.to.forall { to =>
-          val cmp = v.compare(to)
+          val cmp = v.compareSemantic(to)
           cmp < 0 || (cmp == 0 && interval.toIncluded)
         }
     }
@@ -87,7 +87,7 @@ object VersionConstraint {
           interval.from match {
             case Some(from) =>
               allPreferred.filter { v =>
-                val cmp = from.compare(v)
+                val cmp = from.compareSemantic(v)
                 cmp < 0 || (cmp == 0 && interval.fromIncluded)
               }
             case None =>

versions/shared/src/coursier/version/VersionInterval.scala

@@ -12,7 +12,7 @@ case class VersionInterval(
       for {
         f <- from
         t <- to
-        cmd = f.compare(t)
+        cmd = f.compareSemantic(t)
       } yield cmd < 0 || (cmd == 0 && fromIncluded && toIncluded)
 
     fromToOrder.forall(x => x) && (from.nonEmpty || !fromIncluded) && (to.nonEmpty || !toIncluded)
@@ -21,12 +21,12 @@ case class VersionInterval(
   def contains(version: Version): Boolean = {
     val fromCond =
       from.forall { from0 =>
-        val cmp = from0.compare(version)
+        val cmp = from0.compareSemantic(version)
         cmp < 0 || cmp == 0 && fromIncluded
       }
     lazy val toCond =
       to.forall { to0 =>
-        val cmp = version.compare(to0)
+        val cmp = version.compareSemantic(to0)
         cmp < 0 || cmp == 0 && toIncluded
       }
 
@@ -49,7 +49,7 @@ case class VersionInterval(
     val (newFrom, newFromIncluded) =
       (from, other.from) match {
         case (Some(a), Some(b)) =>
-          val cmp = a.compare(b)
+          val cmp = a.compareSemantic(b)
           if (cmp < 0) (Some(b), other.fromIncluded)
           else if (cmp > 0) (Some(a), fromIncluded)
           else (Some(a), fromIncluded && other.fromIncluded)
@@ -62,7 +62,7 @@ case class VersionInterval(
     val (newTo, newToIncluded) =
       (to, other.to) match {
         case (Some(a), Some(b)) =>
-          val cmp = a.compare(b)
+          val cmp = a.compareSemantic(b)
           if (cmp < 0) (Some(a), toIncluded)
           else if (cmp > 0) (Some(b), other.toIncluded)
           else (Some(a), toIncluded && other.toIncluded)

versions/shared/test/src/coursier/version/VersionCompatibilityTests.scala

@@ -115,6 +115,77 @@ object VersionCompatibilityTests extends TestSuite {
       }
     }
 
+    test("maven-style qualifiers") {
+
+      // https://github.com/coursier/versions/issues/10
+      // Netty and Jetty put a qualifier in a fourth, dot-separated segment. Those are releases,
+      // not pre-releases, so compatibility has to be decided on the numeric part.
+
+      test("early semver") {
+        implicit val compat = VersionCompatibility.EarlySemVer
+
+        test("netty") {
+          compatible("3.7.0.Final", "3.10.1.Final")
+          compatible("4.1.100.Final", "4.1.104.Final")
+          incompatible("3.10.1.Final", "3.7.0.Final")
+          incompatible("3.7.0.Final", "4.0.0.Final")
+        }
+        test("jetty") {
+          compatible("9.2.14.v20151106", "9.4.25.v20191220")
+          incompatible("9.4.25.v20191220", "9.2.14.v20151106")
+          incompatible("9.2.14.v20151106", "10.0.0.v20200101")
+        }
+        test("qualifier on one side only") {
+          compatible("4.1.100", "4.1.104.Final")
+          compatible("4.1.100.Final", "4.1.104")
+        }
+        test("other release qualifiers") {
+          compatible("1.2.3.RELEASE", "1.5.0.RELEASE")
+          compatible("1.2.3.GA", "1.5.0.GA")
+          compatible("8.0.1.jre8", "8.4.1.jre8")
+        }
+        test("still significant for 0.x") {
+          compatible("0.1.1.Final", "0.1.2.Final")
+          incompatible("0.1.1.Final", "0.2.0.Final")
+        }
+      }
+
+      test("semver spec") {
+        implicit val compat = VersionCompatibility.SemVerSpec
+
+        test {
+          compatible("3.7.0.Final", "3.10.1.Final")
+        }
+        test {
+          compatible("9.2.14.v20151106", "9.4.25.v20191220")
+        }
+        test {
+          incompatible("3.10.1.Final", "3.7.0.Final")
+        }
+      }
+
+      test("pre-releases stay incompatible") {
+        // a qualifier right after a '-' is a semver pre-release, and a constraint on a
+        // pre-release only accepts itself
+        val compatibilities = Seq(VersionCompatibility.EarlySemVer, VersionCompatibility.SemVerSpec)
+        val preReleases = Seq(
+          "1.2.3-RC1",
+          "1.2.3-M3",
+          "1.2.3-alpha1",
+          "1.2.3-beta2",
+          "1.2.3-SNAPSHOT",
+          "1.2.3-milestone2",
+          // well-known markers count as pre-releases wherever they show up
+          "1.2.3.RC1",
+          "1.2.3.SNAPSHOT"
+        )
+        for (compat <- compatibilities; wanted <- preReleases) {
+          val compatible = compat.isCompatible(wanted, "1.9.9")
+          Predef.assert(!compatible, s"Expected '1.9.9' not to be compatible with '$wanted' per $compat")
+        }
+      }
+    }
+
     test("package versioning") {
 
       implicit val compat = VersionCompatibility.PackVer