Reduce unnecessary Comparators in Range set

There are a few cases that lead to Range's Comparator sets being longer
than strictly necessary.  This reduces performance of methods that
iterate over ranges repeatedly (for example, `intersects` and `subset`),
and leads to some confusing toString output like turning `x || * || X`
into `||||` instead of `*`.

- If any simple range in the set contains the null set <0.0.0-0, then
  the entire simple range is the null set. `2.x <0.0.0-0` is the same as
  just `<0.0.0-0`.  (This is used for `>*` and `<*`, which cannot match
  anything.)
- Ensure that a given Comparator will only occur once within each simple
  range set.  `2.3.x ^2.3` doesn't need to include `>=2.3.0` more than
  once.
- If a simple range set contains more than one comparator, remove any `*`
  comparators.  `* >=2.3.4` is the same as just `>=2.3.4`.  This was
  already being done in the cast to a string, but some `ANY` Comparators
  would be left behind in the set used for matching.
- If a Range set contains the simple range `*`, then drop any other
  simple ranges in the set. `* || 2.x` is the same as `*`.

There's still some unnecessary comparators in there.  For example, the
range `2.3 ^2.3.4` parses to `>=2.3.0 <2.4.0-0 >=2.3.4 <3.0.0-0`.  Of
course, anything that is `<2.4.0-0` is also `<3.0.0-0`, and anything
that is `>=2.3.4` is also `>=2.3.0`, so the `<3.0.0-0` and `>=2.3.0`
Comparators are not necessary.  But simplifying those out would be a bit
more work.

To do that, we could walk the set of Comparators checking to see if they
are a subset of any other Comparators in the list, and if so, removing
them.  The subset check would not have to be a full Range.subset(); we
could just see if the gtlt points in the same direction, and if one
semver is greater than the other.  It's an O(n^2) operation, but one on
typically very small n.

PR-URL: #324
Credit: @isaacs
Close: #324

Author: isaacs

classes/range.js

@@ -46,6 +46,24 @@ class Range {
       throw new TypeError(`Invalid SemVer Range: ${range}`)
     }
 
+    // if we have any that are not the null set, throw out null sets.
+    if (this.set.length > 1) {
+      // keep the first one, in case they're all null sets
+      const first = this.set[0]
+      this.set = this.set.filter(c => !isNullSet(c[0]))
+      if (this.set.length === 0)
+        this.set = [first]
+      else if (this.set.length > 1) {
+        // if we have any that are *, then the range is just *
+        for (const c of this.set) {
+          if (c.length === 1 && isAny(c[0])) {
+            this.set = [c]
+            break
+          }
+        }
+      }
+    }
+
     this.format()
   }
 
@@ -87,15 +105,31 @@ class Range {
     // ready to be split into comparators.
 
     const compRe = loose ? re[t.COMPARATORLOOSE] : re[t.COMPARATOR]
-    return range
+    const rangeList = range
       .split(' ')
       .map(comp => parseComparator(comp, this.options))
       .join(' ')
       .split(/\s+/)
+      // >=0.0.0 is equivalent to *
       .map(comp => replaceGTE0(comp, this.options))
       // in loose mode, throw out any that are not valid comparators
       .filter(this.options.loose ? comp => !!comp.match(compRe) : () => true)
       .map(comp => new Comparator(comp, this.options))
+
+    // if any comparators are the null set, then replace with JUST null set
+    // if more than one comparator, remove any * comparators
+    // also, don't include the same comparator more than once
+    const l = rangeList.length
+    const rangeMap = new Map()
+    for (const comp of rangeList) {
+      if (isNullSet(comp))
+        return [comp]
+      rangeMap.set(comp.value, comp)
+    }
+    if (rangeMap.size > 1 && rangeMap.has(''))
+      rangeMap.delete('')
+
+    return [...rangeMap.values()]
   }
 
   intersects (range, options) {
@@ -155,6 +189,9 @@ const {
   caretTrimReplace
 } = require('../internal/re')
 
+const isNullSet = c => c.value === '<0.0.0-0'
+const isAny = c => c.value === ''
+
 // take a set of comparators and determine whether there
 // exists a version which can satisfy it
 const isSatisfiable = (comparators, options) => {

test/fixtures/range-parse.js

@@ -38,7 +38,7 @@ module.exports = [
   ['>=0.2.3 || <0.0.1', '>=0.2.3||<0.0.1'],
   ['>=0.2.3 || <0.0.1', '>=0.2.3||<0.0.1'],
   ['>=0.2.3 || <0.0.1', '>=0.2.3||<0.0.1'],
-  ['||', '||'],
+  ['||', '*'],
   ['2.x.x', '>=2.0.0 <3.0.0-0'],
   ['1.2.x', '>=1.2.0 <1.3.0-0'],
   ['1.2.x || 2.x', '>=1.2.0 <1.3.0-0||>=2.0.0 <3.0.0-0'],
@@ -79,12 +79,14 @@ module.exports = [
   ['>01.02.03', null],
   ['~1.2.3beta', '>=1.2.3-beta <1.3.0-0', { loose: true }],
   ['~1.2.3beta', null],
-  ['^ 1.2 ^ 1', '>=1.2.0 <2.0.0-0 >=1.0.0 <2.0.0-0'],
+  ['^ 1.2 ^ 1', '>=1.2.0 <2.0.0-0 >=1.0.0'],
   ['1.2 - 3.4.5', '>=1.2.0 <=3.4.5'],
   ['1.2.3 - 3.4', '>=1.2.3 <3.5.0-0'],
   ['1.2 - 3.4', '>=1.2.0 <3.5.0-0'],
   ['>1', '>=2.0.0'],
   ['>1.2', '>=1.3.0'],
   ['>X', '<0.0.0-0'],
   ['<X', '<0.0.0-0'],
+  ['<x <* || >* 2.x', '<0.0.0-0'],
+  ['>x 2.x || * || <x', '*'],
 ]

test/ranges/to-comparators.js

@@ -37,7 +37,7 @@ test('comparators test', (t) => {
     ['>=0.2.3 || <0.0.1', [['>=0.2.3'], ['<0.0.1']]],
     ['>=0.2.3 || <0.0.1', [['>=0.2.3'], ['<0.0.1']]],
     ['>=0.2.3 || <0.0.1', [['>=0.2.3'], ['<0.0.1']]],
-    ['||', [[''], ['']]],
+    ['||', [['']]],
     ['2.x.x', [['>=2.0.0', '<3.0.0-0']]],
     ['1.2.x', [['>=1.2.0', '<1.3.0-0']]],
     ['1.2.x || 2.x', [['>=1.2.0', '<1.3.0-0'], ['>=2.0.0', '<3.0.0-0']]],
@@ -72,7 +72,11 @@ test('comparators test', (t) => {
     ['1.2.3 - 3.4', [['>=1.2.3', '<3.5.0-0']]],
     ['1.2.3 - 3', [['>=1.2.3', '<4.0.0-0']]],
     ['>*', [['<0.0.0-0']]],
-    ['<*', [['<0.0.0-0']]]
+    ['<*', [['<0.0.0-0']]],
+    ['>X', [['<0.0.0-0']]],
+    ['<X', [['<0.0.0-0']]],
+    ['<x <* || >* 2.x', [['<0.0.0-0']]],
+    ['>x 2.x || * || <x', [['']]],
   ].forEach(([pre, wanted]) => {
     const found = toComparators(pre)
     const jw = JSON.stringify(wanted)