[PROPOSAL] Effect as typealias

[nomisRev]

This proposal flattens Effect into a typealias, and renames EffectScope to Shift.

It replaces the existing error handlers with catch,
and due to the flattening of Effect the existing catch inside Shift is the equivalent of catch + bind.

To make the distinction between extension, and DSL method more clear I flagged it as a DslMarker.
Which makes DSL catch usage show up as a keyword. Similar to Route API (get, post, ...) of `Ktor.

Only downside, it requires imports for all Effect methods but since the API surface drastically decreased I don't see this as a problem.

The only APIs available are fold (+ connivence Arrow Core mappers) + catch.

Side-note: with context receivers we can in a future version of Arrow split Effect from Arrow Core, and move it to its own module (kernel).

API Design / Changes

• The Effect<E, A> type is now a typealias.
  This has as a benefit that there is no more intermediate type, or difference between a lambda of the same shape as Effect<E, A> and the type itself. This also removes the need for some cases of @UnsafeVariance, and variance. Downside is that the API has now become top-level and requires explicit imports.

• catch / attempt

  • catch also has an overload that only takes resolve: suspend Shift<E2>.(E) -> A, and simply ignores the Throwable.
  • attempt is a variant that cannot change E to E2, but it only requires a handler for Throwable. It thus takes recover: suspend Shift<E>.(Throwable) -> A instead of recover: suspend Shift<E2>.(Throwable) -> A.
  • attempt has an overload that refines Throwable to T : Throwable. If Throwable !is T than it rethrows the Throwable.
  • I removed the uber catch since it can be composed from catch and attempt by only requiring a single additional Continuation allocation which we can neglect.
    catch is the uber handler, Effect<E, A>.catch(recover: suspend Shift<E2>.(Throwable) -> A, resolve: suspend Shift<E2>.(E) -> A): Effect<E2, A>. It has the ability to change error type E to E2 whilst both handling Throwable, and E. We can see this as the most powerful error handler, that covers all use cases of the existing handlers and more.

TODO:

• Investigate EagerEffect

[kyay10]

If and when you make NullableShift, OptionShift, etc contextual functions instead, do note that IorShift can also be contextual by just taking in a context(Shift, Semigroup) (with whatever type arguments). NVM, leftState won't allow that to happen

[nomisRev]

@kyay10 removing OptionShift, NullableShift and ResultShift would be great but it seems context receivers will be out of scope for Arrow 2.0 since it’ll only become available in 1.9 or later.

So it’ll probably be for Arrow 3.0 in maybe 2 years 😄 Hope we get them way sooner, and then maybe we can use them in 2.0 anyway 🤞

[kyay10]

@nomisRev right yes it won't be stable yet for Arrow 2.0
However though, if we're having our minimal comparability be 1.7.x, could we still have context receiver functions in Arrow 2.0? (maybe in a separate package because I think turning them on makes the artifact a prerelease binary or something)

[nomisRev]

However though, if we're having our minimal comparability be 1.7.x, could we still have context receiver functions in Arrow 2.0? (maybe in a separate package because I think turning them on makes the artifact a prerelease binary or something)

Possibly, if we find something worthy of doing this. Currently the only benefit I currently see is not requiring OptionShift, ResultShift, etc. and the fact that we could move separate Effect to a kernel module.
The benefits there are marginal at best though, so it's not a blocker.

What I find more important is that we design the APIs to work nicely with context receivers, whenever they get released.
Such as the new APIs for handling errors:

object Error

context(Shift<Error>)
fun one(): Int {
  shift(Error)
  -1
}

val resolved: Effect<Error, Int> =
  effect { one() } catch { 1 }
  
 context(Shift<Error>)
 fun resolved2() : Int =
   // immediately returns `Int` here without requiring `bind` because we're inside `Shift` DSL. 
  effect { one() } catch { 1 }

If we're lucky ::one is recognised as suspend context(Shift<Error>) () -> Int and you'd be able to do:

 context(Shift<Error>)
 fun resolved2() : Int =
   ::one catch { 1 }

So this is IMO already ready to take advantage of Context Receivers in a significant way, and we should continue to design things in this way. Designing code for context receivers seems to be the same as designing code for Receiver DSLs in Kotlin.

[JavierSegoviaCordoba]

context receivers feature doesn't work with KMP, so adopting it now implies dropping KMP support, which is not reasonable IMO

[i-walker]

That looks really good , are we also addressing the catch API here since it is a vital part of Effect and is missing. // handleError/With, redeem/With, mapLeft, etc..

[i-walker]

👏🏾

[i-walker]

Suggested change

	private val parent: Continuation<B>,
	private val parent: Continuation<B>

[i-walker]

Sorry for that misunderstanding with the error handlers from 1.X up there

[raulraja]

looks great @nomisRev !

[raulraja]

👍

[nomisRev]

I converted this PR back to a DRAFT to prevent it from getting merged. I am currently still writing docs, and tests and investigating EagerEffect. I've also updated the APIs a bit in discussion with @jrgonzalezg, and taking into account a long-outstanding feature request by @LordRaydenMK which I've also needed quite a lot in the past.

One problem I found is that having Effect<E, A> as typealias breaks @BuilderInference & inference from time to time. Rewriting Effect<E, A> to class Effect<E, A> (..) : suspend Shift<E>.() -> A to partially apply type arguments doesn't seem to improve the situation though. Will raise a branch where I experimented with this. Below some examples.

object User
object Error

val x = effect<Error, User> { // effect<Nothing, User> {
  throw IllegalArgumentException("builder missed args")
}.attempt<IllegalArgumentException, Error, User> { shift(Error) }
//.attempt { ex: IllegalArgumentException -> shift(Error) } // this would also work


// Examples below can also be fixed by explicitly specifying the type on the left-hand side.
val error = effect<Error, User> { shift(Error) } // // Shift(error)

val a = error.catch<Error, Error, User> { error -> User }
val b = error.catch<Error, String, User> { error -> shift("other-failure") }
val c = error.catch<Error, Nothing, User> { error -> throw RuntimeException("BOOM") }

This doesn't pose a huge problem perse, but I thought it was worth noting in this PR.

[serras]

I expect the new partial type annotations in Kotlin 1.7 to help a bit with it. For me the main problem has always been that if need to specify the error type you are forced to specify everything. But maybe now we can get away with less...

val a = error.catch<Error, Error, _> { error -> User }
val b = error.catch<_, _, User> { error -> shift("other-failure") }
val c = error.catch<_, _, User> { error -> throw RuntimeException("BOOM") }

[nomisRev]

@serras thanks for that comment. I need to look into it. I tried to solve the problem on branch effect-as-value-class, but it seems to make things much worse 😅

The biggest issue is that @BuilderInference cannot infer to Nothing when there is no shift,
this cannot be solved by wrapping the lambda in a class.

[nomisRev]

I think this PR is ready for re-review @i-walker @raulraja @serras. I explored the changes for EagerEffect as well.

New Changes

• We used Token to identify the Scope, but we now simply rely on the Shift instance reference.

  I.e.

  effect<String, Int> {
    effect<Nothing, Int> {
    	shift("error")
    }.bind()
  }

• Shift used to have a seperate impl, object : EffectScope<R>.
  This is now implemented by the Continuation<B> impl (FoldContinuation).

• Made all class related to Fold impl private, hiding them from the public API.
  I had to move EagerEffect#fold into EffectKt, but worth it to optimise over the public binary IMO.

• Wrote internal documentation on runtime for Effect and EagerEffect

• changed EagerEffect to a typealias

• Added attempt/catch for EagerEffect

• Renamed EagerEffect to EagerShift

[nomisRev]

If this is approved by everyone, I’ll backport the changes to 1.x were applicable.

[serras]

I’m definitely OK with the backport. However, what is the plan forward? Are you closing this PR and making a new one against a (rebased) arrow-2 branch, or merging with this?

[nomisRev]

Are you closing this PR and making a new one against a (rebased) arrow-2 branch, or merging with this?

Whatever makes most sense for you, and the other reviewers. cc\ @raulraja, @i-walker.
I think I can easily port it to a new branch for a slightly nicer diff, but maybe going through the code together over a call is more meaningful. Whatever makes most sense for all of you is fine with me.

However, what is the plan forward?

In case this was for the back port. My plan forward, after this PR, is to make the relevant changes in 1.x to close the gap as much as possible. Ideally everyone is already using catch and attempt when 2.0 is released.

[nomisRev]

I am going to close this PR in favour of #2797. We can re-open this PR if needed.