Feature/remove validated

[Atternatt]

• Removed Validated From the project
• Had several knit reports (want to discuss and fix)

[Atternatt]

Hey @nomisRev In this Pr I removed only the Validated and I also removed the example files that contained Validated examples. (also renamed the examples to be consecutive).
At this point I tried to run a grade build and I've got a lot of knit warnings and lately an error:

Execution failed for task ':knitCheck'.
> knitCheck task failed, see log for details (use '--info' for detailed log).
  Run 'knit' task to write 23 missing/outdated files.

I just want to double check how to proceed in this case and how to fix it (as well for the checks that the actions are not passing that looks like a lot of them)

[serras]

@Atternatt The easiest fix is to remove all the Validated examples from Knit and run gradle knit again

[serras]

@Atternatt maybe this is a bit too much asking, but could this be based on #2784? I think we are working on removing many similar things (like computation blocks), and duplicating work would be a waste of resources

[Atternatt]

@Atternatt maybe this is a bit too much asking, but could this be based on #2784? I think we are working on removing many similar things (like computation blocks), and duplicating work would be a waste of resources

I was going to pick that feature but @nomisRev told me that you already picked it so we were discussing what I could do. Then we came to this that consisted of removing Validated from Arrow.
So far, I removed all the references in the project and the example files. Perhaps we could remove a common file with coss dependencies between Validated and Tuple?

[serras]

@Atternatt on a second thought, just go on. My removal touches some parts, but merging should take care of it.

[Atternatt]

on the other hand, would you mind to help me understand what the build errors for the github actions are in order to fix them?

[serras]

Sure thing! The first step would be to run gradle apiDump to update the API files and then commit and push. Since you are removing (lots) of functions the API Compatibility Validator screams at you… but this is the point of starting a new point release, right?

[serras]

To fix the Knit problems, remove all the files in arrow-libs/core/arrow-core/src/jvmTest/kotlin/examples and run gradle knit to recreate the examples. Knit expects to find a file for every example, but since you are removing a few, it can no longer backreference them and fails.

[Atternatt]

while removing documentation refering to Validated I found something I'm not sure how to fix in arrow-site/docs/docs/patterns/errorhandling/README.md:229.
How should we update this example?

[nomisRev]

@Atternatt the error handling docs are IMO up for revamping after these changes.

We’ve also changed how to do error handling in Effect, and I hope to change all APIs accordingly. So that would mean this doc would be almost completely outdated. So feel free to remove, or ignore it for now.

[nomisRev]

This PR proposes following changes:

Integrate Validated into Either

Based on some discussion on KotlinLang, and quite some confusion why there is both Validated and Either especially amongst beginners we wanted to explore merging the two. So a small recap between the difference.

Either<E, A> and Validated<E, A> model the same ADT, which is the disjunction between E and A. Either allows you to write happy-path code, and allows sequencing Either<E, A> values that depend on each-other using bind (Monad). Whilst Validated only allows combining values using zip, where you combine both (E, E) -> E and (A, B) -> C. (Applicative). The functionality of Validated can also be exposed over Either, and this PR proposes exactly that.

The reason for doing this is that we want to heavily decrease the API surface, and the resulting learning curve. After the 1.x.x series we realised that combing with the Arrow DSL having both types is not useful, and this is a good opportunity to heavily decrease the API surface. The 1.x.x Either API deprecation PR explains this a little bit as well.

Integrating Validated into Either

In 1.x.x we deprecated Either#zip in favour of either { f(bind(), b.bind(), ...) } which also solves the arity-n issue of zip. This PR replaces the Either#zip behaviour with the accumulating errors powers of Validated, but we can also consider a different name than zip such as fa.validate(fb, ::Tuple2) or fa.accumulate(fb, ::Tuple2). This PR proposes 3 different signatures to work with Either for validation purposes (only arity-2 shown below).

/** The most generic signature */
inline fun <E, A, B, C> Either<E, A>.zip(
  combine: (E, E) -> E,
  other: Either<E, B>,
  transform: (A, B) -> C
): Either<E, C>

/**  Automatically accumulate inside NonEmptyList, without requiring the original `E` to be wrapped */
inline fun <E, A, B, C> Either<E, A>.zip(
  other: Either<E, B>,
  transform: (A, B) -> C
): Either<NonEmptyList<E>, C>

/**
 * A function to combine results of the above function, or validations that would previously return `ValidatedNel`.
 * Regular `Either` values can be lifted into `NonEmptyList<E>` by using `toEitherNel()`.
 */
inline fun <E, A, B, C> Either<NonEmptyList<E>, A>.zip(
  other: Either< NonEmptyList<E>, B>,
  transform: (A, B) -> C
): Either<NonEmptyList<E>, C>

traverse

Besides combining independent values of Validated a second big use-case for using it is traverse which is completely being deprecated for the Arrow DSL, and therefore we also need to find a solution for where you'd previously use traverse + Validated. To replace traverse one can simply use the DSL + Iterable#map, and therefore we thought mapAccumulating offers us an idiomatic name for accumulating typed errors which is what traverse + Validated did.

either<String, List<Int>> {
   listOf(1, 2, 3).map { raise("fail") }
   //listOf(1, 2, 3).map { Either.Left("fail").bind() }
} // Either.Left("fail")

either<String, List<Int>> {
  listOf(1, 2, 3).mapAccumulating(String::plus) { raise("fail") }
  //listOf(1, 2, 3).mapAccumulating(String::plus) { Either.Left("fail").bind() }
} //  Either.Left("failfailfail")

In the same fashion as zip mapAccumulating also provides 2 different methods.

public inline fun <Error, A, B> Iterable<A>.mapAccumulating(
  semigroup: Semigroup<Error>,
  transform: Raise<Error>.(A) -> B,
): Either<Error, List<B>>

public inline fun <Error, A, B> Iterable<A>.mapAccumulating(
  transform: Raise<Error>.(A) -> B,
): Either<NonEmptyList<Error>, List<B>>

It works over Raise<E>, but returns a concrete Either. Snippet below compares traverse to mapAccumulating usages:

fun process(i: Int): Either<String, Int> = i.right()

listOf(1, 2, 3).traverse { process(it) }
listOf(1, 2, 3).mapAccumulating { process(it).bind() }

This API is optimised for context receivers, or working with Raise<E> extension functions. Otherwise it requires the additional call to bind().

Additional changes

Semigroup

To make this PR compile, and these functions to accept (E, E) -> E where fun interface Semigroup is defined the abstract function of Semigroup had to change shape from E.(E) -> E to (E, E) -> E. This is done in a non-binary breaking way, but we can still consider removing Semigroup in a different PR or changing its APIs.

[raulraja]

Thanks @Atternatt @nomisRev !

[pakoito]

Sounds like an overall positive change! We felt the pain of always reaching for ValidatedNel, but playing with types to the point of defining an EitherNel alias :D

My suggestions are:

• mapAccumulating -> mapAccumulatingLeft
• I'd still keep the equivalent of sequence to flatten lists of Either

[nomisRev]

Hey @pakoito,
Thanks for sharing, great to hear that you've also experienced this and looked for a middle ground in 1.x.x 😁

mapAccumulating -> mapAccumulatingLeft

I felt mapAccumulating was already too long 😂 The name is definitely open for suggestions 🙏 I personally feel mapAccumulatingLeft is a bit long, but mapAccumLeft is perhaps a bit foreign. Should it reflect Raise, or did you choose Left in the name because of the Either return type? Perhaps collectRaise 🤔 collect conflicts a bit with Flow#collect though..

Do you like the new signatures?

I'd still keep the equivalent of sequence to flatten lists of Either

We can definitely do that, but sequence is perhaps not the best name for this 🤔 Any suggestions?

[pakoito]

The convention in the stdlib is "XOrY", so this could follow:

