Fix non-terminating shrink streams.

src/main/scala/org/scalacheck/Shrink.scala

@@ -226,38 +226,56 @@ object Shrink extends ShrinkLowPriority {
 }
 
 final class ShrinkIntegral[T](implicit ev: Integral[T]) extends Shrink[T] {
-  import ev.{ equiv, fromInt, zero, minus, times, quot }
-  val minusOne = fromInt(-1)
+  import ev.{ fromInt, lt, gteq, quot, negate, equiv, zero, one }
+
   val two = fromInt(2)
 
-  // assumes x is non-zero
+  // see if T supports negative values or not. this makes some
+  // assumptions about how Integral[T] is defined, which work for
+  // Integral[Char] at least. we can't be sure user-defined
+  // Integral[T] instances will be reasonable.
+  val skipNegation = gteq(negate(one), one)
+
+  // assumes x is non-zero.
   private def halves(x: T): Stream[T] = {
     val q = quot(x, two)
     if (equiv(q, zero)) Stream(zero)
-    else q #:: times(q, minusOne) #:: halves(q)
+    else if (skipNegation) q #:: halves(q)
+    else q #:: negate(q) #:: halves(q)
   }
 
   def shrink(x: T): Stream[T] =
     if (equiv(x, zero)) Stream.empty[T] else halves(x)
 }
 
 final class ShrinkFractional[T](implicit ev: Fractional[T]) extends Shrink[T] {
-  import ev.{ fromInt, abs, zero, one, minus, times, div, lt }
+  import ev.{ fromInt, abs, zero, one, div, lteq, negate, lt }
 
-  val minusOne = fromInt(-1)
   val two = fromInt(2)
-  val hundredK = fromInt(100000)
 
-  def closeToZero(x: T): Boolean =
-    lt(abs(times(x, hundredK)), one)
+  // this makes some assumptions, namely that we can support 1e-5 as a
+  // fractional value. fortunately, if that is not true, it's likely
+  // that our divisions will converge to zero quickly enough anyway.
+  val small = div(one, fromInt(100000))
+  def closeToZero(x: T): Boolean = lteq(abs(x), small)
 
   // assumes x is not close to zero
   private def halves(x: T): Stream[T] = {
     val q = div(x, two)
     if (closeToZero(q)) Stream(zero)
-    else q #:: times(q, minusOne) #:: halves(q)
+    else q #:: negate(q) #:: halves(q)
+  }
+
+  // this catches things like NaN and Infinity. it's not clear that we
+  // should be shrinking those values, since they are sentinels.
+  def isUnusual(x0: T): Boolean = {
+    val x = abs(x0)
+    // the negation here is important -- if we wrote this as gteq(...)
+    // then it would not handle the NaN case correctly.
+    !lt(div(x, two), x)
   }
 
   def shrink(x: T): Stream[T] =
-    if (closeToZero(x)) Stream.empty[T] else halves(x)
+    if (closeToZero(x) || isUnusual(x)) Stream.empty[T]
+    else halves(x)
 }

src/test/scala/org/scalacheck/ShrinkSpecification.scala

@@ -9,7 +9,7 @@
 
 package org.scalacheck
 
-import Prop.{forAll, BooleanOperators}
+import Prop.{forAll, forAllNoShrink, BooleanOperators}
 import Shrink.shrink
 
 object ShrinkSpecification extends Properties("Shrink") {
@@ -83,4 +83,59 @@ object ShrinkSpecification extends Properties("Shrink") {
     val e: Either[Long, Int] = Right(i)
     shrink(e).forall(_.isRight)
   }
+
+  /* Ensure that shrink[T] terminates. (#244)
+   *
+   * Let's say shrinking "terminates" when the stream of values
+   * becomes empty. We can empirically determine the longest possible
+   * sequence for a given type before termination. (Usually this
+   * involves using the type's MinValue.)
+   *
+   * For example, shrink(Byte.MinValue).toList gives us 15 values:
+   *
+   *   List(-64, 64, -32, 32, -16, 16, -8, 8, -4, 4, -2, 2, -1, 1, 0)
+   *
+   * Similarly, shrink(Double.MinValue).size gives us 2081.
+   */
+
+  property("Shrink[Byte] terminates") =
+    forAllNoShrink((n: Byte) => Shrink.shrink(n).drop(15).isEmpty)
+
+  property("Shrink[Char] terminates") =
+    forAllNoShrink((n: Char) => Shrink.shrink(n).drop(16).isEmpty)
+
+  property("Shrink[Short] terminates") =
+    forAllNoShrink((n: Short) => Shrink.shrink(n).drop(31).isEmpty)
+
+  property("Shrink[Int] terminates") =
+    forAllNoShrink((n: Int) => Shrink.shrink(n).drop(63).isEmpty)
+
+  property("Shrink[Long] terminates") =
+    forAllNoShrink((n: Long) => Shrink.shrink(n).drop(127).isEmpty)
+
+  property("Shrink[Float] terminates") =
+    forAllNoShrink((n: Float) => Shrink.shrink(n).drop(2081).isEmpty)
+
+  property("Shrink[Double] terminates") =
+    forAllNoShrink((n: Double) => Shrink.shrink(n).drop(2081).isEmpty)
+
+  // make sure we handle sentinel values appropriately for Float/Double.
+
+  property("Shrink[Float] handles PositiveInfinity") =
+    Prop(Shrink.shrink(Float.PositiveInfinity).isEmpty)
+
+  property("Shrink[Float] handles NegativeInfinity") =
+    Prop(Shrink.shrink(Float.NegativeInfinity).isEmpty)
+
+  property("Shrink[Float] handles NaN") =
+    Prop(Shrink.shrink(Float.NaN).isEmpty)
+
+  property("Shrink[Double] handles PositiveInfinity") =
+    Prop(Shrink.shrink(Double.PositiveInfinity).isEmpty)
+
+  property("Shrink[Double] handles NegativeInfinity") =
+    Prop(Shrink.shrink(Double.NegativeInfinity).isEmpty)
+
+  property("Shrink[Double] handles NaN") =
+    Prop(Shrink.shrink(Double.NaN).isEmpty)
 }