Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CU-g52kvp - Fix Java API for typeclasses in Arrow Optics #2336

Merged
merged 22 commits into from Mar 26, 2021

Conversation

danimontoya
Copy link
Contributor

@danimontoya danimontoya commented Mar 23, 2021

Fixes https://app.clickup.com/t/g52kvp
Move arrow.optics.std.* functions into each type class companion object with JvmStatic annotation and remove imports from deprecations:

  • list
  • either
  • map
  • nonemptylist
  • option
  • sequence
  • set
  • string
  • validated
  • tuple

…Static annotation and remove imports from deprecations
@franciscodr
Copy link
Collaborator

Task linked: CU-g52kvp Improve usage from Java

…vmStatic annotation and remove imports from deprecations
…tatic annotation and remove imports from deprecations
…with JvmStatic annotation and remove imports from deprecations
…vmStatic annotation and remove imports from deprecations
… JvmStatic annotation and remove imports from deprecations
…tatic annotation and remove imports from deprecations
…vmStatic annotation and remove imports from deprecations
…h JvmStatic annotation and remove imports from deprecations
…mStatic annotation and remove imports from deprecations
@danimontoya danimontoya marked this pull request as ready for review March 24, 2021 10:51
@@ -16,7 +16,7 @@ import arrow.typeclasses.Applicative
*/
@Deprecated(
"arrow.optics.extensions package is being deprecated, function is being moved to arrow.optics.traversal",
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either", "arrow.optics.traversal"),
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either"),
ReplaceWith("Traversal.either<L, R>()", "arrow.optics.Traversal"),

@@ -30,7 +30,7 @@ fun <L, R> Either.Companion.traversal(): Traversal<Either<L, R>, R> = object : T
*/
@Deprecated(
"Each is being deprecated. Use Traversal directly instead.",
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either", "arrow.optics.traversal"),
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ReplaceWith("Either.traversal<L, R>()", "arrow.core.Either"),
ReplaceWith("Traversal.either<L, R>()", "arrow.optics.Traversal"),

@@ -28,7 +28,7 @@ internal val at_singleton: SetKAt<Any?> = object : SetKAt<Any?> {}
"arrow.optics.extensions package is being deprecated, and it will be removed in 0.13.",
ReplaceWith(
"this compose At.set<A>().at(i)",
"arrow.optics.set", "arrow.optics.typeclasses.At", "arrow.optics.compose"
"arrow.optics.typeclasses.At", "arrow.optics.compose"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no "arrow.optics.compose" import. This needs to be removed from all ReplaceWith, this was here already before this PR but let's clean it up here.

/**
* [PIso] that defines the equality between [Either] and [Validated]
*/
fun <A1, A2, B1, B2> Either.Companion.toPValidated(): PIso<Either<A1, B1>, Either<A2, B2>, Validated<A1, B1>, Validated<A2, B2>> = PIso(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot be removed since it was present in 0.11.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deprecated in favor of Iso.pValidated()

/**
* [Iso] that defines the equality between [Either] and [Validated]
*/
fun <A, B> Either.Companion.toValidated(): Iso<Either<A, B>, Validated<A, B>> = toPValidated()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot be removed since it was present in 0.11.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deprecated in favor of Iso.validated()


/**
* [Iso] that defines the equality between a Unit value [Map] and a [Set] with its keys
*/
@Deprecated(
"MapK is being deprecated, use the function defined for Map instead.",
"MapK is being deprMapInstanceTest.ktecated, use the function defined for Map instead.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"MapK is being deprMapInstanceTest.ktecated, use the function defined for Map instead.",
"MapK is being deprecated, use the function defined for Map instead.",

/**
* [PIso] that defines the equality between [Option] and the nullable platform type.
*/
fun <A, B> Option.Companion.toPNullable(): PIso<Option<A>, Option<B>, A?, B?> = PIso(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [PIso] that defines the isomorphic relationship between [Option] and the nullable platform type.
*/
fun <A> Option.Companion.toNullable(): Iso<Option<A>, A?> = toPNullable()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [PPrism] to focus into an [arrow.core.Some]
*/
fun <A, B> Option.Companion.PSome(): PPrism<Option<A>, Option<B>, A, B> = PPrism(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [Prism] to focus into an [arrow.core.Some]
*/
fun <A> Option.Companion.some(): Prism<Option<A>, A> = PSome()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [Prism] to focus into an [arrow.core.None]
*/
fun <A> Option.Companion.none(): Prism<Option<A>, Unit> = Prism(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [Iso] that defines the equality between and [arrow.core.Option] and [arrow.core.Either]
*/
fun <A, B> Option.Companion.toPEither(): PIso<Option<A>, Option<B>, Either<Unit, A>, Either<Unit, B>> = PIso(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated

/**
* [Iso] that defines the equality between and [arrow.core.Option] and [arrow.core.Either]
*/
fun <A> Option.Companion.toEither(): Iso<Option<A>, Either<Unit, A>> = toPEither()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be removed, present in 0.11.0. Should be deprecated


/**
* [PLens] to focus into the first value of a [arrow.core.Tuple2]
*/
Copy link
Member

@nomisRev nomisRev Mar 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing in this file can be removed since present in 0.11.0. All these methods need to be deprecated in favor of their @JvmStatic version on Lens Companion.

The ones for Tuple2 & Tuple3 don't have to be recreated on Lens.Companion since they're being deprecated. Deprecate these optics with the same message that is deprecating Tuple2 & Tuple3.
Instead only expose the Pair/Triple variant on Lens.Companion.

/**
* [PIso] that defines equality between [Validated] and [Either]
*/
fun <A1, A2, B1, B2> Validated.Companion.toPEither(): PIso<Validated<A1, B1>, Validated<A2, B2>, Either<A1, B1>, Either<A2, B2>> = PIso(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are present on 0.11.0 and cannot be removed. They need to be deprecated in favor of Iso.pEither() and Iso.either().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem, should've been validatedToPEither and same for the deprecation

Comment on lines 29 to 31
cons() compose Tuple2.first()
cons() compose Lens.tuple2First()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can stay as it is until 0.13.0


/**
* Provides an [Optional] between [S] and its tail [S].
*/
fun tailOption(): Optional<S, S> =
cons() compose Tuple2.second()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can stay as it is until 0.13.0

@@ -27,12 +33,12 @@ fun interface Snoc<S, A> {
/**
* Provides an [Optional] between [S] and its init [S].
*/
fun initOption(): Optional<S, S> = snoc() compose Tuple2.first()
fun initOption(): Optional<S, S> = snoc() compose PLens.tuple2First()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can stay as it is until 0.13.0


/**
* Provides an [Optional] between [S] and its last element [A].
*/
fun lastOption(): Optional<S, A> = snoc() compose Tuple2.second()
fun lastOption(): Optional<S, A> = snoc() compose PLens.tuple2Second()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can stay as it is until 0.13.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @nomisRev , I hope I addressed all the comments! 😄

* [PIso] that defines the equality between [Either] and [Validated]
*/
@JvmStatic
fun <A1, A2, B1, B2> pValidated(): PIso<Either<A1, B1>, Either<A2, B2>, Validated<A1, B1>, Validated<A2, B2>> =
Copy link
Member

@nomisRev nomisRev Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be named eitherToPValidated like listToPOptionNel.

Suggested change
fun <A1, A2, B1, B2> pValidated(): PIso<Either<A1, B1>, Either<A2, B2>, Validated<A1, B1>, Validated<A2, B2>> =
fun <A1, A2, B1, B2> eitherToPValidated(): PIso<Either<A1, B1>, Either<A2, B2>, Validated<A1, B1>, Validated<A2, B2>> =

* [Iso] that defines the equality between [Either] and [Validated]
*/
@JvmStatic
fun <A, B> validated(): Iso<Either<A, B>, Validated<A, B>> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun <A, B> validated(): Iso<Either<A, B>, Validated<A, B>> =
fun <A, B> eitherToValidated(): Iso<Either<A, B>, Validated<A, B>> =

* [PIso] that defines equality between [Validated] and [Either]
*/
@JvmStatic
fun <A1, A2, B1, B2> pEither(): PIso<Validated<A1, B1>, Validated<A2, B2>, Either<A1, B1>, Either<A2, B2>> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun <A1, A2, B1, B2> pEither(): PIso<Validated<A1, B1>, Validated<A2, B2>, Either<A1, B1>, Either<A2, B2>> =
fun <A1, A2, B1, B2> validatedToPEither(): PIso<Validated<A1, B1>, Validated<A2, B2>, Either<A1, B1>, Either<A2, B2>> =

* [Iso] that defines equality between [Validated] and [Either]
*/
@JvmStatic
fun <A, B> either(): Iso<Validated<A, B>, Either<A, B>> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun <A, B> either(): Iso<Validated<A, B>, Either<A, B>> =
fun <A, B> validatedToEither(): Iso<Validated<A, B>, Either<A, B>> =

*/
@JvmStatic
fun <A> tripleTraversal(): Traversal<Triple<A, A, A>, A> =
triplePTraversal()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

traversal for Tuple4 -> Tuple10 is missing here.

@Deprecated(
"Use the pValidated function exposed in the Iso' companion object",
ReplaceWith(
"Iso.pValidated()",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be updated then to eitherToPValidated.

"arrow.optics.Traversal"
),
DeprecationLevel.WARNING
)
fun <A> Tuple3.Companion.traversal(): Traversal<Tuple3<A, A, A>, A> = pTraversal()

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If pTraversal for Tuple4 -> Tuple5 is added to Traversal than all the other ones here have to be deprecated also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @nomisRev for the review! applied the comments 👍

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there @danimontoya. Good work!

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect @danimontoya. 1 small deprecation suggestion ;) Thanks for the great work!!! ❤️ 👏 👏

…d/either.kt

Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @danimontoya !

@danimontoya danimontoya merged commit 0af226b into master Mar 26, 2021
@danimontoya danimontoya deleted the CU-g52kvp-improve-usage-java branch March 26, 2021 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants