Refactor OnceCanBuildFrom to check the collection type.

The builder is now instantiated as an iterator builder only
if a generic builder cannot be found on the collection that
requested the builder.

Reason - we want this behaviour:

scala> scala.util.Random.shuffle(List(1, 2, 3): collection.TraversableOnce[Int])
res0: scala.collection.TraversableOnce[Int] = List(3, 1, 2)

instead of this one:

scala> scala.util.Random.shuffle(List(1, 2, 3): collection.TraversableOnce[Int])
res0: scala.collection.TraversableOnce[Int] = non-empty iterator

which may lead to nasty surprises.

Alternately, to avoid pattern-matching in OnceCanBuildFrom.apply, we
could mix in GenericTraversableTemplate-related functionaly into
TraversableOnce, but this may become too complicated.

2 files changed, 16 insertions, 6 deletions

diff --git a/src/library/scala/collection/Iterator.scala b/src/library/scala/collection/Iterator.scala
index af83291a46..3b2a7c7820 100644
--- a/src/library/scala/collection/Iterator.scala
+++ b/src/library/scala/collection/Iterator.scala
@@ -26,7 +26,8 @@ object Iterator {
    *  See `scala.util.Random.shuffle` for an example.
    */
   implicit def IteratorCanBuildFrom[A] = new TraversableOnce.BufferedCanBuildFrom[A, Iterator] {
-    def toColl[B](coll: ArrayBuffer[B]) = coll.iterator
+    def bufferToColl[B](coll: ArrayBuffer[B]) = coll.iterator
+    def traversableToColl[B](t: GenTraversable[B]) = t.toIterator
   }
 
   /** The iterator which produces no values. */
diff --git a/src/library/scala/collection/TraversableOnce.scala b/src/library/scala/collection/TraversableOnce.scala
index b34daed2be..0334322dbc 100644
--- a/src/library/scala/collection/TraversableOnce.scala
+++ b/src/library/scala/collection/TraversableOnce.scala
@@ -367,16 +367,23 @@ object TraversableOnce {
   implicit def flattenTraversableOnce[A, CC[_]](travs: TraversableOnce[CC[A]])(implicit ev: CC[A] => TraversableOnce[A]) =
     new FlattenOps[A](travs map ev)
   
-  abstract class BufferedCanBuildFrom[A, Coll[X] <: TraversableOnce[X]] extends generic.CanBuildFrom[Coll[_], A, Coll[A]] {
-    def toColl[B](buff: ArrayBuffer[B]): Coll[B]
+  /* Functionality reused in Iterator.CanBuildFrom */
+  private[collection] abstract class BufferedCanBuildFrom[A, Coll[X] <: TraversableOnce[X]] extends generic.CanBuildFrom[Coll[_], A, Coll[A]] {
+    def bufferToColl[B](buff: ArrayBuffer[B]): Coll[B]
+    def traversableToColl[B](t: GenTraversable[B]): Coll[B]
     
-    def newIterator: Builder[A, Coll[A]] = new ArrayBuffer[A] mapResult toColl
+    def newIterator: Builder[A, Coll[A]] = new ArrayBuffer[A] mapResult bufferToColl
 
     /** Creates a new builder on request of a collection.
      *  @param from  the collection requesting the builder to be created.
      *  @return the result of invoking the `genericBuilder` method on `from`.
      */
-    def apply(from: Coll[_]): Builder[A, Coll[A]] = newIterator
+    def apply(from: Coll[_]): Builder[A, Coll[A]] = from match {
+      case xs: generic.GenericTraversableTemplate[_, _] => xs.genericBuilder mapResult {
+        case res => traversableToColl(res.asInstanceOf[GenTraversable[A]])
+      }
+      case _ => newIterator
+    }
 
     /** Creates a new builder from scratch
      *  @return the result of invoking the `newBuilder` method of this factory.
@@ -389,9 +396,11 @@ object TraversableOnce {
    *  See `scala.util.Random.shuffle` or `scala.concurrent.Future.sequence` for an example.
    */
   class OnceCanBuildFrom[A] extends BufferedCanBuildFrom[A, TraversableOnce] {
-    def toColl[B](buff: ArrayBuffer[B]) = buff.iterator
+    def bufferToColl[B](buff: ArrayBuffer[B]) = buff.iterator
+    def traversableToColl[B](t: GenTraversable[B]) = t.seq
   }
   
+  /** Evidence for building collections from `TraversableOnce` collections */
   implicit def OnceCanBuildFrom[A] = new OnceCanBuildFrom[A]
   
   class FlattenOps[A](travs: TraversableOnce[TraversableOnce[A]]) {