Have the size parameter really put a hard limit on the size of genera…

…ted recursive structures

core/shared/src/main/scala/org/scalacheck/derive/MkArbitrary.scala

@@ -111,21 +111,32 @@ object MkHListArbitrary {
    ): MkHListArbitrary[H :: T] =
     instance(
       Arbitrary {
-        Gen.sized { size =>
-          val sig = math.signum(size)
-          val remainder = sig * size % (n() + 1)
-          val fromRemainderGen =
-            if (remainder > 0)
-              Gen.choose(1, n()).map(r => if (r <= remainder) sig else 0)
-            else
-              Gen.const(0)
-
-          for {
-            fromRemainder <- fromRemainderGen
-            headSize = size / (n() + 1) + fromRemainder
-            head <- Gen.resize(headSize, Gen.lzy(headArbitrary.value.arbitrary))
-            tail <- Gen.resize(size - headSize, Gen.lzy(tailArbitrary.arbitrary.arbitrary))
-          } yield head :: tail
+        Gen.sized { size0 =>
+          if (size0 < 0)
+            // unlike positive values, don't split negative sizes any further, and let subsequent Gen handle them
+            for {
+              head <- Gen.resize(size0, Gen.lzy(headArbitrary.value.arbitrary))
+              tail <- Gen.resize(size0, Gen.lzy(tailArbitrary.arbitrary.arbitrary))
+            } yield head :: tail
+          else {
+            // take a fraction of approximately 1 / (n + 1) from size for the head, leave the
+            // remaining for the tail
+
+            val size = size0 max 0
+            val remainder = size % (n() + 1)
+            val fromRemainderGen =
+              if (remainder > 0)
+                Gen.choose(1, n()).map(r => if (r <= remainder) 1 else 0)
+              else
+                Gen.const(0)
+
+            for {
+              fromRemainder <- fromRemainderGen
+              headSize = size / (n() + 1) + fromRemainder
+              head <- Gen.resize(headSize, Gen.lzy(headArbitrary.value.arbitrary))
+              tail <- Gen.resize(size - headSize, Gen.lzy(tailArbitrary.arbitrary.arbitrary))
+            } yield head :: tail
+          }
         }
       }
     )

test/shared/src/main/scala/org/scalacheck/SizeTestsDefinitions.scala

@@ -0,0 +1,35 @@
+package org.scalacheck
+
+object SizeTestsDefinitions {
+  // see https://github.com/rickynils/scalacheck/issues/305
+  sealed trait Tree {
+    def depth: Int = {
+
+      var max = 0
+      val m = new scala.collection.mutable.Queue[(Int, Branch)]
+
+      def handle(t: Tree, s: Int) =
+        t match {
+          case Leaf => max = max max s
+          case b: Branch =>
+            m += (s + 1) -> b
+        }
+
+      handle(this, 0)
+
+      while (m.nonEmpty) {
+        val (s, b) = m.dequeue
+        handle(b.left, s)
+        handle(b.right, s)
+      }
+
+      max
+    }
+  }
+  case object Leaf extends Tree
+  case class Branch(left: Tree, right: Tree) extends Tree
+
+  object Tree {
+    implicit val recursive = derive.Recursive[Tree](Gen.const(Leaf))
+  }
+}

test/shared/src/test/scala/org/scalacheck/SizeTests.scala

@@ -0,0 +1,54 @@
+package org.scalacheck
+
+import utest._
+
+import org.scalacheck.rng.Seed
+
+object SizeTests0 {
+  import org.scalacheck.Shapeless._
+
+  import SizeTestsDefinitions._
+
+  val arbTree = Arbitrary(Arbitrary.arbitrary[Tree])
+
+}
+
+object SizeTests extends TestSuite {
+
+  import SizeTestsDefinitions._
+
+  assert(Leaf.depth == 0)
+  assert(Branch(Leaf, Leaf).depth == 1)
+  assert(Branch(Branch(Leaf, Leaf), Leaf).depth == 2)
+
+
+  def stream[T: Arbitrary](p: Gen.Parameters, seed: rng.Seed): Stream[Option[T]] = {
+    val r = Arbitrary.arbitrary[T].doPureApply(p, seed)
+    r.retrieve #:: stream[T](p, r.seed)
+  }
+
+  // manually calculated, grows approx. like log(size)
+  val maxDepths = Seq(
+    10 -> 5,
+    100 -> 8,
+    300 -> 10
+  )
+
+  val tests = TestSuite {
+    'tree - {
+      val seed = Seed.random()
+      val inspect = 10000
+
+      for ((size, expectedMaxDepth) <- maxDepths) {
+        val maxDepth = stream[Tree](Gen.Parameters.default.withSize(size), seed)(SizeTests0.arbTree)
+          .map(_.get) // the corresponding generator doesn't fail thanks to the implicit derive.Recursive[Tree] in Tree's companion
+          .map(_.depth)
+          .take(inspect)
+          .max
+
+        assert(maxDepth == expectedMaxDepth)
+      }
+    }
+  }
+
+}