Modify default behaviour of MavenVisitor::isPluginTag() to search everywhere, not just build

[dawngerpony]

What's changed?

• Modify the default behaviour of MavenVisitor::isPluginTag() to search everywhere, not just in the <build><plugins> section. Recipes that use this method to modify plugin definitions will now find that their pluginManagement sections are being updated too.
• Add a few more tests to verify the new behaviour.

What's your motivation?

We need this behaviour in our own recipes and we'd like to push it down the stack, because I thought others would find it useful too.

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

Not any more since the review.

Anyone you would like to review specifically?

@timtebeek perhaps?

Have you considered any alternatives or workarounds?

I don't see any reasonable alternatives, other than adding the matching logic to every recipe which needs it.

Any additional context

None.

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

[timtebeek]

Thanks for getting this started @dawngerpony ! Reminds me of this change we did a while back

• Also update Maven managed dependency by default in ChangeDependencyGroupIdAndArtifactId #3962

I should probably have been consistent at the time to roll that out for plugins as well; thanks for picking that up now.

For dependencies we ended up with updating dependencies wherever they are found by default; with a new option only to disable that behaviour. The motivation being that it's less surprising to users: then want to change a dependency; not have to remember to also change managed dependencies, or plugin dependencies, or profile managed plugin dependencies, and so on...

I'm thinking we can do the same here: by default update all locations where a plugin might be found, and maybe even leave out the option to limit that until we get specific complaints. To that end I'm wondering if we can simplify the implementation here by updating this specific matcher to use for instance //plugins/plugin.

rewrite/rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java

Line 43 in d25357f

static final XPathMatcher PLUGIN_MATCHER = new XPathMatcher("/project/*/plugins/plugin");

What would be your thoughts to such an approach? I know you've been careful not to make any breaking changes, but in this case I think the gain in functionality is tolerable.

[dawngerpony]

What would be your thoughts to such an approach? I know you've been careful not to make any breaking changes, but in this case I think the gain in functionality is tolerable.

If you're on board then I'm on board! Would you even recommend that we literally just update isPlugin() to check the new matcher? That would most certainly modify the default behaviour.

[timtebeek]

If you're on board then I'm on board! Would you even recommend that we literally just update isPlugin() to check the new matcher? That would most certainly modify the default behaviour.

That's what I was eyeing this afternoon indeed, as I had trouble thinking of use cases where folks would want to to do one but not the other, without resorting to a custom recipe.

[timtebeek]

Thanks a lot! Good to be consistent, with less code to boot.

[knutwannheden]

Not strictly related to this PR, but should we possibly also here match plugin dependencies in profiles?

Suggested change

	static final XPathMatcher PLUGIN_DEPENDENCY_MATCHER = new XPathMatcher("/project/*/plugins/plugin/dependencies/dependency");
	static final XPathMatcher PLUGIN_DEPENDENCY_MATCHER = new XPathMatcher("//plugins/plugin/dependencies/dependency");

[knutwannheden]

Note: The corresponding isPluginDependencyTag() method is being used by the UpgradeDependencyVersion recipe.

[timtebeek]

Thanks! Followed up with cfed3dc just now.