Fix unreported bug in View.Patched

Fix unreported bug in `View.Patched` where iterator
is incorrect due to the patch only being iterable once,
and already having been exhausted.

Discovered while attempting to optimise `ArrayBuffer` in an earlier
version of the previous commit.

Simplify `ArrayBuffer#insertAll` code.

src/library/scala/collection/View.scala

@@ -404,8 +404,14 @@ object View extends IterableFactory[View] {
 
   @SerialVersionUID(3L)
   private[collection] class Patched[A](underlying: SomeIterableOps[A], from: Int, other: IterableOnce[A], replaced: Int) extends AbstractView[A] {
-    def iterator: Iterator[A] = underlying.iterator.patch(from, other.iterator, replaced)
-    override def knownSize: Int = if (underlying.knownSize == 0 && other.knownSize == 0) 0 else super.knownSize
+    // we may be unable to traverse `other` more than once, so we need to cache it if that's the case
+    private val _other: Iterable[A] = other match {
+      case other: Iterable[A] => other
+      case other              => LazyList.from(other)
+    }
+
+    def iterator: Iterator[A] = underlying.iterator.patch(from, _other.iterator, replaced)
+    override def knownSize: Int = if (underlying.knownSize == 0 && _other.knownSize == 0) 0 else super.knownSize
     override def isEmpty: Boolean = if (knownSize == 0) true else iterator.isEmpty
   }
 

test/junit/scala/collection/ViewTest.scala

@@ -1,10 +1,10 @@
 package scala.collection
 
-import scala.collection.immutable.List
 import org.junit.Assert._
 import org.junit.Test
 
 import scala.collection.mutable.{ArrayBuffer, ListBuffer}
+import scala.tools.testkit.AssertUtil.assertSameElements
 
 class ViewTest {
 
@@ -113,4 +113,21 @@ class ViewTest {
   def _toString(): Unit = {
     assertEquals("View(<not computed>)", View(1, 2, 3).toString)
   }
+
+  // see scala/scala#9388
+  @Test
+  def patch(): Unit = {
+    // test re-iterability
+    val v1 = List(2).view.patch(1, List(3, 4, 5).iterator, 0)
+    assertSameElements(Seq(2, 3, 4, 5), v1.toList)
+    assertSameElements(Seq(2, 3, 4, 5), v1.toList) // check that it works twice
+
+    // https://github.com/scala/scala/pull/9388#discussion_r709392221
+    val v2 = List(2).view.patch(1, Nil, 0)
+    assert(!v2.isEmpty)
+
+    // https://github.com/scala/scala/pull/9388#discussion_r709481748
+    val v3 = Nil.view.patch(0, List(1).iterator, 0)
+    assert(v3.knownSize != 0)
+  }
 }

test/scalacheck/scala/collection/ViewProperties.scala

@@ -0,0 +1,57 @@
+/*
+ * Scala (https://www.scala-lang.org)
+ *
+ * Copyright EPFL and Lightbend, Inc.
+ *
+ * Licensed under Apache License 2.0
+ * (http://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package scala.collection
+
+import org.scalacheck._
+import org.scalacheck.Prop._
+
+import scala.collection.mutable.ListBuffer
+
+object ViewProperties extends Properties("View") {
+
+  type Elem = Int
+  type SomeSeqOps = SeqOps[Elem, Iterable, Iterable[Elem]]
+
+  private def expectedPatch(seq: SomeSeqOps, from: Int, other: Iterable[Elem], replaced: Int): Seq[Elem] = {
+    val (prefix, suffix) = seq.splitAt(from)
+    ListBuffer.empty[Elem] ++= prefix ++= other ++= suffix.drop(replaced)
+  }
+
+  property("`SeqOps#patch(...)` (i.e. `iterableFactory.from(View.Patched(...))`) correctness") = {
+    // we use `mutable.ArraySeq` because it uses the default `patch`
+    // implementation, rather than one from `StrictOptimisedSeqOps`
+    forAll { (seq: mutable.ArraySeq[Elem], from: Int, other: Iterable[Elem], replaced: Int) =>
+      val expected = expectedPatch(seq, from, other, replaced)
+      val patchedWithIterable = seq.patch(from, other, replaced)
+      val patchedWithIterableOnce = seq.patch(from, other.iterator, replaced)
+
+      // we don't need to use `sameElements` like below, because
+      // both `expected` and patched are `Seq` this time
+      ((expected =? patchedWithIterable) :| "`patch(_, Iterable, _)` is performed correctly") &&
+        ((expected =? patchedWithIterableOnce) :| "`patch(_, IterableOnce, _)` is performed correctly")
+    }
+  }
+
+
+  property("`SeqOps#view.patch(...)` (i.e. `View.Patched` used directly) correctness and consistency") =
+    forAll { (seq: Seq[Elem], from: Int, other: Iterable[Elem], replaced: Int) =>
+      val expected = expectedPatch(seq, from, other, replaced)
+      val patchedWithIterable = seq.view.patch(from, other, replaced)
+      val patchedWithIterableOnce = seq.view.patch(from, other.iterator, replaced)
+
+      (expected.sameElements(patchedWithIterable) :| "`patch(_, Iterable, _)` is performed correctly") &&
+        (expected.sameElements(patchedWithIterable) :| "`view.patch(_, Iterable, _)` remains the same after multiple iterations") &&
+        (expected.sameElements(patchedWithIterableOnce) :| "`patch(_, IterableOnce, _)` is performed correctly") &&
+        (expected.sameElements(patchedWithIterableOnce) :| "`view.patch(_, IterableOnce, _)` remains the same after multiple iterations")
+    }
+}