UpgradeParentVersion: option to only upgrade external parents

[DidierLoiseau]

What's changed?

Added an onlyExternal optional parameter to UpgradeParentVersion in order to only perform an upgrade when the parent is external to the repository.

Had to adapt ChangeParentPom, which is used under the hood, because it wasn’t taking into account that an absent <relativePath> really means ../pom.xml, not null or "". I also changed the logic so that, if newRelativePath is the empty string, it will replace an existing tag with <relativePath /> (i.e. same as if it were adding the tag).

What's your motivation?

When the parent is in the same repository as the child, their versions are usually managed together, e.g. by the Maven Release Plugin or the Git Commit Id Maven Plugin.

This change allows to perform an upgrade of parents using wildcards for the groupId/artifactId while making sure that it will not try to upgrade references to parents located in the same repository.

For it to work properly, however, you need to properly specify <relativePath /> (or <relativePath></relativePath>) when the parent is located elsewhere. As people may forget about it (even though the IDE or Maven would warn about that), I didn’t make it the default. In fact, some tests would have been affected as well.

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

My changes to ChangeParentPom didn’t break any test, which was a bit surprising, so I added 2 parameterized tests to cover different combinations of relativePath.

Have you considered any alternatives or workarounds?

Currently we are only upgrading parents with specific Maven coordinates, due to this limitation. We can’t have an “upgrade all parents” recipe that wouldn’t break multi-module projects (we are using the Git Commit Id Maven Plugin so the parent version in the pom.xml must not be touched for local poms)

Any additional context

The new Parent.DEFAULT_RELATIVE_PATH constant is only used by ChangeParentPom for now. I think it could make sense if it was used as a default, but I’m not sure the current Jackson configuration makes a difference between an absent tag and an empty tag, so this would need to be asserted first. Xml.Tag appears to handle it properly.

I also noticed that ChangeTagValueVisitor keeps the spaces in <relativePath /> when expanding it (so it becomes <relativePath >value</relativePath>), which I find a bit strange, so I wrote the tests without space in the original pom.

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 – as usual: no. When I do this, the whole file often gets changed. I did it for the existing multiModuleRelativePath() and multiModuleRelativePathChangeChildren() tests, whose indentation was broken, and to the tests I added. IntelliJ also warns about trailing whitespace characters in most text blocks of ChangeParentPomTest, which would get removed by formatting the file.

[ammachado]

Looks like this also fix part of

• External parent versions being overridden when using IncrementProjectVersion #4607.

[timtebeek]

Great measured improvement, thanks! Love how thorough you are with tests. I know I've said that before, but it really helps build confidence in the changes you propose.

[timtebeek]

You've also made me wonder if we should add to our Maven best practices recipes a recipe that makes explicit the now implied or forgotten <relativePath, as we similarly do for a number of other tags already.

rewrite/rewrite-maven/src/main/resources/META-INF/rewrite/maven.yml

Lines 18 to 27 in 8cd654c

	name: org.openrewrite.maven.BestPractices
	displayName: Apache Maven best practices
	description: Applies best practices to Maven POMs.
	recipeList:
	- org.openrewrite.maven.cleanup.ExplicitPluginGroupId
	- org.openrewrite.maven.cleanup.ExplicitPluginVersion
	- org.openrewrite.maven.cleanup.PrefixlessExpressions
	- org.openrewrite.maven.OrderPomElements
	- org.openrewrite.maven.RemoveDuplicateDependencies
	- org.openrewrite.maven.RemoveRedundantDependencyVersions