Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for range and comparators comparison #187

Closed
wants to merge 5 commits into from
Closed

Add support for range and comparators comparison #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a29e8db
Add support for range and comparators comparison
rtfpessoa Apr 1, 2017
b5504e7
Move methods to comparator and range
rtfpessoa Apr 1, 2017
57c77f3
Add range and comparator intersection docs to README
rtfpessoa Apr 9, 2017
0cb989f
Reduce line length and remove unused platform parameter
rtfpessoa Jul 15, 2017
4fa71d9
Make exposed apis more clear
rtfpessoa Jul 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
.*.swp
coverage/
.nyc_output/
.idea
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this belongs in a global git config, rather than polluting specific git repos with IDE-specific knowledge (nbd tho)

3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ strings that they parse.
(`major`, `premajor`, `minor`, `preminor`, `patch`, `prepatch`, or `prerelease`),
or null if the versions are the same.

### Comparators
* `intersects(comparator)`: Return true if the comparators intersect

### Ranges

Expand All @@ -347,6 +349,7 @@ strings that they parse.
the bounds of the range in either the high or low direction. The
`hilo` argument must be either the string `'>'` or `'<'`. (This is
the function called by `gtr` and `ltr`.)
* `intersects(range)`: Return true if any of the ranges comparators intersect

Note that, since ranges may be non-contiguous, a version might not be
greater than a range, less than a range, *or* satisfy a range! For
Expand Down
55 changes: 55 additions & 0 deletions semver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,45 @@ Comparator.prototype.test = function(version) {
return cmp(version, this.operator, this.semver, this.loose);
};

Comparator.prototype.intersects = function(comp, loose) {
if (!(comp instanceof Comparator)) {
throw new TypeError('a Comparator is required');
}

var rangeTmp;

if (this.operator === '') {
rangeTmp = new Range(comp.value, loose);
return satisfies(this.value, rangeTmp, loose);
} else if (comp.operator === '') {
rangeTmp = new Range(this.value, loose);
return satisfies(comp.semver, rangeTmp, loose);
}

var sameDirectionIncreasing =
(this.operator === '>=' || this.operator === '>') &&
(comp.operator === '>=' || comp.operator === '>');
var sameDirectionDecreasing =
(this.operator === '<=' || this.operator === '<') &&
(comp.operator === '<=' || comp.operator === '<');
var sameSemVer = this.semver.raw === comp.semver.raw;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you're after isn't semver.raw here, but semver.version. Consider this:

> r2 = new s.Range('>=v1.2.3')
Range {
  loose: undefined,
  raw: '>=v1.2.3',
  set: [ [ [Object] ] ],
  range: '>=1.2.3' }
> r = new s.Range('<=1.2.3')
Range {
  loose: undefined,
  raw: '<=1.2.3',
  set: [ [ [Object] ] ],
  range: '<=1.2.3' }
> r.intersects(r2)
false

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that fixes it. I'll include it in the merge, no worries.

var differentDirectionsInclusive =
(this.operator === '>=' || this.operator === '<=') &&
(comp.operator === '>=' || comp.operator === '<=');
var oppositeDirectionsLessThan =
cmp(this.semver, '<', comp.semver, loose) &&
((this.operator === '>=' || this.operator === '>') &&
(comp.operator === '<=' || comp.operator === '<'));
var oppositeDirectionsGreaterThan =
cmp(this.semver, '>', comp.semver, loose) &&
((this.operator === '<=' || this.operator === '<') &&
(comp.operator === '>=' || comp.operator === '>'));

return sameDirectionIncreasing || sameDirectionDecreasing ||
(sameSemVer && differentDirectionsInclusive) ||
oppositeDirectionsLessThan || oppositeDirectionsGreaterThan;
};


