diff --git a/src/library/scala/collection/View.scala b/src/library/scala/collection/View.scala index c84c126626f6..441790c3c6e5 100644 --- a/src/library/scala/collection/View.scala +++ b/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 } diff --git a/test/junit/scala/collection/ViewTest.scala b/test/junit/scala/collection/ViewTest.scala index 89418aa6a024..cb5814654e37 100644 --- a/test/junit/scala/collection/ViewTest.scala +++ b/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()", 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) + } } diff --git a/test/scalacheck/scala/collection/ViewProperties.scala b/test/scalacheck/scala/collection/ViewProperties.scala new file mode 100644 index 000000000000..1814adc1c690 --- /dev/null +++ b/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") + } +}