jvm Future instances do not obey laws

[ceedubs]

Most of the type class instances in https://github.com/non/cats/blob/master/jvm/src/main/scala/cats/jvm/std/future.scala (Comonad, Eq, PartialOrder, Order) are not checked against their laws. If we were to check them against their laws they would fail, because they throw exceptions in the case of a failed future.

Furthermore, it isn't necessarily obvious how an Eq[Future[A]] instance should behave. The current one throws an exception if either Future fails. Consider the following scenario:

val fa: Future[Int] = Future.failed(new RuntimeException("foo"))

fa === fa

This last line throws an exception. One could reasonably expect it to return true instead. This does require creating an explicit Eq instance (with a supplied timeout), so it takes a little effort to get yourself into trouble. Still, I think that these instances belong in alleycats or tests but not in cats core. In addition to being unlawful, they aren't especially useful. Other than in tests, you probably always want to get back a Future[Boolean] when comparing two Future[A] values instead of blocking the thread waiting for both futures to complete.

[non]

Let's move all the unlawful stuff to alleycats (and also put an Eq[Future[A]] in tests).

[tpolecat]

I think there's kind of a lot of subtlety involved with Future due to the way it's used in practice. But in principle I don't think any of this is a problem. Future has nothing to say about side-effects so it's really only fair to test it with referentially transparent expressions. Nontermination doesn't manifest until you force it, as with Eval.Later ... fa === fa above should not have an answer.

I think the real problem is that everyone uses Future to side-effect and nobody ever uses Await.result to get the value out, so treating it like a well-behaved data type is kind of disingenuous. I'm all in favor of banishing it to alleycats or maybe litterbox.

[ceedubs]

@non @inthenow I went to move these instances to alleycats, but it doesn't seem to have quite the same setup for JVM/JS that this project does. I'm not quite sure where this should go over there. Any pointers?

[non]

I think we want to mirror Cats' setup in Alleycats, but haven't had the time to do it yet.

Specifically, I imagine having alleycats.js and alleycats.jvm.

[ghost]

The JS version of future started out in alleycats so no surprise to me that it's all going there,

I think we want to mirror Cats' setup in Alleycats

sbt-catalysts is ready to roll - how about we set it up in alleycats first?

[non]

@inthenow That's a great idea. Let's do it! 🐈 🔬

[ghost]

👍 (Sunday)

[ceedubs]

@inthenow I see that non/alleycats#21 has been merged. Does that mean that alleycats is ready for this now?

[ghost]

@ceedubs I think we need non/alleycats#22 in first, but otherwise yes.