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

feat: If DeclarativeRecipe only has already-initialized recipes, don't require initialization #4267

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

kmccarp
Copy link
Contributor

@kmccarp kmccarp commented Jun 17, 2024

What's changed?

I've updated the way that DeclarativeRecipe.validate() works. Before, it was always required to call DeclarativeRecipe.initialize() in order to validate that all recipes were initialized before using the recipe.

What's your motivation?

There are use cases where a DeclarativeRecipe can be constructed with already-instantiated recipe objects. In this case, we don't want to require initialization.

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

N/A

Anyone you would like to review specifically?

@sambsnyd

Have you considered any alternatives or workarounds?

Yes, the most important part here was to fix a null pointer. Before this change, the validation code was as follows:

        return Validated.<Object>test("initialization",
                        "initialize(..) must be called on DeclarativeRecipe prior to use.",
                        this, r -> initValidation != null)
                .and(validation)
                .and(initValidation);

If initialize was not called, then initValidation would be null. the .and(initValidation) would throw a NullPointerException.

The most important aspect of this PR was to fix that NullPointerException, and I considered doing so and keeping the validation the same otherwise:

        return Validated.<Object>test("initialization",
                        "initialize(..) must be called on DeclarativeRecipe prior to use.",
                        this, r -> initValidation != null)
                .and(validation)
                .and(initValidation == null ? Validated.none() : initValidation);

This would have the effect that a DeclarativeRecipe that didn't have .initialize() called on it would still throw a warning, but not blow up.

Any additional context

N/A

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

@kmccarp kmccarp requested a review from sambsnyd June 17, 2024 20:40
@sambsnyd sambsnyd merged commit 1e409ed into openrewrite:main Jun 19, 2024
2 checks passed
crankydillo pushed a commit to crankydillo/rewrite that referenced this pull request Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants