Add attemptOption to Alternative#4862
Conversation
Implements the standard `optional` parser-combinator (Haskell's `Control.Applicative.optional`): lift a possibly-empty F[A] into an always-non-empty F[Option[A]] by combining `map(_.some)` with `pure(None)` via `combineK`. Added to the `Alternative` trait with a default implementation and to the simulacrum-style `Ops` trait so it is reachable via `import cats.syntax.alternative._`. Closes typelevel#2936. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
|
@MavenRain , thank you for the PR! I've got a few comments:
|
| test("attemptOption") { | ||
| assert(Alternative[Option].attemptOption(Option(5)) === Some(Some(5))) | ||
| assert(Alternative[Option].attemptOption(Option.empty[Int]) === Some(None)) | ||
| assert(Alternative[List].attemptOption(List(1, 2, 3)) === List(Some(1), Some(2), Some(3), None)) | ||
| assert(Alternative[List].attemptOption(List.empty[Int]) === List(None)) | ||
| } |
There was a problem hiding this comment.
This test doesn't seem providing any value because the following two property based tests should cover all these cases already.
| * }}} | ||
| */ | ||
| def attemptOption[A](fa: F[A]): F[Option[A]] = | ||
| combineK(map(fa)((a: A) => Some(a): Option[A]), pure(Option.empty[A])) |
There was a problem hiding this comment.
nit: I'd rather add:
private val fempty: F[Option[Nothing]] = pure(Option.empty[Nothing])
def attemptOption[A](fa: F[A]): F[Option[A]] =
combineK(map(fa)((a: A) => Some(a): Option[A]), widen[Option[A]](fempty))Since widen by default is a cast (and generally 0 cost), this prevents allocating the fempty on every call.
There was a problem hiding this comment.
Thanks @johnynek. I tried the cached fempty but had to back it out: a cached field (I used a lazy val) materializes on every concrete instance, and several NonEmptyAlternative instances are value classes, e.g. NonEmptySeq is final class ... extends AnyVal, so the extra field breaks their Serializable law tests. Instead I've expressed the method via appendK: on the instances that override appendK (List uses :+, Option short-circuits on isDefined) that avoids constructing pure(None) per call entirely, which gets at your allocation point without a cached field.
…rnative + add law
- Move `attemptOption` from `Alternative` to `NonEmptyAlternative`
(satorg typelevel#1). Implementation only needs `combineK + map + pure`.
- Use `widen(fempty)` with a cached `F[Option[Nothing]]` to avoid
allocating a fresh `pure(None)` on every call (johnynek nit).
- Add `nonEmptyAlternativeAttemptOptionConsistentWithCombineKAndPure`
to `NonEmptyAlternativeLaws` and wire it into the `nonEmptyAlternative`
rule set (satorg typelevel#3). This required adding `EqFOA: Eq[F[Option[A]]]`
to the `nonEmptyAlternative` and `alternative` rule sets' implicit
parameter lists.
- Drop the redundant point test in `AlternativeSuite` (satorg inline).
- Move the two `attemptOption` property tests to `NonEmptyAlternativeSuite`
to follow the method's new location.
- Add two `mima.sbt` exclusions for the test-helper signature change.
Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
scalafmtSbtCheck on the 2.12 / catsJS lane caught a long-line wrap that scalafmtCheckAll did not surface locally. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
The `private lazy val fempty: F[Option[Nothing]]` in NonEmptyAlternative materialised as a field on every concrete instance. For `NonEmptySeq` (a `final class … extends AnyVal`) the cached boxed value is not `java.io.Serializable`, which broke SerializableTests for every typeclass instance sharing catsDataInstancesForNonEmptySeqBinCompat1 (Bimonad, Align, NonEmptyAlternative, Semigroup-via-.algebra). Inline `pure(Option.empty[A])` directly; matches the example in the PR description and removes the regression. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
|
The only red check is the Scala Native job, which died with |
Good catch on the |
|
@satorg On |
Closes #2936.
Adds
attemptOption[A](fa: F[A]): F[Option[A]]toAlternative,implementing the standard
optionalparser-combinator (Haskell'sControl.Applicative.optional): tryfa, surface its result asSome, and combine withpure(None)so the result always succeeds atleast once.