openrewrite
Make ParentPomInsight recursive
What's changed?
Made ParentPomInsight recursive, with a parameter to disable recursivity.
What's your motivation?
We needed to check the parents more deeply before performing some cleanup. @sambsnyd suggested updating this recipe instead of creating a new one. @timtebeek suggested to make it the default.
This could also be used to conditionally perform some upgrades, e.g. only perform Spring Boot 3 migration if we are not already in Spring Boot 3 (see also openrewrite/rewrite-spring#361) – although you would also need to check the imported poms in this case.
Anything in particular you'd like reviewers to focus on?
Note how the change affects matchNonSnapshot() which must explicitly disable recursivity to keep the same behavior (otherwise all child modules would match as well).
I also took the opportunity to document Semver.validate(), although the documentation is mostly copied from existing recipes and may need some clarification.
The recipe does not support a versionPattern – I’m not sure how to handle it when version isn’t provided, which could be useful for example if someone wanted to check if any ancestry is a snapshot or release candidate.
Have you considered any alternatives or workarounds?
The current ParentPomInsight only allows checking the direct parent, so you would have to list all possible values instead.
Checklist
- [x] I've added unit tests to cover both positive and negative cases
- [x] I've read and applied the recipe conventions and best practices
- [x] I've used the IntelliJ IDEA auto-formatter on affected files – yes, except I reverted the change of indentation the
versionfield multi-line description.
@sambsnyd this one is recursive, but maybe I could make ParentPomInsight optionally recursive as well instead?
(also using SearchResult.found seems simpler than what I did here)
Still catch up from last week; was great to meet briefly at Devoxx! Indeed seems best to make the existing ParentPomInsight recursive as well; judging by how that's used we should be fine to just change that by default, or at least until someone comes along with a convincing case to be selective.
Feel free to use this same pull request; we'll squash merge anyway, and I imagine it's mostly moving over your test case and introducing a while loop. Thanks again!
@timtebeek & @sambsnyd, I have updated the PR and its description accordingly. I made it recursive by default as suggested.
Great to see how thorough your are with your tests when adapting an existing recipe!
I try to follow a TDD approach, I find it much more enjoyable than writing tests after, and I also find parameterized tests are a very nice way to check many different cases while staying fairly easy to implement and understand.