For mapping: mapOrAccumulate, mapOrRaiseFirst

For sequencing, "merge" isn't taken: mergeOrRaiseFirst + mergeOrAccumulate.

Do you like the new signatures?

You know I do :D Nested zips will be a bit meh, but I'll probably reach for mergeOrAccumulate + deconstruction instead.

[nomisRev]

For mapping: mapOrAccumulate, mapOrRaiseFirst

mapOrRaiseFirst do you mean short-circuit behaviour? That is just map.

object Error

fun process(i: Int): Either<Error, Int> = i.right()
fun Raise<Error>.process2(i: Int): Int = i

either<Error, List<Int>> {
  listOf(1, 2, 3).map { process(it).bind() }
  listOf(1, 2, 3).map { process2(it) }
}

I do like mapOrAccumulate! It's more descriptive than mapAccumulate since with mapAccumulate the Accumulate seems to refer to happy-path whilst Or is a Kotlin Std established pattern that it refers to unhappy-path. Thanks @pakoito ❤️

For sequencing, "merge" isn't taken: mergeOrRaiseFirst + mergeOrAccumulate.

I was originally thinking of flatten but didn't propose it since there is no actually flattening (of List or otherwise) happening. Arrow already has Either<A, A>.merge, do you think it would be confusing to use merge here as well?

Nested zips will be a bit meh, but I'll probably reach for mergeOrAccumulate + deconstruction instead.

Do you mean nested zips to achieve arities higher than 9? mergeOrAccumulate + deconstruction is untyped 😭

[pakoito]

Arrow already has Either<A, A>.merge, do you think it would be confusing to use merge here as well?

I'd rename that merge to converge or similar, if that frees the term. It's an extfun over a different type so it'll light up during migration.

[i-walker]

is this still part of the Validated removal?
And is append going to replace combine

[nomisRev]

I addressed it briefly in my comment, the problem is that the current combine signature doesn't match the signature we want for zip. So these changes belong together. There has been some talk about removing Semigroup, which would remove this changes (and horrible API name). And would replace the Semigroup<E> argument with a simple lambda (E, E) -> E. cc\ @raulraja

Additional changes

Semigroup

To make this PR compile, and these functions to accept (E, E) -> E where fun interface Semigroup is defined the abstract function of Semigroup had to change shape from E.(E) -> E to (E, E) -> E. This is done in a non-binary breaking way, but we can still consider removing Semigroup in a different PR or changing its APIs.

[i-walker]

Sorry I missed that :)

[i-walker]

Looks good, I would opt to separate the changes in Semigroup in a separate PR.
Is there going to be another PR on writing the stack-safety tests for NonEmptyList, Iterable, etc, or can I pick that up?

[nomisRev]

I'd rename that merge to converge or similar, if that frees the term. It's an extfun over a different type so it'll light up during migration.

@pakoito I'm hesitant to rename it since it's been quite well established by now. It's a quite popular API afaik, and there will already be so many changes. Also (List<Either<E, A>>) -> Either<E, List<A>> doesn't really merge. When I explain it to someone unfamiliar with the pattern I typically say it flips the container List and Either. But I don't like flip for an API name.

[serras]

@pakoito @nomisRev About the sequence function, I feel like the following expresses the intent quite well.

either { thing.map { it.bind() } } 

I could see that this is a bit complicated to write the first time, though. However, we should remember that the intention is to move people towards the Raise DSL, where you don't need explicit bind at all.

Going into the bind direction, may I suggest instead to have the following function?

fun <E, A> Iterable<Either<E, A>>.bindAll(): List<A> = map { it.bind() }

This would be just a short-hand for "bind everything in this Iterable", in other words, "check that everything in this Iterable is correct". This is slightly more generic than sequence, because we can use it within an either block, but still write either { listOfEithers.bindAll() } if needed.

[serras]

I'm merging this PR so we can continue work in other branches. I've moved the discussion about the "missing function" for iterables of Eithers to #2861.