SI-8932 Fix dropRight/takeRight implementations

ruippeixotog · ruippeixotog · commit 729e7a91f43c · 2014-11-11T01:45:22.000Z
I looked up at all the overrides of `IterableLike#takeRight` and `IterableLike#dropRight` and replaced usages of its argument `n` with `math.max(n, 0)` every time it was being used in an index subtraction. Fixes [SI-8932](https://issues.scala-lang.org/browse/SI-8932).
diff --git a/src/library/scala/collection/IndexedSeqOptimized.scala b/src/library/scala/collection/IndexedSeqOptimized.scala
@@ -141,10 +141,10 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {
   def drop(n: Int): Repr = slice(n, length)
 
   override /*IterableLike*/
-  def takeRight(n: Int): Repr = slice(length - n, length)
+  def takeRight(n: Int): Repr = slice(length - math.max(n, 0), length)
 
   override /*IterableLike*/
-  def dropRight(n: Int): Repr = slice(0, length - n)
+  def dropRight(n: Int): Repr = slice(0, length - math.max(n, 0))
 
   override /*TraversableLike*/
   def splitAt(n: Int): (Repr, Repr) = (take(n), drop(n))
diff --git a/src/library/scala/collection/IterableViewLike.scala b/src/library/scala/collection/IterableViewLike.scala
@@ -150,10 +150,10 @@ trait IterableViewLike[+A,
     sliding(size, 1) // we could inherit this, but that implies knowledge of the way the super class is implemented.
 
   override def dropRight(n: Int): This =
-    take(thisSeq.length - n)
+    take(thisSeq.length - math.max(n, 0))
 
   override def takeRight(n: Int): This =
-    drop(thisSeq.length - n)
+    drop(thisSeq.length - math.max(n, 0))
 
   override def stringPrefix = "IterableView"
 }
diff --git a/src/library/scala/collection/immutable/TreeMap.scala b/src/library/scala/collection/immutable/TreeMap.scala
@@ -101,8 +101,8 @@ class TreeMap[A, +B] private (tree: RB.Tree[A, B])(implicit val ordering: Orderi
     else new TreeMap(RB.slice(tree, from, until))
   }
 
-  override def dropRight(n: Int) = take(size - n)
-  override def takeRight(n: Int) = drop(size - n)
+  override def dropRight(n: Int) = take(size - math.max(n, 0))
+  override def takeRight(n: Int) = drop(size - math.max(n, 0))
   override def splitAt(n: Int) = (take(n), drop(n))
 
   private[this] def countWhile(p: ((A, B)) => Boolean): Int = {
diff --git a/src/library/scala/collection/immutable/TreeSet.scala b/src/library/scala/collection/immutable/TreeSet.scala
@@ -87,8 +87,8 @@ class TreeSet[A] private (tree: RB.Tree[A, Unit])(implicit val ordering: Orderin
     else newSet(RB.slice(tree, from, until))
   }
 
-  override def dropRight(n: Int) = take(size - n)
-  override def takeRight(n: Int) = drop(size - n)
+  override def dropRight(n: Int) = take(size - math.max(n, 0))
+  override def takeRight(n: Int) = drop(size - math.max(n, 0))
   override def splitAt(n: Int) = (take(n), drop(n))
 
   private[this] def countWhile(p: A => Boolean): Int = {
diff --git a/test/junit/scala/collection/IndexedSeqOptimizedTest.scala b/test/junit/scala/collection/IndexedSeqOptimizedTest.scala
@@ -13,4 +13,17 @@ class IndexedSeqOptimizedTest {
     assertEquals(0, (Array(2): collection.mutable.WrappedArray[Int]).lastIndexWhere(_ => true, 1))
     assertEquals(2, "abc123".lastIndexWhere(_.isLetter, 6))
   }
+
+  @Test
+  def hasCorrectDropAndTakeMethods() {
+    assertEquals("", "abc" take Int.MinValue)
+    assertEquals("", "abc" takeRight Int.MinValue)
+    assertEquals("abc", "abc" drop Int.MinValue)
+    assertEquals("abc", "abc" dropRight Int.MinValue)
+
+    assertArrayEquals(Array.empty[Int], Array(1, 2, 3) take Int.MinValue)
+    assertArrayEquals(Array.empty[Int], Array(1, 2, 3) takeRight Int.MinValue)
+    assertArrayEquals(Array(1, 2, 3), Array(1, 2, 3) drop Int.MinValue)
+    assertArrayEquals(Array(1, 2, 3), Array(1, 2, 3) dropRight Int.MinValue)
+  }
 }
diff --git a/test/junit/scala/collection/IterableViewLikeTest.scala b/test/junit/scala/collection/IterableViewLikeTest.scala
@@ -0,0 +1,20 @@
+package scala.collection
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(classOf[JUnit4])
+class IterableViewLikeTest {
+
+  @Test
+  def hasCorrectDropAndTakeMethods() {
+    val iter = Iterable(1, 2, 3)
+
+    assertEquals(Iterable.empty[Int], iter.view take Int.MinValue force)
+    assertEquals(Iterable.empty[Int], iter.view takeRight Int.MinValue force)
+    assertEquals(iter, iter.view drop Int.MinValue force)
+    assertEquals(iter, iter.view dropRight Int.MinValue force)
+  }
+}
diff --git a/test/junit/scala/collection/immutable/TreeMapTest.scala b/test/junit/scala/collection/immutable/TreeMapTest.scala
@@ -0,0 +1,20 @@
+package scala.collection.immutable
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(classOf[JUnit4])
+class TreeMapTest {
+
+  @Test
+  def hasCorrectDropAndTakeMethods() {
+    val tree = TreeMap(1 -> "a", 2 -> "b", 3 -> "c")
+
+    assertEquals(TreeMap.empty[Int, String], tree take Int.MinValue)
+    assertEquals(TreeMap.empty[Int, String], tree takeRight Int.MinValue)
+    assertEquals(tree, tree drop Int.MinValue)
+    assertEquals(tree, tree dropRight Int.MinValue)
+  }
+}
diff --git a/test/junit/scala/collection/immutable/TreeSetTest.scala b/test/junit/scala/collection/immutable/TreeSetTest.scala
@@ -0,0 +1,20 @@
+package scala.collection.immutable
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(classOf[JUnit4])
+class TreeSetTest {
+
+  @Test
+  def hasCorrectDropAndTakeMethods() {
+    val set = TreeSet(1, 2, 3)
+
+    assertEquals(TreeSet.empty[Int], set take Int.MinValue)
+    assertEquals(TreeSet.empty[Int], set takeRight Int.MinValue)
+    assertEquals(set, set drop Int.MinValue)
+    assertEquals(set, set dropRight Int.MinValue)
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -101,8 +101,8 @@ class TreeMap[A, +B] private (tree: RB.Tree[A, B])(implicit val ordering: Orderi`
`101`	`101`	`else new TreeMap(RB.slice(tree, from, until))`
`102`	`102`	`}`
`103`	`103`
`104`		`- override def dropRight(n: Int) = take(size - n)`
`105`		`- override def takeRight(n: Int) = drop(size - n)`
	`104`	`+ override def dropRight(n: Int) = take(size - math.max(n, 0))`
	`105`	`+ override def takeRight(n: Int) = drop(size - math.max(n, 0))`
`106`	`106`	`override def splitAt(n: Int) = (take(n), drop(n))`
`107`	`107`
`108`	`108`	`private[this] def countWhile(p: ((A, B)) => Boolean): Int = {`
Original file line number	Diff line number	Diff line change
`@@ -87,8 +87,8 @@ class TreeSet[A] private (tree: RB.Tree[A, Unit])(implicit val ordering: Orderin`
`87`	`87`	`else newSet(RB.slice(tree, from, until))`
`88`	`88`	`}`
`89`	`89`
`90`		`- override def dropRight(n: Int) = take(size - n)`
`91`		`- override def takeRight(n: Int) = drop(size - n)`
	`90`	`+ override def dropRight(n: Int) = take(size - math.max(n, 0))`
	`91`	`+ override def takeRight(n: Int) = drop(size - math.max(n, 0))`
`92`	`92`	`override def splitAt(n: Int) = (take(n), drop(n))`
`93`	`93`
`94`	`94`	`private[this] def countWhile(p: A => Boolean): Int = {`