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

Recipe to simplify assertions on Optionals #472

Closed

Conversation

timo-abele
Copy link
Contributor

@timo-abele timo-abele commented Jan 25, 2024

What's changed?

Adds first refaster recipe

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

copied from rewrite-recipe-starter
@timtebeek
Copy link
Contributor

timtebeek commented Jan 26, 2024

Thanks for getting this started! As indicated in Slack as well we have two small hurdles to cross here before we can merge this in

  1. support for generic over in https://github.com/openrewrite/rewrite-templating
  2. allowing Java 9+ features in refaster recipes with something similar to

We're very near with all of those, so we can keep this PR open and return to it once we're ready.

@timtebeek timtebeek added the recipe Recipe request label Feb 5, 2024
@timtebeek timtebeek changed the title Feature: simplify optionals Recipe to simplify assertions on Optionals Feb 5, 2024
@timtebeek
Copy link
Contributor

I'm guessing this has since been surpassed by

If so we can close this one.

Thanks though! Good attempt if we were just a tad further with rewrite-templating such that generated recipes are safe to run on Java 8.

@timo-abele
Copy link
Contributor Author

I'm guessing this has since been surpassed by

* [Add more simplifications #474](https://github.com/openrewrite/rewrite-testing-frameworks/pull/474) ?

If so we can close this one.

Thanks though! Good attempt if we were just a tad further with rewrite-templating such that generated recipes are safe to run on Java 8.

In terms of functionality this PR is obsolete. Once the blockers are resolved, rewriting the recipes currently covered by SimplifyChainedAssertJAssertion into refaster recipes would make the code more readable. Then, the code written here could be reused. But it wouldn't be much effort to write it anew either...

@timtebeek
Copy link
Contributor

I'm leaning towards closing this one at least for now then, as we have some blockers to sort first, and even then there might be performance challenges to solve before we rewrite more recipes to Refaster style (as much as I like that option!).

@timo-abele timo-abele closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

assertThat(anOptional.isPresent()).isTrue() should be converted to assertThat(anOptional).isPresent()
2 participants