From f8efdb1ac2094bc1c8585009b02d5b45427cf880 Mon Sep 17 00:00:00 2001 From: tanishiking24 Date: Fri, 11 May 2018 02:15:43 +0900 Subject: [PATCH] Make semantic selector honor semver ordering around pre-release tags. - When major, minor, and patch are equal, a pre-release version has lower precedence than a normal version. Example: 1.0.0-alpha < 1.0.0. - Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot hyphen separated identifier from left to right until a difference is found as follows - identifiers consisting of only digits are compared numerically and identifiers with letters or hyphens are compared lexically in ASCII sort order. - Numeric identifiers always have lower precedence than non-numeric identifiers. - A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal. - Example: 1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta < 1.0.0-beta < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0. https://semver.org/#spec-item-11 --- .../librarymanagement/SemanticSelector.scala | 70 ++++++++++-- .../SemanticSelectorSpec.scala | 106 ++++++++++++++---- 2 files changed, 148 insertions(+), 28 deletions(-) diff --git a/core/src/main/scala/sbt/librarymanagement/SemanticSelector.scala b/core/src/main/scala/sbt/librarymanagement/SemanticSelector.scala index b728c939a..279f4fdd0 100644 --- a/core/src/main/scala/sbt/librarymanagement/SemanticSelector.scala +++ b/core/src/main/scala/sbt/librarymanagement/SemanticSelector.scala @@ -1,5 +1,7 @@ package sbt.librarymanagement +import scala.annotation.tailrec + /** * Semantic version selector API to check if the VersionNumber satisfies * conditions described by semantic version selector. @@ -43,9 +45,9 @@ object SemanticSelector { * * Comparators can be combined by spaces to form the intersection set of the comparators. * For example, `>1.2.3 <4.5.6` matches versions that are `greater than 1.2.3 AND less than 4.5.6`. - * - * The (intersection) set of comparators can combined by ` || ` (spaces are required) to form the - * union set of the intersection sets. So the semantic selector is in disjunctive normal form. + * + * The (intersection) set of comparators can combined by ` || ` (spaces are required) to form the + * union set of the intersection sets. So the semantic selector is in disjunctive normal form. * * Metadata and pre-release of VersionNumber are ignored. * So `1.0.0` matches any versions that have `1.0.0` as normal version with any pre-release version @@ -141,7 +143,8 @@ object SemanticSelector { op: SemSelOperator, major: Option[Long], minor: Option[Long], - patch: Option[Long] + patch: Option[Long], + tags: Seq[String] ) { def matches(version: VersionNumber): Boolean = { // Fill empty fields of version specifier with 0 or max value of Long. @@ -165,7 +168,11 @@ object SemanticSelector { val versionNumber = (version._1.getOrElse(0L), version._2.getOrElse(0L), version._3.getOrElse(0L)) val selector = (major.getOrElse(assumed), minor.getOrElse(assumed), patch.getOrElse(assumed)) - val cmp = implicitly[Ordering[(Long, Long, Long)]].compare(versionNumber, selector) + val normalVersionCmp = + implicitly[Ordering[(Long, Long, Long)]].compare(versionNumber, selector) + val cmp = + if (normalVersionCmp == 0) SemComparator.comparePreReleaseTags(version.tags, tags) + else normalVersionCmp op match { case Lte if cmp <= 0 => true case Lt if cmp < 0 => true @@ -194,7 +201,8 @@ object SemanticSelector { case Some(v) => v.toString } .mkString(".") - s"$op$versionStr" + val tagsStr = if (tags.nonEmpty) s"-${tags.mkString("-")}" else "" + s"$op$versionStr$tagsStr" } } private[SemanticSelector] object SemComparator { @@ -205,15 +213,16 @@ object SemanticSelector { (?:\.(\d+|[xX*]) (?:\.(\d+|[xX*]))? )? - )$ + )((?:-\w+)*)$ """.r private[this] def parse(comparator: String): SemComparator = { comparator match { - case ComparatorRegex(rawOp, rawMajor, rawMinor, rawPatch) => + case ComparatorRegex(rawOp, rawMajor, rawMinor, rawPatch, ts) => val opStr = Option(rawOp) val major = Option(rawMajor) val minor = Option(rawMinor) val patch = Option(rawPatch) + val tags = splitDash(ts) // Trim wildcard(x, X, *) and re-parse it. // By trimming it, comparator realize the property like @@ -223,6 +232,9 @@ object SemanticSelector { case None => false } if (hasXrangeSelector) { + if (tags.nonEmpty) + throw new IllegalArgumentException( + s"Pre-release version requires major, minor, patch versions to be specified: $comparator") val numbers = Seq(major, minor, patch).takeWhile { case Some(str) => str.matches("\\d+") case None => false @@ -235,6 +247,9 @@ object SemanticSelector { .mkString(".") ) } else { + if (tags.nonEmpty && (major.isEmpty || minor.isEmpty || patch.isEmpty)) + throw new IllegalArgumentException( + s"Pre-release version requires major, minor, patch versions to be specified: $comparator") val operator = opStr match { case Some("<") => Lt case Some("<=") => Lte @@ -249,11 +264,48 @@ object SemanticSelector { operator, major.map(_.toLong), minor.map(_.toLong), - patch.map(_.toLong) + patch.map(_.toLong), + tags ) } case _ => throw new IllegalArgumentException(s"Invalid comparator: $comparator") } } + private[this] def splitOn[A](s: String, sep: Char): Vector[String] = + if (s eq null) Vector() + else s.split(sep).filterNot(_ == "").toVector + private[this] def splitDash(s: String) = splitOn(s, '-') + + private[SemComparator] def comparePreReleaseTags(ts1: Seq[String], ts2: Seq[String]): Int = { + // > When major, minor, and patch are equal, a pre-release version has lower precedence than a normal version. + if (ts1.isEmpty && ts2.isEmpty) 0 + else if (ts1.nonEmpty && ts2.isEmpty) -1 // ts1 is pre-release version + else if (ts1.isEmpty && ts2.nonEmpty) 1 // ts2 is pre-release version + else compareTags(ts1, ts2) + } + + @tailrec + private[this] def compareTags(ts1: Seq[String], ts2: Seq[String]): Int = { + // > A larger set of pre-release fields has a higher precedence than a smaller set, + // > if all of the preceding identifiers are equal. + if (ts1.isEmpty && ts2.isEmpty) 0 + else if (ts1.nonEmpty && ts2.isEmpty) 1 + else if (ts1.isEmpty && ts2.nonEmpty) -1 + else { + val ts1head = ts1.head + val ts2head = ts2.head + val cmp = (ts1head.matches("\\d+"), ts2head.matches("\\d+")) match { + // Identifiers consisting of only digits are compared numerically. + // Numeric identifiers always have lower precedence than non-numeric identifiers. + // Identifiers with letters are compared case insensitive lexical order. + case (true, true) => implicitly[Ordering[Long]].compare(ts1head.toLong, ts2head.toLong) + case (false, true) => 1 + case (true, false) => -1 + case (false, false) => ts1head.toLowerCase.compareTo(ts2head.toLowerCase) + } + if (cmp == 0) compareTags(ts1.tail, ts2.tail) + else cmp + } + } } } diff --git a/core/src/test/scala/sbt/librarymanagement/SemanticSelectorSpec.scala b/core/src/test/scala/sbt/librarymanagement/SemanticSelectorSpec.scala index 4a33d45c5..f7bf4f5bf 100644 --- a/core/src/test/scala/sbt/librarymanagement/SemanticSelectorSpec.scala +++ b/core/src/test/scala/sbt/librarymanagement/SemanticSelectorSpec.scala @@ -5,9 +5,11 @@ import org.scalatest.{ FreeSpec, Matchers } class SemanticSelectorSpec extends FreeSpec with Matchers { semsel("<=1.2.3") { sel => assertMatches(sel, "1.2.3") + assertMatches(sel, "1.2-beta") assertMatches(sel, "1.2.3-beta") assertMatches(sel, "1.2") assertMatches(sel, "1") + assertNotMatches(sel, "1.2.4-alpha") assertNotMatches(sel, "1.2.4") assertNotMatches(sel, "1.3") assertNotMatches(sel, "1.3.0") @@ -15,68 +17,89 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { } semsel("<=1.2") { sel => - assertMatches(sel, "1.2.3") + assertMatches(sel, "1.2.345-beta") assertMatches(sel, "1.2.3-beta") + assertMatches(sel, "1.2.3") assertMatches(sel, "1.2") assertMatches(sel, "1") assertNotMatches(sel, "1.3.0") + assertNotMatches(sel, "1.3.0-alpha") } semsel("<=1") { sel => - assertMatches(sel, "1.12.12") - assertMatches(sel, "1.12.12-alpha") - assertMatches(sel, "1.2") + assertMatches(sel, "1.234.567-alpha") + assertMatches(sel, "1.234.567") + assertMatches(sel, "1.234") + assertMatches(sel, "1.0.0-alpha") + assertMatches(sel, "1.0.0") + assertMatches(sel, "1.0") + assertMatches(sel, "1") assertNotMatches(sel, "2.0.0") assertNotMatches(sel, "2.0.0-alpha") } semsel("<1.2.3") { sel => + assertMatches(sel, "1.2.3-alpha") assertMatches(sel, "1.2.2") assertMatches(sel, "1.2") - assertNotMatches(sel, "1.2.3-alpha") + assertMatches(sel, "1") + assertNotMatches(sel, "1.2.4-beta") assertNotMatches(sel, "1.2.3") + assertNotMatches(sel, "1.3") + assertNotMatches(sel, "2") } semsel("<1.2") { sel => + assertMatches(sel, "1.2.0-alpha") assertMatches(sel, "1.1.23") assertMatches(sel, "1.1") + assertMatches(sel, "1") + assertNotMatches(sel, "1.3-beta") + assertNotMatches(sel, "1.2.0") assertNotMatches(sel, "1.2") - assertNotMatches(sel, "1.2.0-alpha") + assertNotMatches(sel, "2") } semsel("<1") { sel => + assertMatches(sel, "1.0.0-beta") + assertMatches(sel, "0.9.9-beta") assertMatches(sel, "0.9.12") assertMatches(sel, "0.8") + assertMatches(sel, "0") + assertNotMatches(sel, "1.0.1-beta") assertNotMatches(sel, "1") assertNotMatches(sel, "1.0") assertNotMatches(sel, "1.0.0") } semsel(">=1.2.3") { sel => + assertMatches(sel, "1.2.4-beta") assertMatches(sel, "1.2.3") - assertMatches(sel, "1.2.3-beta") assertMatches(sel, "1.3") assertMatches(sel, "2") + assertNotMatches(sel, "1.2.3-beta") assertNotMatches(sel, "1.2.2") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1") } semsel(">=1.2") { sel => + assertMatches(sel, "1.2.1-beta") assertMatches(sel, "1.2.0") - assertMatches(sel, "1.2.0-beta") assertMatches(sel, "1.2") assertMatches(sel, "2") + assertNotMatches(sel, "1.2.0-beta") assertNotMatches(sel, "1.1.23") assertNotMatches(sel, "1.1") assertNotMatches(sel, "1") } semsel(">=1") { sel => + assertMatches(sel, "1.0.1-beta") assertMatches(sel, "1.0.0") - assertMatches(sel, "1.0.0-beta") assertMatches(sel, "1.0") assertMatches(sel, "1") + assertNotMatches(sel, "1.0.0-beta") assertNotMatches(sel, "0.9.9") assertNotMatches(sel, "0.1") assertNotMatches(sel, "0") @@ -87,6 +110,7 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { assertMatches(sel, "1.2.4-alpha") assertMatches(sel, "1.3") assertMatches(sel, "2") + assertNotMatches(sel, "1.2.3-alpha") assertNotMatches(sel, "1.2.3") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1") @@ -97,15 +121,18 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { assertMatches(sel, "1.3.0-alpha") assertMatches(sel, "1.3") assertMatches(sel, "2") + assertNotMatches(sel, "1.2.0-alpha") assertNotMatches(sel, "1.2.9") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1") } semsel(">1") { sel => + assertMatches(sel, "2.0.0-alpha") assertMatches(sel, "2.0.0") assertMatches(sel, "2.0") assertMatches(sel, "2") + assertNotMatches(sel, "1.2.3-alpha") assertNotMatches(sel, "1.2.3") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1") @@ -113,17 +140,19 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { semsel("1.2.3") { sel => assertMatches(sel, "1.2.3") - assertMatches(sel, "1.2.3-alpha") + assertNotMatches(sel, "1.2.3-alpha") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1.2.4") } Seq(".x", ".X", ".*", ".x.x", "").foreach { xrange => semsel(s"1$xrange") { sel => + assertMatches(sel, "1.2.3-alpha") assertMatches(sel, "1.0.0") assertMatches(sel, "1.0.1") assertMatches(sel, "1.1.1") - assertMatches(sel, "1.0.0-alpha") + assertNotMatches(sel, "1.0.0-alpha") + assertNotMatches(sel, "2.0.0-alpha") assertNotMatches(sel, "2.0.0") assertNotMatches(sel, "0.1.0") } @@ -132,8 +161,10 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { Seq(".x", ".X", ".*", "").foreach { xrange => semsel(s"1.2$xrange") { sel => assertMatches(sel, "1.2.0") - assertMatches(sel, "1.2.0-beta") assertMatches(sel, "1.2.3") + assertNotMatches(sel, "1.2.0-alpha") + assertNotMatches(sel, "1.2.0-beta") + assertNotMatches(sel, "1.3.0-beta") assertNotMatches(sel, "1.3.0") assertNotMatches(sel, "1.1.1") } @@ -141,23 +172,27 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { semsel("=1.2.3") { sel => assertMatches(sel, "1.2.3") - assertMatches(sel, "1.2.3-alpha") + assertNotMatches(sel, "1.2.3-alpha") assertNotMatches(sel, "1.2") assertNotMatches(sel, "1.2.4") } semsel("=1.2") { sel => assertMatches(sel, "1.2.0") - assertMatches(sel, "1.2.0-alpha") assertMatches(sel, "1.2") assertMatches(sel, "1.2.1") assertMatches(sel, "1.2.4") + assertNotMatches(sel, "1.1.0") + assertNotMatches(sel, "1.3.0") + assertNotMatches(sel, "1.2.0-alpha") + assertNotMatches(sel, "1.3.0-alpha") } semsel("=1") { sel => assertMatches(sel, "1.0.0") - assertMatches(sel, "1.0.0-alpha") assertMatches(sel, "1.0") assertMatches(sel, "1.0.1") assertMatches(sel, "1.2.3") + assertNotMatches(sel, "1.0.0-alpha") + assertNotMatches(sel, "2.0.0") } semsel("1.2.3 || 2.0.0") { sel => assertMatches(sel, "1.2.3") @@ -235,14 +270,41 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { semsel(">=1.x") { sel => assertMatches(sel, "1.0.0") - assertMatches(sel, "1.0.0-beta") assertMatches(sel, "1.0") assertMatches(sel, "1") + assertNotMatches(sel, "1.0.0-beta") assertNotMatches(sel, "0.9.9") assertNotMatches(sel, "0.1") assertNotMatches(sel, "0") } + semsel(">=1.2.3-beta") { sel => + assertMatches(sel, "1.3-alpha") + assertMatches(sel, "1.2.3") + assertMatches(sel, "1.2.3-beta") + assertMatches(sel, "1.2.3-beta-2") + assertMatches(sel, "1.2.3-beta-gamma") + assertMatches(sel, "1.2.4") + assertMatches(sel, "1.3") + assertNotMatches(sel, "1.2.3-alpha") + assertNotMatches(sel, "1.2.2") + } + + semsel(">=1.2.3-beta-2") { sel => + assertMatches(sel, "1.3-alpha") + assertMatches(sel, "1.2.3") + assertMatches(sel, "1.2.3-beta-2") + assertMatches(sel, "1.2.3-beta-2-3") + assertMatches(sel, "1.2.3-beta-3") + assertMatches(sel, "1.2.3-beta-gamma") + assertMatches(sel, "1.2.4") + assertMatches(sel, "1.3") + assertNotMatches(sel, "1.2.3-alpha-3") + assertNotMatches(sel, "1.2.3-beta-1") + assertNotMatches(sel, "1.2.3-beta") + assertNotMatches(sel, "1.2.2") + } + Seq( // invalid operator "~1.2.3", @@ -278,9 +340,15 @@ class SemanticSelectorSpec extends FreeSpec with Matchers { "1.0.0 -2.0.0", "1.0.0-2.0.0", "-", - // cannot specify pre-release or metadata - "1.2.3-alpha", - "1.2-alpha", + // minor and patch versions are required for pre-release version + "1.2-alpha-beta", + "1-beta", + "<=1.2-beta", + "<=1-beta", + "1.2-beta - 1.3-alpha", + "1.2.x-beta", + "1.x.*-beta", + // cannot specify metadata "1.2.3+meta" ).foreach { selectorStr => semsel(selectorStr) { sel =>