Deprecate latestError and hasAccumulatedErrors

[kyay10]

I realized this is a bad API to have.
Accumulate is meant to be an effect. Already, it's important to keep effect interfaces minimal.
The crucial fault here is that latestError reveals too much about Accumulate: it lets you check whether a failure has happened, and short-circuit early if so.
This means that introducing accumulate { } changes that behaviour easily. We want an Accumulate-using expression e to be equivalent to accumulate(::either) { e }.bindNelOrAccumulate(), but this wasn"t the case with latestError exposed. Now this equivalence is true.
The only "legitimate" use for it was in accumulate, which heavily implies that it was an implementation detail; accumulate wants to know, after the block runs, whether any errors happened. It now gets access to that implementation detail by picking the exact implementation of Accumulate it wants (namely ListAccumulate).

[github-actions]

Kover Report

File	Coverage [33.38%]
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Builders.kt	96.47%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/RaiseAccumulate.kt	24.46%

Total Project Coverage	47.67%

[serras]

To be honest, I'm not super-happy with this change.

It's true that you don't expose things like latestError, but on the other hand you are constraining to the ListAccumulate implementation (here). I'd rather prefer to leave the implementation open to other potential cases (imagine something that logs the errors instead).

ScopedRaiseAccumulate changes its behavior, since you now need all the additional accumulate wrappers.

[kyay10]

That behaviour change is intentional, but I should've done it in a separate PR. parZipOrAccumulate previously didn't function the same as zipOrAccumulate w.r.t. accumulated errors. It's fine to change this behaviour since it's more correct, and it only concerns accumulated errors, which arise only thru experimental raise accumulate APIs.
Should I undo the ScopedRaiseAccumulate changes and do it later in another PR?

[kyay10]

w.r.t. constraining it to ListAccumulate, this constraining only happens in hidden API that's there for binary compatibility. underlyingListAccumulate is only called in these deprecated methods, and those methods were always called from a call to accumulate, which always used ListAccumulate (and still does). The only exception is if an experimental API user used latestError or hasAccumulatedErrors directly, but, I believe, breaking behaviour like this is fine for an experimental API. LMK if I should add a clearer error message or comment on that line, or perhaps mark it with Deprecated to make this even clearer
TL;DR: the line you highlight only runs when there's old inlined bytecode running against new Arrow. That old inlined bytecode only happened in experimental uses (which we can break), or in accumulate and forEachAccumulating, which both use ListAccumulate, so the line succeeds.
Thus, the user is free to use any implementation they want. They won't, however, get latestError or hasAccumulatedError anymore, and so if they want similar functionality, they can add it to their custom implementation (like we've done for ListAccumulate).

[serras]

Should I undo the ScopedRaiseAccumulate changes and do it later in another PR?

Yes, please. I think that aligning parZipOrAccumulate with zipOrAccumulate is a good thing. But about changing the implementation of RaiseAccumulate altogether, I'm way less sure about.

[kyay10]

@serras Thanks for steering me away from implementation changes! The PR is much simpler now. It's just marking things as Deprecated. The only code changes should be straightforward enough to verify (accumulate now uses an internal method to get access to a way to raise errors upon finishing, and forEachAccumulatingImpl now tracks hasErrors on its own)