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/CHANGELOG.md b/CHANGELOG.md
index dc169fb..cf95427 100644
--- a/CHANGELOG.md
+++ b/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/lib/src/utils.dart b/lib/src/utils.dart
index 74d015a..6493dda 100644
--- a/lib/src/utils.dart
+++ b/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/lib/src/version.dart b/lib/src/version.dart
index 9872289..f2662b9 100644
--- a/lib/src/version.dart
+++ b/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/lib/src/version_constraint.dart b/lib/src/version_constraint.dart
index 9f55da0..68827a4 100644
--- a/lib/src/version_constraint.dart
+++ b/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/lib/src/version_range.dart b/lib/src/version_range.dart
index 145151f..a8d6069 100644
--- a/lib/src/version_range.dart
+++ b/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/lib/src/version_union.dart b/lib/src/version_union.dart
index dbdadcf..d0e1e2b 100644
--- a/lib/src/version_union.dart
+++ b/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/test/version_range_test.dart b/test/version_range_test.dart
index 9eaa980..eaa473f 100644
--- a/test/version_range_test.dart
+++ b/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");
}
