Better handling of version intervals / hints reconciliation

Fixes https://github.com/alexarchambault/coursier/issues/303

build.sbt

@@ -148,6 +148,14 @@ lazy val core = crossProject
       
       Seq(
         // Since 1.0.0-M13
+        // reworked VersionConstraint
+        ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Interval"),
+        ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Preferred"),
+        ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Preferred$"),
+        ProblemFilters.exclude[MissingClassProblem]("coursier.core.VersionConstraint$Interval$"),
+        ProblemFilters.exclude[FinalClassProblem]("coursier.core.VersionConstraint"),
+        ProblemFilters.exclude[IncompatibleResultTypeProblem]("coursier.core.VersionConstraint.repr"),
+        ProblemFilters.exclude[IncompatibleMethTypeProblem]("coursier.core.VersionConstraint.this"),
         // Extra `actualVersion` field in `Project`
         ProblemFilters.exclude[MissingTypesProblem]("coursier.core.Project$"),
         ProblemFilters.exclude[MissingMethodProblem]("coursier.core.Project.apply"),

core/shared/src/main/scala/coursier/core/Parse.scala

@@ -43,12 +43,12 @@ object Parse {
   }
 
   def versionConstraint(s: String): Option[VersionConstraint] = {
-    def noConstraint = if (s.isEmpty) Some(VersionConstraint.None) else None
+    def noConstraint = if (s.isEmpty) Some(VersionConstraint.all) else None
 
     noConstraint
-      .orElse(ivyLatestSubRevisionInterval(s).map(VersionConstraint.Interval))
+      .orElse(ivyLatestSubRevisionInterval(s).map(VersionConstraint.interval))
-      .orElse(version(s).map(VersionConstraint.Preferred))
+      .orElse(version(s).map(VersionConstraint.preferred))
-      .orElse(versionInterval(s).map(VersionConstraint.Interval))
+      .orElse(versionInterval(s).map(VersionConstraint.interval))
   }
 
   val fallbackConfigRegex = {

core/shared/src/main/scala/coursier/core/Resolution.scala

@@ -140,25 +140,26 @@ object Resolution {
    * Returns `None` in case of conflict.
    */
   def mergeVersions(versions: Seq[String]): Option[String] = {
-    val (nonParsedConstraints, parsedConstraints) =
+
-      versions
+    val parseResults = versions.map(v => v -> Parse.versionConstraint(v))
-        .map(v => v -> Parse.versionConstraint(v))
+
-        .partition(_._2.isEmpty)
+    val nonParsedConstraints = parseResults.collect {
+      case (repr, None) => repr
+    }
 
     // FIXME Report this in return type, not this way
     if (nonParsedConstraints.nonEmpty)
       Console.err.println(
-        s"Ignoring unparsed versions: ${nonParsedConstraints.map(_._1)}"
+        s"Ignoring unparsed versions: $nonParsedConstraints"
       )
 
-    val intervalOpt =
+    val parsedConstraints = parseResults.collect {
-      (Option(VersionInterval.zero) /: parsedConstraints) {
+      case (_, Some(c)) => c
-        case (acc, (_, someCstr)) =>
+    }
-          acc.flatMap(_.merge(someCstr.get.interval))
-      }
 
-    intervalOpt
+    VersionConstraint
-      .map(_.constraint.repr)
+      .merge(parsedConstraints: _*)
+      .flatMap(_.repr)
   }
 
   /**

core/shared/src/main/scala/coursier/core/Versions.scala

@@ -1,5 +1,8 @@
 package coursier.core
 
+import scalaz.{ -\/, \/, \/- }
+import scalaz.Scalaz.ToEitherOps
+
 case class VersionInterval(from: Option[Version],
                            to: Option[Version],
                            fromIncluded: Boolean,
@@ -64,9 +67,9 @@ case class VersionInterval(from: Option[Version],
 
   def constraint: VersionConstraint =
     this match {
-      case VersionInterval.zero => VersionConstraint.None
+      case VersionInterval.zero => VersionConstraint.all
-      case VersionInterval(Some(version), None, true, false) => VersionConstraint.Preferred(version)
+      case VersionInterval(Some(version), None, true, false) => VersionConstraint.preferred(version)
-      case itv => VersionConstraint.Interval(itv)
+      case itv => VersionConstraint.interval(itv)
     }
 
   def repr: String = Seq(
@@ -82,23 +85,54 @@ object VersionInterval {
   val zero = VersionInterval(None, None, fromIncluded = false, toIncluded = false)
 }
 
-sealed abstract class VersionConstraint(
+final case class VersionConstraint(
-  val interval: VersionInterval,
+  interval: VersionInterval,
-  val repr: String
+  preferred: Seq[Version]
-)
+) {
+  def blend: Option[VersionInterval \/ Version] =
+    if (interval.isValid) {
+      val preferredInInterval = preferred.filter(interval.contains)
+
+      if (preferredInInterval.isEmpty)
+        Some(interval.left)
+      else
+        Some(preferredInInterval.max.right)
+    } else
+      None
+
+  def repr: Option[String] =
+    blend.map {
+      case -\/(itv) =>
+        if (itv == VersionInterval.zero)
+          ""
+        else
+          itv.repr
+      case \/-(v) => v.repr
+    }
+}
 
 object VersionConstraint {
-  /** Currently treated as minimum... */
+
-  final case class Preferred(version: Version) extends VersionConstraint(
+  def preferred(version: Version): VersionConstraint =
-    VersionInterval(Some(version), Option.empty, fromIncluded = true, toIncluded = false),
+    VersionConstraint(VersionInterval.zero, Seq(version))
-    version.repr
+  def interval(interval: VersionInterval): VersionConstraint =
-  )
+    VersionConstraint(interval, Nil)
-  final case class Interval(interval0: VersionInterval) extends VersionConstraint(
+
-    interval0,
+  val all = VersionConstraint(VersionInterval.zero, Nil)
-    interval0.repr
+
-  )
+  def merge(constraints: VersionConstraint*): Option[VersionConstraint] = {
-  case object None extends VersionConstraint(
+
-    VersionInterval.zero,
+    val intervals = constraints.map(_.interval)
-    "" // Once parsed, "(,)" becomes "" because of this
+
-  )
+    val intervalOpt =
+      (Option(VersionInterval.zero) /: intervals) {
+        case (acc, itv) =>
+          acc.flatMap(_.merge(itv))
+      }
+
+    for (interval <- intervalOpt) yield {
+      val preferreds = constraints.flatMap(_.preferred).distinct
+      VersionConstraint(interval, preferreds)
+    }
+  }
 }

tests/shared/src/test/resources/resolutions/org.webjars.bower/malihu-custom-scrollbar-plugin/3.1.5

@@ -0,0 +1,3 @@
+org.webjars.bower:jquery:3.1.0:compile
+org.webjars.bower:jquery-mousewheel:3.1.13:compile
+org.webjars.bower:malihu-custom-scrollbar-plugin:3.1.5:compile

tests/shared/src/test/scala/coursier/test/CentralTests.scala

@@ -237,6 +237,15 @@ object CentralTests extends TestSuite {
       )
     }
 
+    'versionInterval - {
+      // Warning: needs to be updated when new versions of org.webjars.bower:jquery and
+      // org.webjars.bower:jquery-mousewheel are published :-|
+      resolutionCheck(
+        Module("org.webjars.bower", "malihu-custom-scrollbar-plugin"),
+        "3.1.5"
+      )
+    }
+
     'latestRevision - {
       * - resolutionCheck(
         Module("com.chuusai", "shapeless_2.11"),

tests/shared/src/test/scala/coursier/test/VersionConstraintTests.scala

@@ -10,45 +10,30 @@ object VersionConstraintTests extends TestSuite {
     'parse{
       'empty{
         val c0 = Parse.versionConstraint("")
-        assert(c0 == Some(VersionConstraint.None))
+        assert(c0 == Some(VersionConstraint.all))
       }
       'basicVersion{
         val c0 = Parse.versionConstraint("1.2")
-        assert(c0 == Some(VersionConstraint.Preferred(Version("1.2"))))
+        assert(c0 == Some(VersionConstraint.preferred(Version("1.2"))))
       }
       'basicVersionInterval{
         val c0 = Parse.versionConstraint("(,1.2]")
