The `compare` Implementation On Version Is Inconsistent with `equals` And Invalid For PVP

[isomarcte]

The implementation of the compare method on Version is inconsistent with equals.

scala> val a: Version = Version("1")
val a: coursier.version.Version = Version(1)

scala> val b: Version = Version("1.0")
val b: coursier.version.Version = Version(1.0)

scala> a.compareTo(b)
val res0: Int = 0

scala> a == b
val res1: Boolean = false

It is also inconsistent with hashCode.

scala> a.hashCode == b.hashCode
val res3: Boolean = false

It is also incorrect if one assumes the version scheme is pvp. From the specification for PVP,

Version number ordering is already defined by Cabal as the lexicographic ordering of the components. For example, 2.0.1 > 1.3.2, and 2.0.1.0 > 2.0.1. (The Data.Version.Version type and its Ord instance embody this ordering).

The important example there being that 2.0.1.0 > 2.0.1, not equal.

This means that the current compare definition for Version is incorrect if that version is supposed to represent a PVP version.

scala> Version("1.0.0").compare(Version("1.0.0.0"))
val res0: Int = 0

According to PVP, these should compare the same as,

scala> "1.0.0".compare("1.0.0.0")
val res2: Int = -2

I'm not exactly sure how this can easily be fixed. If for a moment we ignore the PVP use case and just focus on equals and hashCode, then we could resolve the issue using #12. This works, but as noted on the PR, the behavior here is likely unexpected for most users, though technically valid.

scala> Set(Version("1.0.0"), Version("1.0"), Version("1.0.0+SOME_META"))
val res0: scala.collection.immutable.Set[coursier.version.Version] = Set(Version(1.0.0))

On the other hand, we could redefine compare to just be lexicographic ordering and introduce a new method for comparing the expressed binary API with respect to a specific VersionCompatibility value. This https://github.com/isomarcte/versions/tree/fix-pvp-ordering has some work towards that front, however I paused on it as I'm pretty concerned that changing the definition of compare may have negative effects on coursier as a whole, though this does seem like the more viable solution to me.

Another option would be to change the definition for compare and introduce a new type which is effectively a tuple a Version and a VersionCompatibility. This could provide methods to attempt to compare binary APIs with respect to a specific version scheme, making the behavior more explicit.

Thoughts?

[alexarchambault]

Not sure why this slipped my attention for 3.5 years 😬

I kind of agree that the inconsistency between compare and equals can be surprising, but it's not totally inconsistent: if a.compare(b) != 0, we have !a.equals(b). The only problem is that sometimes, a.compare(b) == 0 while !a.equals(b).

This behavior is mentioned in the java.lang.Comparable javadoc (scala.math.Ordered extends that interface). Seems it's only a problem for some sorted set / map implementations. This javadoc mentions quotient sets: the order here induces quotient sets with equivalent classes containing elements that are unequal per equals. Somehow, the compare applies more to these equivalent classes.

[alexarchambault]

That being said, I've tended to dislike compare returning 0 for versions with different representations (things like 1.0 and 1.0.0 for example, that you mention too, also 1.0 and 1.0+thing). So I'll probably just make the order total at some point…