Add Nullable type to cats data with instances#4823
Conversation
| import scala.language.strictEquality | ||
| import scala.quoted.* | ||
|
|
||
| opaque type Nullable[+A] = A | Null |
There was a problem hiding this comment.
we could make Nullable[+A] >: A to allow passing an A where a Nullable is being accepted. That said, you would be adding a bunch of no-op if checks to your code by doing so, and I tend to think the cost of calling Nullable(a) for such cases isn't a bad idea.
Maximum convenience isn't the goal here. Maximum correctness with zero-cost is my goal.
| given [A](using A: Arbitrary[A]): Arbitrary[Nullable[A]] = | ||
| Arbitrary(Gen.oneOf(Gen.const(Nullable.empty[A]), A.arbitrary.map(Nullable(_)))) | ||
|
|
||
| given [A](using A: Cogen[A]): Cogen[Nullable[A]] = | ||
| Cogen[Option[A]].contramap(_.toOption) |
There was a problem hiding this comment.
Ideally, these instances should be defined in cats-laws module, object cats.laws.discipline.arbitrary.
However, since it is for Scala 3 only, it may require adding some tweaks to the file structure in that module.
| } | ||
| } | ||
|
|
||
| test("option-like Applicative is not lawful for nested Nullable") { |
There was a problem hiding this comment.
I hate to break it to you, but Functor[Nullable] seems to be unlawful for nested Nullable too:
// These cases all pass:
checkAll("Option[Option[*]]", FunctorTests[Option].functor[Option[Int], Option[Int], Option[Int]])
checkAll("Option[Nullable[*]]", FunctorTests[Option].functor[Nullable[Int], Nullable[Int], Nullable[Int]])
checkAll("Nullable[Option[*]]", FunctorTests[Nullable].functor[Option[Int], Option[Int], Option[Int]])
// But this one fails:
checkAll("Nullable[Nullable[*]]", FunctorTests[Nullable].functor[Nullable[Int], Nullable[Int], Nullable[Int]])The latter fails on "invariant composition" and "covariant composition".
There was a problem hiding this comment.
No need to "hate to break it" this is good news. This is what code review is for.
In fact, it suggests our law tests are too weak. It might be good to test all F[_] types on [X] =>> F[F[X]] when they compose (like Functor, Applicative, Traverse, etc...)
I'll remove the functor based ones, but I think that really leaves only Foldable, Hash, Ordered, etc...
I'll double check the nested Foldable as well and see if it fails.
There was a problem hiding this comment.
In fact, it suggests our law tests are too weak.
Agree, I see that many typeclasses declare compose* methods (up to Applicative because Monad won't compose with itself, I guess). But it seems they aren’t backed by a sufficient set of laws.
|
@satorg please take a look. I think it's safe now. One question: should we add a method like transform, like transformFlat? You can implement it with fold. It's not a lawful flatMap but still useful. Honestly I wonder if we should just call it map and flatMap so we can use for-comprehensions. We aren't providing lawless instances so maybe that would be okay? |
| type Flattened[A] = A match { | ||
| case Nullable[x] => Nullable[x] | ||
| case _ => Nullable[A] | ||
| } |
There was a problem hiding this comment.
This type doesn't seem to be used anywhere. Is that intentional?
|
@johnynek you might forget to run |
|
Sorry this wasn't quite ready for a push (the last bit with the attempts a better flattening. Still not happy with it). |
2 participants
closes #4822
The idea is to be able to use nullable types from java (or even scala for unboxed optionality) but do so fully safely, so we can't confuse the nullable type with a non-nullable type.
Note: as suspected in #4822 both Monad and Applicative are not lawful. So instances were not added (someone could add them to alleycats, but I'm not motivated to add non-lawful instances).
For those cases, the user should be able to use
.foldwhich is as efficient as a hand written if/else with casting and can express all the operations you want to do.The functor methods are implemented which could be more efficiently done than with map and we test against the default implementations.