Overloaded Prop.collect behaves surprisingly with Seq[T] argument

[pford19]

Summary

Prop.collect is overloaded. There is a "simple" version

• collect(T)(Prop): Prop

and a "functional" version

• collect(T=>P)(implicit P=>Prop): T=>Prop

Using collect in properties based on generators that produce Seq[T] values (for example Gen.pick) can lead to potentially surprising behavior. In the example detailed below, the compiler selects the functional version over the simple version when the argument is of type Seq[T] with a Type mismatch compiler error.

This function version selection occurs because Seq[T] extends PartialFunction[Int, T] which matches (in part) the signature collect(Int => T)(implicit T => Prop).

Once the issue is understood, there are straightforward workarounds.

Improved documentation in the source file for the two collect methods seems a straightforward way to mitigate potentially complex and frustrating diagnosis by your typical developer (assuming she gets as far a clicking through to the Scalacheck sources, or pulling up the equivalent of Intellij's quick documentation).

Removing the overload completely by renaming the functional version is a more ambitious solution.

Details

I was using collect in a property along these lines

val prop = forAll(Gen.pick(2, (0 to 9)) { ns => ns.size == 2 }

which I then augmented with collect

val prop = forAll(Gen.pick(2, (0 to 9)) { ns => collect(ns) { ns.size == 2 }}

At which point I get this mysterious (to me, at the time) compiler error

Type mismatch: expected: Int => Prop, actual: Boolean

Eventually I figured out what was going on, but it took a while, mostly because my eyes start to glaze over when I end up looking at a declaration like:

def collect[T, P](f: T => P)(implicit ev: P => Prop): T => Prop = ...

The problem is that pick is type Gen[Seq[T]], and Seq[T] extends PartialFunction[Int,T]. This explains the compiler's decision to use collect(Int=>T) instead of collect(Seq[T]). (At least in part, I wonder why it doesn't complain about ambiguous overloading. But that is a topic for another day.)

Workarounds are relatively simple, such as

• Cast to Iterable (which is not a function type)
  val prop = forAll(Gen.pick(2, (0 to 9)) { ns => collect(ns.toIterable) { ns.size == 2 }}

• Convert to String
val prop = forAll(Gen.pick(2, (0 to 9)) { ns => collect(ns.toString) { ns.size == 2 }}

• Rewrite to intentionally use functional collect(Seq[Int] => Boolean)
val prop = forAll(gen)(collect{ns: Seq[Int] => ns.size == 2})

So the issue is ergonomic, not functional. Surprising and unexpected behavior like this can be frustrating and a time waster.

In my limited experience as a Scalacheck user, my preference is for the collect(T) form over the collect(T=>P) form, in part because the former allows more manipulation of the collected values.

Suggestions

• Eliminate the overload by renaming collect(T=>P)(implicit P=>Prop)
• Provide more guidance in the source documentation. Current doc is skimpy:
  /** Collect data for presentation in test report */
  Documentation could be enhanced to note the potentially surprising behavior when collect's
  argument is a Seq. This would have saved me quite a bit of time, since Intellij certainly took me
  right to that line in Prop.scala.

[ashawley]

I know I experienced this trauma, but completely forgot about it. All I know is I always do ns.toString in Prop.collect to avoid the error. It feels great to know my enemy.

I don't use the Prop.collect(T=>P) version. There's no example of using it in ScalaCheck: The definitive guide.

I wonder how one would use it that way? Is such usage well-known? If proposed, I'd second the motion to rename or even drop it if it's not commonly used. I'd argue Prop.collect is too useful to not have this usability problem (or as @pford19 rightly calls "ergonomic") be resolved.

[ashawley]

I've proposed that the functional version of Prop.collect be deprecated in #449, since I'm skeptical it's even used.