Deprecate Monoid and Semigroup in Optics #2985
Conversation
| @@ -100,7 +104,7 @@ public abstract interface class arrow/optics/Getter : arrow/optics/Fold { | |||
| public abstract fun choice (Larrow/optics/Getter;)Larrow/optics/Getter; | |||
| public abstract fun compose (Larrow/optics/Getter;)Larrow/optics/Getter; | |||
| public abstract fun first ()Larrow/optics/Getter; | |||
| public abstract fun foldMap (Larrow/typeclasses/Monoid;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | |||
There was a problem hiding this comment.
Unfortunately the change of the empty / combine method being the "main" one in Fold is breaking binary compatibility. But I don't know how to make these changes otherwise; any help is welcome.
| public fun <R> foldMap(empty: R, combine: (R, R) -> R, source: S, map: (focus: A) -> R): R | ||
|
|
||
| /** | ||
| * Map each target to a type R and use a Monoid to fold the results | ||
| */ | ||
| public fun <R> foldMap(M: Monoid<R>, source: S, map: (focus: A) -> R): R | ||
| @Deprecated(MonoidDeprecation, ReplaceWith("foldMap(M.empty(), M::combine, source, map)", "arrow.typeclasses.combine")) | ||
| public fun <R> foldMap(M: Monoid<R>, source: S, map: (focus: A) -> R): R = | ||
| foldMap(M.empty(), M::combine, source, map) |
There was a problem hiding this comment.
I think the only way to do this in a non-binary breaking change is to:
- Deprecate
foldMapwithMonoidbut keep it abstract - Implement the new
foldMapthrough the abstract method. (This would require creating an anonymousMonoid)
There was a problem hiding this comment.
This would meant that someone who implements a custom Fold needs to refactor a breaking change in 2.0, but a consumer would not face a breaking change when not relying on the deprecated code.
There was a problem hiding this comment.
I think I've managed to keep ABI compatibility by:
- Keep the
Foldinterface as it was, usingMonoid; - Define the new
foldandfoldMapas extension functions, so they don't become part of the interface.
arrow-libs/optics/arrow-optics/src/commonTest/kotlin/arrow/optics/EveryTest.kt
Outdated
Show resolved
Hide resolved
arrow-libs/optics/arrow-optics/src/commonTest/kotlin/arrow/optics/EveryTest.kt
Show resolved
Hide resolved
| @@ -50,6 +50,12 @@ public interface Monoid<A> : Semigroup<A> { | |||
| public fun fold(elems: List<A>): A = elems.fold() | |||
|
|
|||
| public companion object { | |||
| @JvmStatic | |||
| public fun <A> of(empty: A, combine: (A, A) -> A): Monoid<A> = object : Monoid<A> { | |||
There was a problem hiding this comment.
Should all these constructors not be deprecated?
| /** | ||
| * Fold using the given [empty] element and [combine]. | ||
| */ | ||
| public fun <S, A> Fold<S, A>.fold(empty: A, combine: (A, A) -> A, source: S): A = |
There was a problem hiding this comment.
Can this be added as a method on Fold?
There was a problem hiding this comment.
I made this change (and the one below) for ABI compatibility. If I include it in the interface, it becomes part of it, so in theory previous users are not binary-compatible with it.
If we prefer not requiring the additional imports, we just need to revert the last two commits in this PR.
| /** | ||
| * Map each target to a type [R] and combine the results as a fold. | ||
| */ | ||
| public fun <S, A, R> Fold<S, A>.foldMap(empty: R, combine: (R, R) -> R, source: S, map: (focus: A) -> R): R = |
There was a problem hiding this comment.
Is this an extension because it was a conflicting API? and @JvmName doesn't work on interface?
There was a problem hiding this comment.
As discussed above, this is an extension so that the method is not added to the interface, which would break binary compatibility.
There was a problem hiding this comment.
Adding methods is not a problem, only removal, since this would break compatibility for users relying on dependencies with older Arrow versions of the 1.x.x series.
There was a problem hiding this comment.
Oh, I misunderstood the binary compatibility then. I've moved the functions back to Fold, where they should logically belong.
There was a problem hiding this comment.
It's really a JVM thing I guess, the problem is the following:
- Library A has a dependency on Arrow 1.1.5 relies on
Fold.foldMap - Project B has dependency on Library A & has dependency on Arrow 1.1.5
- Arrow 1.2.0 removes
Fold.foldMap - Project B bumps dependency to Arrow 1.2.0 => Fold.foldMap: MethodNotFoundException at runtime.
So adding methods is non-problematic.
Fixes #2975
This PR also includes some additional changes in the code to remove calls to deprecated methods like
bimapandorElse.