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

Convert Guava Optional to JDK #197

Closed
5 of 7 tasks
timtebeek opened this issue Mar 23, 2023 · 7 comments
Closed
5 of 7 tasks

Convert Guava Optional to JDK #197

timtebeek opened this issue Mar 23, 2023 · 7 comments
Labels
good first issue Good for newcomers recipe Recipe requested

Comments

@timtebeek
Copy link
Contributor

timtebeek commented Mar 23, 2023

We already have quite some recipes to convert Guava classes to standard JDK equivalents:
https://github.com/openrewrite/rewrite-migrate-java/blob/main/src/main/resources/META-INF/rewrite/no-guava.yml
Seems like Guava's Optional could be a nice addition to the list of supported migrations.

Guava

https://guava.dev/releases/19.0/api/docs/com/google/common/base/Optional.html

Modifier and Type Method and Description
static  Optional absent()Returns an Optional instance with no contained reference.
abstract Set asSet()Returns an immutable singleton Set whose only element is the contained instance if it is present; an empty immutable Set otherwise.
static  Optional fromNullable(T nullableReference)If nullableReference is non-null, returns an Optional instance containing that reference; otherwise returns absent().
abstract T get()Returns the contained instance, which must be present.
abstract boolean isPresent()Returns true if this holder contains a (non-null) instance.
static  Optional of(T reference)Returns an Optional instance containing the given non-null reference.
abstract Optional or(Optional<? extends T> secondChoice)Returns this Optional if it has a value present; secondChoice otherwise.
abstract T or(Supplier<? extends T> supplier)Returns the contained instance if it is present; supplier.get() otherwise.
abstract T or(T defaultValue)Returns the contained instance if it is present; defaultValue otherwise.
abstract T orNull()Returns the contained instance if it is present; null otherwise.
static  Iterable presentInstances(Iterable<? extends Optional<? extends T>> optionals)Returns the value of each present instance from the supplied optionals, in order, skipping over occurrences of absent().
abstract  Optional transform(Function<? super T,V> function)If the instance is present, it is transformed with the given Function; otherwise, absent() is returned.

JDK

https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html

Modifier and Type Method and Description
static  Optional empty()Returns an empty Optional instance.
Optional filter(Predicate<? super T> predicate)If a value is present, and the value matches the given predicate, return an Optional describing the value, otherwise return an empty Optional.
 Optional flatMap(Function<? super T,Optional> mapper)If a value is present, apply the provided Optional-bearing mapping function to it, return that result, otherwise return an empty Optional.
T get()If a value is present in this Optional, returns the value, otherwise throws NoSuchElementException.
void ifPresent(Consumer<? super T> consumer)If a value is present, invoke the specified consumer with the value, otherwise do nothing.
boolean isPresent()Return true if there is a value present, otherwise false.
 Optional map(Function<? super T,? extends U> mapper)If a value is present, apply the provided mapping function to it, and if the result is non-null, return an Optional describing the result.
static  Optional of(T value)Returns an Optional with the specified present non-null value.
static  Optional ofNullable(T value)Returns an Optional describing the specified value, if non-null, otherwise returns an empty Optional.
T orElse(T other)Return the value if present, otherwise return other.
T orElseGet(Supplier<? extends T> other)Return the value if present, otherwise invoke other and return the result of that invocation.
<X extends Throwable>T orElseThrow(Supplier<? extends X> exceptionSupplier)Return the contained value, if present, otherwise throw an exception to be created by the provided supplier.

Quick analysis

There's a few cases that will need to be looked at in detail, such as:

  • absent -> empty
  • fromNullable -> ofNullable
  • or -> orElse
  • orNull -> orElse(null)
  • transform -> map
  • asSet -> dedicated recipe
  • static presentInstances -> dedicated recipe
@knutwannheden
Copy link
Contributor

There could be a few more small corner cases. The Guava Javadoc actually tell you what the difference is and what the Java equivalent is. Also note that version 21 added the utilities fromJavaUtil() and toJavaUtil() which should be easy to replace :-)

@timtebeek
Copy link
Contributor Author

That Comparison to java.util.Optional note is helpful indeed; hadn't noticed that yet; think I arrived at mostly the same, but good to check that when implementing.

I think we can cover most of these quite nicely with declarative recipes already; the remaining ones might be doable with rewrite templates once that's further along, and don't seem to be as highly used as some of the other ones.

@knutwannheden
Copy link
Contributor

I thought a bit about this recipe and it seems like it could be a bit difficult to implement if we have to ensure that the type attribution is still correct everywhere. But I may think about it the wrong way.

@timtebeek
Copy link
Contributor Author

Looking at some of the other recipes we seem to mostly call out to ChangeType in similar cases.
This one would be slightly different as it would also require a first few ChangeMethodName calls before ChangeType, and dedicated recipes for the not-one-to-one cases. To start with I don't there's much else right?

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.guava.PreferJavaUtilFunction
displayName: Prefer `java.util.function.Function`
description: Prefer `java.util.function.Function` instead of using `com.google.common.base.Function`.
tags:
- guava, RSPEC-4738
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: com.google.common.base.Function
newFullyQualifiedTypeName: java.util.function.Function
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.guava.PreferJavaUtilPredicate
displayName: Prefer `java.util.function.Predicate`
description: Prefer `java.util.function.Predicate` instead of using `com.google.common.base.Predicate`.
tags:
- guava, RSPEC-4738
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: com.google.common.base.Predicate
newFullyQualifiedTypeName: java.util.function.Predicate
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.guava.PreferJavaUtilSupplier
displayName: Prefer `java.util.function.Supplier`
description: Prefer `java.util.function.Supplier` instead of using `com.google.common.base.Supplier`.
tags:
- guava, RSPEC-4738
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: com.google.common.base.Supplier
newFullyQualifiedTypeName: java.util.function.Supplier

@knutwannheden
Copy link
Contributor

I guess you are right. It probably just gets a bit weird when we rewrite uses of Guava Optional where some called third-party API expects a Guava Optional. Especially if this is just some other Guava API. But I guess this kind of problem can happen everywhere and is hard to do anything about. Of course, in this specific case, we could use the fromJavaUtil() and toJavaUtil() utilities in these places. But this sounds non-trivial.

@knutwannheden
Copy link
Contributor

The other problem I was thinking of is what happens to the type attribution between the ChangeMethodName, ChangeMethodTargetToStatic , and ChangeType recipes and where in this order we insert the imperative recipes. Will the imperative recipe be able to rely on the type attribution to do its work?

Cases like Optional.fromNullable(null).or(Optional.of("foo")) may require a very careful ordering of the recipes so that this works properly.

@timtebeek
Copy link
Contributor Author

Mostly implemented in #202 ; propose we close this one and pick up the asSet and presentInstances methods only when asked. Preferably by then we're further along with rewrite-templating to make that even easier to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers recipe Recipe requested
Projects
Archived in project
Development

No branches or pull requests

2 participants