fix: use the tightest bound in gtr and ltr when a range has redundant bounds

ranges/outside.js

@@ -9,26 +9,29 @@ const gt = require('../functions/gt')
 const lt = require('../functions/lt')
 const lte = require('../functions/lte')
 const gte = require('../functions/gte')
+const eq = require('../functions/eq')
 
 const outside = (version, range, hilo, options) => {
   version = new SemVer(version, options)
   range = new Range(range, options)
 
-  let gtfn, ltefn, ltfn, comp, ecomp
+  let gtfn, ltefn, ltfn, comp, ecomp, xcomp
   switch (hilo) {
     case '>':
       gtfn = gt
       ltefn = lte
       ltfn = lt
       comp = '>'
       ecomp = '>='
+      xcomp = '<'
       break
     case '<':
       gtfn = lt
       ltefn = gte
       ltfn = gt
       comp = '<'
       ecomp = '<='
+      xcomp = '>'
       break
     default:
       throw new TypeError('Must provide a hilo val of "<" or ">"')
@@ -42,41 +45,65 @@ const outside = (version, range, hilo, options) => {
   // From now on, variable terms are as if we're in "gtr" mode.
   // but note that everything is flipped for the "ltr" function.
 
+  let satisfiable = false
   for (let i = 0; i < range.set.length; ++i) {
     const comparators = range.set[i]
 
+    // A set can hold several bounds on the same side, e.g. `>1.0.0 >=2.0.0 <3.0.0`
+    // (x-ranges and intersected ranges produce these). Only the tightest one is the
+    // real edge of the set, so the edges are found by operator, not by taking
+    // whichever comparators have the highest and lowest versions.
     let high = null
     let low = null
 
     comparators.forEach((comparator) => {
       if (comparator.semver === ANY) {
         comparator = new Comparator('>=0.0.0')
       }
-      high = high || comparator
-      low = low || comparator
-      if (gtfn(comparator.semver, high.semver, options)) {
-        high = comparator
-      } else if (ltfn(comparator.semver, low.semver, options)) {
-        low = comparator
+      const exact = !comparator.operator
+      if (exact || comparator.operator === comp || comparator.operator === ecomp) {
+        if (!low || gtfn(comparator.semver, low.semver, options) ||
+            (eq(comparator.semver, low.semver, options) &&
+             comparator.operator === comp)) {
+          low = comparator
+        }
+      }
+      if (exact || (comparator.operator !== comp && comparator.operator !== ecomp)) {
+        if (!high || ltfn(comparator.semver, high.semver, options) ||
+            (eq(comparator.semver, high.semver, options) &&
+             comparator.operator === xcomp)) {
+          high = comparator
+        }
       }
     })
 
-    // If the edge version comparator has a operator then our version
-    // isn't outside it
-    if (high.operator === comp || high.operator === ecomp) {
+    // If nothing bounds the set on this side, our version can't be beyond it
+    if (!high) {
       return false
     }
 
-    // If the lowest version comparator has an operator and our version
-    // is less than it then it isn't higher than the range
+    // A set whose edges cross can't be satisfied by any version, so it has
+    // nothing for our version to be beyond
+    if (low && (gtfn(low.semver, high.semver, options) ||
+        (eq(low.semver, high.semver, options) &&
+         (low.operator === comp || high.operator === xcomp)))) {
+      continue
+    }
+    satisfiable = true
+
+    // If our version is at or below the tightest lower edge of the set, then it
+    // isn't higher than the range
+    if (!low) {
+      continue
+    }
     if ((!low.operator || low.operator === comp) &&
         ltefn(version, low.semver)) {
       return false
     } else if (low.operator === ecomp && ltfn(version, low.semver)) {
       return false
     }
   }
-  return true
+  return satisfiable
 }
 
 module.exports = outside

test/fixtures/version-gt-range.js

@@ -59,4 +59,11 @@ module.exports = [
   ['=0.7.x', '0.8.2'],
   ['<0.7.x', '0.7.2'],
   ['0.7.x', '0.7.2-beta'],
+  // redundant bounds on the same side: only the tightest one is the edge
+  ['>=0.3.3 <=0.3.3', '3.3.3'],
+  ['<=0.3.3 >=0.3.3', '3.3.3'],
+  // a set no version can satisfy doesn't hide the rest of the range
+  ['>=2.0.0 <1.0.0 || 1.x', '3.0.0'],
+  ['>1.0.0 <=1.0.0 || 0.x', '3.0.0'],
+  ['<2.0.0 <=2.0.0', '2.0.0'],
 ]

test/fixtures/version-lt-range.js

@@ -61,4 +61,11 @@ module.exports = [
   ['1', '1.0.0beta', true],
   ['>=0.7.x', '0.6.2'],
   ['>1.2.3', '1.3.0-alpha'],
+  // redundant bounds on the same side: only the tightest one is the edge
+  ['<=3.3.3 >=3.3.3', '1.2.1'],
+  ['>=3.3.3 <=3.3.3', '1.2.1'],
+  // a set no version can satisfy doesn't hide the rest of the range
+  ['>=2.0.0 <1.0.0 || 1.x', '0.5.0'],
+  ['>=1.0.0 <1.0.0 || 2.x', '0.5.0'],
+  ['>2.0.0 >=2.0.0', '2.0.0'],
 ]

test/fixtures/version-not-gt-range.js

@@ -83,4 +83,12 @@ module.exports = [
   ['^0.1.0 || ~3.0.1 || 5.0.0', '5.0.0-0', true],
   ['^0.1.0 || ~3.0.1 || >4 <=5.0.0', '3.5.0'],
   ['0.7.x', '0.7.2-beta', { includePrerelease: true }],
+  // redundant bounds on the same side: only the tightest one is the edge
+  ['>1.0.0 >=2.0.0 <3.0.0', '1.5.0'],
+  ['1.2 <1.2.9 || >2.0.0', '1.2.10'],
+  ['>=1.0.0 >1.0.0 <2.0.0', '1.0.0'],
+  ['>1.0.0 >=1.0.0 <2.0.0', '1.0.0'],
+  // no version satisfies these, so none is greater than them
+  ['>=2.0.0 <1.0.0', '3.0.0'],
+  ['>=2.0.0 <=2.0.0 <2.0.0', '3.0.0'],
 ]

test/fixtures/version-not-lt-range.js

@@ -86,4 +86,12 @@ module.exports = [
   ['~1.0.0-alpha', '1.0.0-beta'],
   ['=0.1.0', '1.0.0'],
   ['>1.2.3', '1.3.0-alpha', { includePrerelease: true }],
+  // redundant bounds on the same side: only the tightest one is the edge
+  ['<3.0.0 <=2.0.0 >1.0.0', '2.5.0'],
+  ['1.2 <1.2.9 || >2.0.0', '1.2.10'],
+  ['<=2.0.0 <2.0.0 >1.0.0', '2.0.0'],
+  ['<2.0.0 <=2.0.0 >1.0.0', '2.0.0'],
+  // no version satisfies these, so none is less than them
+  ['>=2.0.0 <1.0.0', '0.5.0'],
+  ['<=2.0.0 >=2.0.0 >2.0.0', '1.0.0'],
 ]