Allow opting out of serializable checks

[ceedubs]

Many laws (such as the group laws) force a serializability check. I understand that this can be helpful for making sure your type class instances are Spark/Scalding/etc friendly. However, it would be nice if there were a way to opt-out of this (or even make it opt-in), since it's not always possible to create a serializable instance.

For example, Future[A] has a law-abiding Monoid instance as long as A does, but currently we can't run the monoid laws on it, because the instance captures an ExecutionContext, which is not Serializable.

[johnynek]

I was the one that pushed for this. I can see your point. There are actually two questions:

1. Do we remove the extends Serializable
2. Do we remove the test?

Without the marker serializable, Java won't even try to serialize, but even if you mark something as serializable it may fail. Both of these failures generally happen at runtime and almost never at compile time.

I'm 100% okay with an option to opt out in the test.

Even though it is a lie, I would probably prefer to keep the extends on the traits because of how easy it is to forget it and how frustrating it is to have to publish a new jar just to add an extends to a dependency.

Serialization fundamentally has runtime failure possibility, so I'm okay with unusual cases causing exceptions.

[non]

I pretty much agree with @johnynek. I think instances should default to serializable. Future is already a bit of a black sheep in some ways, so I think opting-out of the serializability test for it makes the most sense.

[ceedubs]

I'm okay with the proposed plan. Since you get runtime as opposed to compile-time failure if you try to serialize an unserializable class whether or not it's marked with Serializable, I don't see a lot of point in worrying about whether a Serializable trait is inherited.

So I guess then the question becomes how will it look to have tests where you can opt out of serializability? A flag passed to the test constructor/method?

[johnynek]

I can think of a few ways:

1. a flag which defaults to checking (hopefully not a boolean, but for instance a simple ADT).
2. a trait to "unmark" serializability. If the item extends that trait, we don't check it. A little weird to extend serializable, and then be unserializable, but this approach allows others to possibly check that you opted out.
3. a typeclass approach for the above. The default is that there is no instance for NotSerializable[T] when T <: Serializable, but in the companion object your your method you could do this. Problem with this is that we usually use subtyping for typeclasses, and you lose the exact type. I guess this approach is not so great.

[ceedubs]

Thank you @johnynek. To me number 1 sounds the most appealing.

You mentioned a problem with number 3. If I understand number 2 correct, a couple shortcomings I see are:

1. It forces us into changing the types on our real-world code for testing purposes (extends the marker trait).
2. This kind of follows from the previous point, but ideally I could test an instance that is being provided by a 3rd party lib without changing the source to extend a marker trait.

The first option might result in a bit more boiler-plate in the tests, but hopefully it won't be too bad.

[sritchie]

(Rusty Scala coder getting back into the game, here.)

Here's a pattern we've used before for 1: https://github.com/non/algebra/pull/129

Happy to thread this, or something like this, if there's a better way to implement this pattern these days, through the rest of the callsites. Lots of methods on GroupLaws, LatticeLaws, OrderLaws and RingLaws all need extra implicits, but that should take care of it. Users can add implicit val check = SkipSerialization inside tests that don't want this behavior.

[sritchie]

Here's another attempt using dynamic variables: https://github.com/non/algebra/pull/141 Let me know what you guys think.