Add 'node-semver' version range comparison functions #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 25, 2020
The way [VersionRange] comparators were structured before, not enough information was retained for [CompareTo] to work without effectively repeating the implementations of the comparators. Broadly, changes in this commit restructure [VersionRange] to hold necessary information. More specifically, this commit: o Adds [SemanticVersion.MaxValue] and [MinValue] fields, with their documentation. o Adds [IComparable<SemanticVersion>] to the [IComparator] internal interface, allowing comparators to provide their own methods that the higher-level [VersionRange.CompareTo] can defer to. o Adds a [ContiguousComparator] class from which [UnaryComparator] and [BinaryComparator] derive, providing a common implementation where almost every comparator (except 'less than' and 'greater than') is decomposed into an inclusive range 'm <= x <= n'. o For 'less than' and 'greater than' comparators, adds a special class [ComparatorShell], which takes delegates to implement the [IComparator] interface in these two special cases. The effect of this on performance will probably be minimal. The worst contributor will be the introduction of an abstract class, but apart from this roughly the same number of objects are created as before. This commit knowingly breaks the following tests: o [RangeIntlParsingTests.BasicIdentification] o [RangeIntlParsingTests.MultipleComparatorParsing] o [RangeIntlParsingTests.MultipleSetParsing] These tests rely on inspecting hidden implementation detail in the instances of [IComparator] returned, using it to directly verify the output of the parser (rather than relying on functionality tests). It may be reasonable to delete these tests as functionality tests should indirectly verify the same behaviour.
This implementation passes many tests (156 of 234). Of those that it doesn't pass, all but 3 appear related to issues #19 and #20. Two of those three are surprising. They are: o '0.7.2-beta' > '0.7.x' o '1.3.0-alpha' < '>1.2.3' These are tests taken directly from 'node-semver', and at the moment they don't make sense to me. To start with, I would have said that it isn't possible for both to be true at once. Following on, '0.7.x' ---> '>=0.7.0 <0.8.0', and so '0.7.2-beta' does not seem like it should be greater (because it is lower in precedence than '0.8.0'). The code in this commit produces a result that says as much, which is the result I would expect. For the second case, the Semantic Versioning spec says precedence is based on the first difference evaluating two versions left-to-right which, if you ask me, is the minor version. Although '1.3.0-alpha' is a pre-release version, its minor version is above '1.2.3' and so it should have higher precedence. As '>1.2.3' includes anything with a precedence higher than '1.2.3', versions with a higher precedence than '1.3.0-alpha' will be matched and so I would expect comparison to say 'neither greater nor lesser', i.e. 0. This is the result that this code gives, but the 'node-semver' tests say otherwise. It might be worth raising an issue with 'node-semver'.
|
Until the situation 5a48a1c mentions is resolved, this feature is on hold (and so won't be included in the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Under
node-semver, it's possible to compare a version range against a semantic version to determine more than just whether the version range is satisfied. Similarly, version ranges can be compared and operated on with each other. Supporting this inMcSherry.SemanticVersioningseems beneficial as it maintains parity withnode-semveron version ranges.The functions are:
outside/gtr/ltr— comparisons to determine whether a semantic version is outwith the range covered by the version rangeintersects— determines whether two version ranges overlapsubset— determines whether one version range entirely includes anotherSomething to review in doing this will be changes to the
node-semverversion range specification. At present, we claim compatibility with v6.0.0, butnode-semveris now on v7.3.2. Revising our claim to a later version is just good housekeeping.