-        assert(c0 == Some(VersionConstraint.Interval(VersionInterval(None, Some(Version("1.2")), false, true))))
+        assert(c0 == Some(VersionConstraint.interval(VersionInterval(None, Some(Version("1.2")), false, true))))
       }
     }
 
     'repr{
       'empty{
-        val s0 = VersionConstraint.None.repr
+        val s0 = VersionConstraint.all.repr
-        assert(s0 == "")
+        assert(s0 == Some(""))
       }
       'preferred{
-        val s0 = VersionConstraint.Preferred(Version("2.1")).repr
+        val s0 = VersionConstraint.preferred(Version("2.1")).repr
-        assert(s0 == "2.1")
+        assert(s0 == Some("2.1"))
       }
       'interval{
-        val s0 = VersionConstraint.Interval(VersionInterval(None, Some(Version("2.1")), false, true)).repr
+        val s0 = VersionConstraint.interval(VersionInterval(None, Some(Version("2.1")), false, true)).repr
-        assert(s0 == "(,2.1]")
+        assert(s0 == Some("(,2.1]"))
-      }
-    }
-
-    'interval{
-      'empty{
-        val s0 = VersionConstraint.None.interval
-        assert(s0 == VersionInterval.zero)
-      }
-      'preferred{
-        val s0 = VersionConstraint.Preferred(Version("2.1")).interval
-        assert(s0 == VersionInterval(Some(Version("2.1")), None, true, false))
-      }
-      'interval{
-        val s0 = VersionConstraint.Interval(VersionInterval(None, Some(Version("2.1")), false, true)).interval
-        assert(s0 == VersionInterval(None, Some(Version("2.1")), false, true))
       }
     }
   }

tests/shared/src/test/scala/coursier/test/VersionIntervalTests.scala

@@ -269,17 +269,17 @@ object VersionIntervalTests extends TestSuite {
       'none{
         val s1 = "(,)"
         val c1 = Parse.versionInterval(s1).map(_.constraint)
-        assert(c1 == Some(VersionConstraint.None))
+        assert(c1 == Some(VersionConstraint.all))
       }
       'preferred{
         val s1 = "[1.3,)"
         val c1 = Parse.versionInterval(s1).map(_.constraint)
-        assert(c1 == Some(VersionConstraint.Preferred(Parse.version("1.3").get)))
+        assert(c1 == Some(VersionConstraint.preferred(Parse.version("1.3").get)))
       }
       'interval{
         val s1 = "[1.3,2.4)"
         val c1 = Parse.versionInterval(s1).map(_.constraint)
-        assert(c1 == Some(VersionConstraint.Interval(VersionInterval(Parse.version("1.3"), Parse.version("2.4"), true, false))))
+        assert(c1 == Some(VersionConstraint.interval(VersionInterval(Parse.version("1.3"), Parse.version("2.4"), true, false))))
       }
     }
   }