Make VersionRange implement Comparable. This changes the ordering of ranges in a VersionUnion. It used to order solely by the upper bound; it now orders by lower bound, then upper bound. In addition to being a more intuitive ordering, this is easier to programatically work with, because iterating through the ranges reaches each one in the order of the versions it contains. R=rnystrom@google.com Review URL: https://codereview.chromium.org//2035983002 .
diff --git a/pkgs/pub_semver/CHANGELOG.md b/pkgs/pub_semver/CHANGELOG.md index dc169fb..cf95427 100644 --- a/pkgs/pub_semver/CHANGELOG.md +++ b/pkgs/pub_semver/CHANGELOG.md
@@ -4,6 +4,9 @@ implement `new VersionConstraint.unionOf()` and `VersionConstraint.union()`. Now it's public so you can use it too. +* Make `VersionRange` implement `Comparable<VersionRange>`. Ranges are ordered + first by lower bound, then by upper bound. + # 1.2.4 * Fix all remaining strong mode warnings.
diff --git a/pkgs/pub_semver/lib/src/utils.dart b/pkgs/pub_semver/lib/src/utils.dart index 74d015a..6493dda 100644 --- a/pkgs/pub_semver/lib/src/utils.dart +++ b/pkgs/pub_semver/lib/src/utils.dart
@@ -15,10 +15,15 @@ /// A [Comparator] that compares the maximum versions of [range1] and [range2]. int compareMax(VersionRange range1, VersionRange range2) { - if (range1.max < range2.max) return -1; - if (range1.max > range2.max) return 1; + if (range1.max == null) { + if (range2.max == null) return 0; + return 1; + } else if (range2.max == null) { + return -1; + } - if (!range1.includeMax && range2.includeMax) return -1; - if (range1.includeMax && !range2.includeMax) return 1; + var result = range1.max.compareTo(range2.max); + if (result != 0) return result; + if (range1.includeMax != range2.includeMax) return range1.includeMax ? 1 : -1; return 0; }
diff --git a/pkgs/pub_semver/lib/src/version.dart b/pkgs/pub_semver/lib/src/version.dart index 9872289..f2662b9 100644 --- a/pkgs/pub_semver/lib/src/version.dart +++ b/pkgs/pub_semver/lib/src/version.dart
@@ -14,7 +14,7 @@ final _equality = const IterableEquality(); /// A parsed semantic version number. -class Version implements Comparable<Version>, VersionConstraint, VersionRange { +class Version implements VersionConstraint, VersionRange { /// No released version: i.e. "0.0.0". static Version get none => new Version(0, 0, 0); @@ -267,22 +267,26 @@ return new VersionConstraint.unionOf([this, other]); } - int compareTo(Version other) { - if (major != other.major) return major.compareTo(other.major); - if (minor != other.minor) return minor.compareTo(other.minor); - if (patch != other.patch) return patch.compareTo(other.patch); + int compareTo(VersionRange other) { + if (other is Version) { + if (major != other.major) return major.compareTo(other.major); + if (minor != other.minor) return minor.compareTo(other.minor); + if (patch != other.patch) return patch.compareTo(other.patch); - // Pre-releases always come before no pre-release string. - if (!isPreRelease && other.isPreRelease) return 1; - if (!other.isPreRelease && isPreRelease) return -1; + // Pre-releases always come before no pre-release string. + if (!isPreRelease && other.isPreRelease) return 1; + if (!other.isPreRelease && isPreRelease) return -1; - var comparison = _compareLists(preRelease, other.preRelease); - if (comparison != 0) return comparison; + var comparison = _compareLists(preRelease, other.preRelease); + if (comparison != 0) return comparison; - // Builds always come after no build string. - if (build.isEmpty && other.build.isNotEmpty) return -1; - if (other.build.isEmpty && build.isNotEmpty) return 1; - return _compareLists(build, other.build); + // Builds always come after no build string. + if (build.isEmpty && other.build.isNotEmpty) return -1; + if (other.build.isEmpty && build.isNotEmpty) return 1; + return _compareLists(build, other.build); + } else { + return -other.compareTo(this); + } } String toString() => _text;
diff --git a/pkgs/pub_semver/lib/src/version_constraint.dart b/pkgs/pub_semver/lib/src/version_constraint.dart index 9f55da0..68827a4 100644 --- a/pkgs/pub_semver/lib/src/version_constraint.dart +++ b/pkgs/pub_semver/lib/src/version_constraint.dart
@@ -194,7 +194,7 @@ throw new ArgumentError('Unknown VersionConstraint type $constraint.'); } - flattened.sort(compareMax); + flattened.sort(); var merged = <VersionRange>[]; for (var constraint in flattened) {
diff --git a/pkgs/pub_semver/lib/src/version_range.dart b/pkgs/pub_semver/lib/src/version_range.dart index 145151f..a8d6069 100644 --- a/pkgs/pub_semver/lib/src/version_range.dart +++ b/pkgs/pub_semver/lib/src/version_range.dart
@@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'utils.dart'; import 'version.dart'; import 'version_constraint.dart'; import 'version_union.dart'; @@ -11,7 +12,11 @@ /// If there is a minimum, then this only allows versions that are at that /// minimum or greater. If there is a maximum, then only versions less than /// that are allowed. In other words, this allows `>= min, < max`. -class VersionRange implements VersionConstraint { +/// +/// Version ranges are ordered first by their lower bounds, then by their upper +/// bounds. For example, `>=1.0.0 <2.0.0` is before `>=1.5.0 <2.0.0` is before +/// `>=1.5.0 <3.0.0`. +class VersionRange implements Comparable<VersionRange>, VersionConstraint { /// The minimum end of the range. /// /// If [includeMin] is `true`, this will be the minimum allowed version. @@ -315,6 +320,21 @@ return new VersionConstraint.unionOf([this, other]); } + int compareTo(VersionRange other) { + if (min == null) { + if (other.min == null) return compareMax(this, other); + return -1; + } else if (other.min == null) { + return 1; + } + + var result = min.compareTo(other.min); + if (result != 0) return result; + if (includeMin != other.includeMin) return includeMin ? -1 : 1; + + return compareMax(this, other); + } + String toString() { var buffer = new StringBuffer();
diff --git a/pkgs/pub_semver/lib/src/version_union.dart b/pkgs/pub_semver/lib/src/version_union.dart index dbdadcf..d0e1e2b 100644 --- a/pkgs/pub_semver/lib/src/version_union.dart +++ b/pkgs/pub_semver/lib/src/version_union.dart
@@ -19,7 +19,7 @@ /// /// This list has two invariants: /// - /// * Its contents are sorted from lowest to highest matched versions. + /// * Its contents are sorted using the standard ordering of [VersionRange]s. /// * Its contents are disjoint and non-adjacent. In other words, for any two /// constraints next to each other in the list, there's some version between /// those constraints that they don't match. @@ -74,8 +74,8 @@ return true; } - // Move the constraint with the higher max value forward. This ensures - // that we keep both lists in sync as much as possible. + // Move the constraint with the lower max value forward. This ensures that + // we keep both lists in sync as much as possible. if (compareMax(ourRanges.current, theirRanges.current) < 0) { ourRanges.moveNext(); } else { @@ -101,9 +101,9 @@ if (!intersection.isEmpty) newRanges.add(intersection); - // Move the constraint with the higher max value forward. This ensures - // that we keep both lists in sync as much as possible, and that large - // ranges have a chance to match multiple small ranges that they contain. + // Move the constraint with the lower max value forward. This ensures that + // we keep both lists in sync as much as possible, and that large ranges + // have a chance to match multiple small ranges that they contain. if (compareMax(ourRanges.current, theirRanges.current) < 0) { ourRanges.moveNext(); } else {
diff --git a/pkgs/pub_semver/test/version_range_test.dart b/pkgs/pub_semver/test/version_range_test.dart index 9eaa980..eaa473f 100644 --- a/pkgs/pub_semver/test/version_range_test.dart +++ b/pkgs/pub_semver/test/version_range_test.dart
@@ -445,4 +445,61 @@ expect(new VersionRange().isEmpty, isFalse); expect(new VersionRange(min: v123, max: v124).isEmpty, isFalse); }); + + group('compareTo()', () { + test("orders by minimum first", () { + _expectComparesSmaller( + new VersionRange(min: v003, max: v080), + new VersionRange(min: v010, max: v072)); + _expectComparesSmaller( + new VersionRange(min: v003, max: v080), + new VersionRange(min: v010, max: v080)); + _expectComparesSmaller( + new VersionRange(min: v003, max: v080), + new VersionRange(min: v010, max: v114)); + }); + + test("orders by maximum second", () { + _expectComparesSmaller( + new VersionRange(min: v003, max: v010), + new VersionRange(min: v003, max: v072)); + }); + + test("includeMin comes before !includeMin", () { + _expectComparesSmaller( + new VersionRange(min: v003, max: v080, includeMin: true), + new VersionRange(min: v003, max: v080, includeMin: false)); + }); + + test("includeMax comes after !includeMax", () { + _expectComparesSmaller( + new VersionRange(min: v003, max: v080, includeMax: false), + new VersionRange(min: v003, max: v080, includeMax: true)); + }); + + test("no minimum comes before small minimum", () { + _expectComparesSmaller( + new VersionRange(max: v010), + new VersionRange(min: v003, max: v010)); + _expectComparesSmaller( + new VersionRange(max: v010, includeMin: true), + new VersionRange(min: v003, max: v010)); + }); + + test("no maximium comes after large maximum", () { + _expectComparesSmaller( + new VersionRange(min: v003, max: v300), + new VersionRange(min: v003)); + _expectComparesSmaller( + new VersionRange(min: v003, max: v300), + new VersionRange(min: v003, includeMax: true)); + }); + }); +} + +void _expectComparesSmaller(VersionRange smaller, VersionRange larger) { + expect(smaller.compareTo(larger), lessThan(0), + reason: "expected $smaller to sort below $larger"); + expect(larger.compareTo(smaller), greaterThan(0), + reason: "expected $larger to sort above $smaller"); }