From df57a25052740f37b216100e642f848506c710b6 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 3 Apr 2018 14:28:55 -0700 Subject: [PATCH] Fix a pre-release-exclusion bug The special logic for excluding 1.0.0-dev from <1.0.0 was only implemented for VersionRange.allows(), not for any other methods. This brings the other methods in line with allows(). Closes #20 --- CHANGELOG.md | 6 +++ lib/src/utils.dart | 46 +++++++++++++++++ lib/src/version_range.dart | 34 +------------ test/version_range_test.dart | 97 ++++++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d50c76826cbcb..53de84120a5db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# 1.3.4 + +* Fix a bug where `VersionRange.allowsAll()`, `VersionRange.allowsAny()`, and + `VersionRange.difference()` would return incorrect results for pre-release + versions with the same base version number as release versions. + # 1.3.3 * Fix a bug where `VersionRange.difference()` with a union constraint that diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 9527d2dbae99e..d793e7a957790 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -2,6 +2,9 @@ // 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 'package:collection/collection.dart'; + +import 'version.dart'; import 'version_range.dart'; /// Returns whether [range1] is immediately next to, but not overlapping, @@ -29,6 +32,10 @@ bool allowsHigher(VersionRange range1, VersionRange range2) { if (range1.max == null) return range2.max != null; if (range2.max == null) return false; + // `<1.0.0-dev.1` allows `1.0.0-dev.0` which is higher than any versions + // allowed by `<1.0.0`. + if (disallowedByPreRelease(range2, range1.max)) return true; + var comparison = range1.max.compareTo(range2.max); if (comparison == 1) return true; if (comparison == -1) return false; @@ -39,6 +46,7 @@ bool allowsHigher(VersionRange range1, VersionRange range2) { /// [range2]. bool strictlyLower(VersionRange range1, VersionRange range2) { if (range1.max == null || range2.min == null) return false; + if (disallowedByPreRelease(range1, range2.min)) return true; var comparison = range1.max.compareTo(range2.min); if (comparison == -1) return true; @@ -50,3 +58,41 @@ bool strictlyLower(VersionRange range1, VersionRange range2) { /// [range2]. bool strictlyHigher(VersionRange range1, VersionRange range2) => strictlyLower(range2, range1); + +// Returns whether [other] is disallowed by [range] because we disallow +// pre-release versions that have the same major, minor, and patch version as +// the max of a range, but only if neither the max nor the min is a pre-release +// of that version. +// +// This ensures that `^1.2.3` doesn't include `2.0.0-pre`, while also allowing +// both `>=2.0.0-pre.2 <2.0.0` and `>=1.2.3 <2.0.0-pre.7` to match +// `2.0.0-pre.5`. +// +// It's worth noting that this is different than [NPM's semantics][]. NPM +// disallows **all** pre-release versions unless their major, minor, and +// patch numbers match those of a prerelease min or max. This ensures that +// no prerelease versions will ever be selected if the user doesn't +// explicitly allow them. +// +// [NPM's semantics]: https://www.npmjs.org/doc/misc/semver.html#prerelease-tags +// +// Instead, we ensure that release versions will always be preferred over +// prerelease versions by ordering the release versions first in +// [Version.prioritize]. This means that constraints like `any` or +// `>1.2.3` can still match prerelease versions if they're the only things +// available. +bool disallowedByPreRelease(VersionRange range, Version other) { + var maxIsReleaseOfOther = !range.includeMax && + !range.max.isPreRelease && + other.isPreRelease && + _equalsWithoutPreRelease(other, range.max); + var minIsPreReleaseOfOther = range.min != null && + range.min.isPreRelease && + _equalsWithoutPreRelease(other, range.min); + return maxIsReleaseOfOther && !minIsPreReleaseOfOther; +} + +bool _equalsWithoutPreRelease(Version version1, Version version2) => + version1.major == version2.major && + version1.minor == version2.minor && + version1.patch == version2.patch; diff --git a/lib/src/version_range.dart b/lib/src/version_range.dart index 8e9b4ad13d7e8..b615c6f3db2e2 100644 --- a/lib/src/version_range.dart +++ b/lib/src/version_range.dart @@ -90,44 +90,12 @@ class VersionRange implements Comparable, VersionConstraint { if (max != null) { if (other > max) return false; if (!includeMax && other == max) return false; - - // Disallow pre-release versions that have the same major, minor, and - // patch version as the max, but only if neither the max nor the min is a - // pre-release of that version. This ensures that "^1.2.3" doesn't include - // "2.0.0-pre", while also allowing both ">=2.0.0-pre.2 <2.0.0" and - // ">=1.2.3 <2.0.0-pre.7" to match "2.0.0-pre.5". - // - // It's worth noting that this is different than [NPM's semantics][]. NPM - // disallows **all** pre-release versions unless their major, minor, and - // patch numbers match those of a prerelease min or max. This ensures that - // no prerelease versions will ever be selected if the user doesn't - // explicitly allow them. - // - // [NPM's semantics]: https://www.npmjs.org/doc/misc/semver.html#prerelease-tags - // - // Instead, we ensure that release versions will always be preferred over - // prerelease versions by ordering the release versions first in - // [Version.prioritize]. This means that constraints like "any" or - // ">1.2.3" can still match prerelease versions if they're the only things - // available. - var maxIsReleaseOfOther = !includeMax && - !max.isPreRelease && - other.isPreRelease && - _equalsWithoutPreRelease(other, max); - var minIsPreReleaseOfOther = min != null && - min.isPreRelease && - _equalsWithoutPreRelease(other, min); - if (maxIsReleaseOfOther && !minIsPreReleaseOfOther) return false; + if (disallowedByPreRelease(this, other)) return false; } return true; } - bool _equalsWithoutPreRelease(Version version1, Version version2) => - version1.major == version2.major && - version1.minor == version2.minor && - version1.patch == version2.patch; - bool allowsAll(VersionConstraint other) { if (other.isEmpty) return true; if (other is Version) return allows(other); diff --git a/test/version_range_test.dart b/test/version_range_test.dart index cefd9d370fe24..f01101269c69b 100644 --- a/test/version_range_test.dart +++ b/test/version_range_test.dart @@ -228,6 +228,50 @@ main() { range.allowsAll(new VersionRange(min: v123, max: v234).union(v140)), isFalse); }); + + group('pre-release versions', () { + test('of inclusive min are excluded', () { + var range = new VersionRange(min: v123, includeMin: true); + + expect( + range.allowsAll(new VersionConstraint.parse('>1.2.4-dev')), isTrue); + expect(range.allowsAll(new VersionConstraint.parse('>1.2.3-dev')), + isFalse); + }); + + test('of non-pre-release max are excluded', () { + var range = new VersionRange(max: v234); + + expect(range.allowsAll(new VersionConstraint.parse('<2.3.3')), isTrue); + expect(range.allowsAll(new VersionConstraint.parse('<2.3.4-dev')), + isFalse); + }); + + test( + 'of non-pre-release max are included if min is a pre-release of the ' + 'same version', () { + var range = + new VersionRange(min: new Version.parse('2.3.4-dev.0'), max: v234); + + expect( + range.allowsAll( + new VersionConstraint.parse('>2.3.4-dev.0 <2.3.4-dev.1')), + isTrue); + }); + + test('of pre-release max are included', () { + var range = new VersionRange(max: new Version.parse('2.3.4-dev.2')); + + expect(range.allowsAll(new VersionConstraint.parse('<2.3.4-dev.1')), + isTrue); + expect(range.allowsAll(new VersionConstraint.parse('<2.3.4-dev.2')), + isTrue); + expect(range.allowsAll(new VersionConstraint.parse('<=2.3.4-dev.2')), + isFalse); + expect(range.allowsAll(new VersionConstraint.parse('<2.3.4-dev.3')), + isFalse); + }); + }); }); group('allowsAny()', () { @@ -325,6 +369,52 @@ main() { range.allowsAny(new VersionRange(min: v234, max: v300).union(v010)), isFalse); }); + + group('pre-release versions', () { + test('of inclusive min are excluded', () { + var range = new VersionRange(min: v123, includeMin: true); + + expect( + range.allowsAny(new VersionConstraint.parse('<1.2.4-dev')), isTrue); + expect(range.allowsAny(new VersionConstraint.parse('<1.2.3-dev')), + isFalse); + }); + + test('of non-pre-release max are excluded', () { + var range = new VersionRange(max: v234); + + expect(range.allowsAny(new VersionConstraint.parse('>2.3.3')), isTrue); + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4-dev')), + isFalse); + }); + + test( + 'of non-pre-release max are included if min is a pre-release of the ' + 'same version', () { + var range = + new VersionRange(min: new Version.parse('2.3.4-dev.0'), max: v234); + + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4-dev.1')), + isTrue); + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4')), isFalse); + + expect(range.allowsAny(new VersionConstraint.parse('<2.3.4-dev.1')), + isTrue); + expect(range.allowsAny(new VersionConstraint.parse('<2.3.4-dev')), + isFalse); + }); + + test('of pre-release max are included', () { + var range = new VersionConstraint.parse('<2.3.4-dev.2'); + + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4-dev.1')), + isTrue); + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4-dev.2')), + isFalse); + expect(range.allowsAny(new VersionConstraint.parse('>2.3.4-dev.3')), + isFalse); + }); + }); }); group('intersect()', () { @@ -614,6 +704,13 @@ main() { [v003, new VersionRange(min: v010)])), equals(VersionConstraint.empty)); }); + + test("with a range with a pre-release min, returns the original", () { + expect( + new VersionRange(max: v200) + .difference(new VersionConstraint.parse(">=2.0.0-dev")), + equals(new VersionRange(max: v200))); + }); }); test('isEmpty', () {