SI-6736 Range.contains is wrong

Removed once-used private method that was calculating ranges in error and corrected the contains method (plus improved performance).

2 files changed, 17 insertions, 8 deletions

diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala
index 786b18cd21..ba695dfbdc 100644
--- a/src/library/scala/collection/immutable/Range.scala
+++ b/src/library/scala/collection/immutable/Range.scala
@@ -203,12 +203,6 @@ extends scala.collection.AbstractSeq[Int]
     }
     counted
   }
-  // Tests whether a number is within the endpoints, without testing
-  // whether it is a member of the sequence (i.e. when step > 1.)
-  private def isWithinBoundaries(elem: Int) = !isEmpty && (
-    (step > 0 && start <= elem && elem <= last ) ||
-    (step < 0 &&  last <= elem && elem <= start)
-  )
   // Methods like apply throw exceptions on invalid n, but methods like take/drop
   // are forgiving: therefore the checks are with the methods.
   private def locationAfterN(n: Int) = start + (step * n)
@@ -256,7 +250,17 @@ extends scala.collection.AbstractSeq[Int]
     if (isInclusive) this
     else new Range.Inclusive(start, end, step)
 
-  final def contains(x: Int) = isWithinBoundaries(x) && ((x - start) % step == 0)
+  final def contains(x: Int) = {
+    if (x==end && !isInclusive) false
+    else if (step > 0) {
+      if (x < start || x > end) false
+      else (step == 1) || (((x - start) % step) == 0)
+    }
+    else {
+      if (x < end || x > start) false
+      else (step == -1) || (((x - start) % step) == 0)
+    }
+  }
 
   final override def sum[B >: Int](implicit num: Numeric[B]): Int = {
     if (num eq scala.math.Numeric.IntIsIntegral) {
diff --git a/test/junit/scala/collection/NumericRangeTest.scala b/test/junit/scala/collection/NumericRangeTest.scala
index 0260723b9d..f03bf1c498 100644
--- a/test/junit/scala/collection/NumericRangeTest.scala
+++ b/test/junit/scala/collection/NumericRangeTest.scala
@@ -6,7 +6,7 @@ import org.junit.Test
 import scala.math._
 import scala.util._
 
-/* Tests various maps by making sure they all agree on the same answers. */
+/* Tests various ranges by making sure they all agree on the same answers. */
 @RunWith(classOf[JUnit4])
 class RangeConsistencyTest {
   def r2nr[T: Integral](
@@ -120,4 +120,9 @@ class RangeConsistencyTest {
       case _ => false
     }
   }}
+  
+  @Test
+  def testSI6736() { assert{ 
+    (0 to Int.MaxValue).contains(4) && !((Int.MinValue to 0).contains(4))
+  } }
 }