exports.Range = Range;
function Range(range, loose) {
Expand Down Expand Up @@ -783,6 +822,22 @@ Range.prototype.parseRange = function(range) {
return set;
};

Range.prototype.intersects = function(range, loose) {
if (!(range instanceof Range)) {
throw new TypeError('a Range is required');
}

return this.set.some(function(thisComparators) {
return thisComparators.every(function(thisComparator) {
return range.set.some(function(rangeComparators) {
return rangeComparators.every(function(rangeComparator) {
return thisComparator.intersects(rangeComparator, loose);
});
});
});
});
};

// Mostly just for testing and legacy API reasons
exports.toComparators = toComparators;
function toComparators(range, loose) {
Expand Down
81 changes: 81 additions & 0 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var replaceStars = semver.replaceStars;
var toComparators = semver.toComparators;
var SemVer = semver.SemVer;
var Range = semver.Range;
var Comparator = semver.Comparator;

test('\ncomparison tests', function(t) {
// [version1, version2]
Expand Down Expand Up @@ -712,3 +713,83 @@ test('\nmin satisfying', function(t) {
});
t.end();
});

test('\nintersect comparators', function(t) {
[
// One is a Version
['1.3.0', '>=1.3.0', true],
['1.3.0', '>1.3.0', false],
['>=1.3.0', '1.3.0', true],
['>1.3.0', '1.3.0', false],
// Same direction increasing
['>1.3.0', '>1.2.0', true],
['>1.2.0', '>1.3.0', true],
['>=1.2.0', '>1.3.0', true],
['>1.2.0', '>=1.3.0', true],
// Same direction decreasing
['<1.3.0', '<1.2.0', true],
['<1.2.0', '<1.3.0', true],
['<=1.2.0', '<1.3.0', true],
['<1.2.0', '<=1.3.0', true],
// Different directions, same semver and inclusive operator
['>=1.3.0', '<=1.3.0', true],
['>=1.3.0', '>=1.3.0', true],
['<=1.3.0', '<=1.3.0', true],
['>1.3.0', '<=1.3.0', false],
['>=1.3.0', '<1.3.0', false],
// Opposite matching directions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could instead be done in the iterator function, for every test, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not understanding exactly what you mean here. Can you extend a bit?

Also, should I amend my previous commit or should I create new commits to fix the requested changes?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, line 752 asks if comparator1 intersects comparator2; could you also add an assertion after that asking if comparator2 intersects comparator1?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think amending and force pushing is always fine; unless one of this repo's collabs says different.

['>1.0.0', '<2.0.0', true],
['>=1.0.0', '<2.0.0', true],
['>=1.0.0', '<=2.0.0', true],
['>1.0.0', '<=2.0.0', true],
['<=2.0.0', '>1.0.0', true],
['<=1.0.0', '>=2.0.0', false]
].forEach(function(v) {
var comparator1 = new Comparator(v[0]);
var comparator2 = new Comparator(v[1]);
var expect = v[2];

var actual1 = comparator1.intersects(comparator2);
var actual2 = comparator2.intersects(comparator1);
t.equal(actual1, expect);
t.equal(actual2, expect);
});
t.end();
});

test('\nmissing comparator parameter in intersect comparators', function(t) {
t.throws(function() {
new Comparator('>1.0.0').intersects();
}, new TypeError('a Comparator is required'),
'throws type error');
t.end();
});

test('\nranges intersect', function(t) {
[
['1.3.0 || <1.0.0 >2.0.0', '1.3.0 || <1.0.0 >2.0.0', true],
['<1.0.0 >2.0.0', '>0.0.0', true],
['<1.0.0 >2.0.0', '>1.4.0 <1.6.0', false],
['<1.0.0 >2.0.0', '>1.4.0 <1.6.0 || 2.0.0', false],
['>1.0.0 <=2.0.0', '2.0.0', true],
['<1.0.0 >=2.0.0', '2.1.0', false],
['<1.0.0 >=2.0.0', '>1.4.0 <1.6.0 || 2.0.0', false]
].forEach(function(v) {
var range1 = new Range(v[0]);
var range2 = new Range(v[1]);
var expect = v[2];
var actual1 = range1.intersects(range2);
var actual2 = range2.intersects(range1);
t.equal(actual1, expect);
t.equal(actual2, expect);
});
t.end();
});

test('\nmissing range parameter in range intersect', function(t) {
t.throws(function() {
new Range('1.0.0').intersects();
}, new TypeError('a Range is required'),
'throws type error');
t.end();
});