Preventing the addition of a version tag on a direct dependency for scenarios where the local parent had the managed dependency that we were also changing. Also prevented property creation that could cause breakages of other managed dependencies inherited from parent.

[steve-aom-elliott]

What's changed?

• When we're calling ChangeDependencyGroupIdAndArtifactId, where we'd also be updating the managed dependency, and the managed dependency lives in a parent POM (that is within the repo), it will now be prevented from also adding the version tag to the direct dependency because it should be getting inherited from the managed dependency in the parent.
• When we're calling ChangeDependencyGroupIdAndArtifactId or ChangeManagedDependencyGroupIdAndArtifactId, it will now prevent the creation of a local POM property when changing a tag value for groupId, artifactId or version, even if the resolved POM would have known about the property being used, provided the requested POM would not have known about it.

What's your motivation?

• When you had the managed dependency outside of the current POM but were going to change it as well, because the operations ran using doAfterVisit(..), the updated POM for the parent would not be visible at the time of deciding whether to add a version tag to your direct dependency.
• When changing the dependency's GAV, namely the groupId and artifactId, you're no longer truly representing the same dependency anymore, and so introducing a local property to override an inherited property could cause it to break other inherited managed dependencies or direct dependencies also relying on inherited properties. Note that this doesn't affect altering of already existing properties in use by multiple dependencies. That will have to be a different change.

Other considerations

• Ideally we'd also prevent changing properties in our current POM that would have effects on other dependencies, as that could potentially break them. Fully preventing property changes and instead swapping the placeholders in the dependency for actual values would be one approach, but not sure if we want to be more intelligent with this.

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

[steve-aom-elliott]

With regards to property changes for existing properties (which isn't covered currently), perhaps it makes sense to effectively say that all bets are off in terms of updating existing properties when you use ChangeDependencyGroupIdAndArtifactId or ChangeManagedDependencyGroupIdAndArtifactId specifically, as there are too many situations to contend with, and in those cases, opt for not modifying the properties but rather use the actual values on the changed dependency rather than placeholders.

[shanman190]

I feel like trying to reuse the property definitely has risks for doing harm. Even trying to match all of the currently used GAs to the target GA doesn't always pan out for cases like jackson-bom.

[steve-aom-elliott]

@shanman190 This last push is quick pass of making it so that if you ask for properties to not be changed (such as when we're running ChangeDependencyGroupIdAndArtifactId or ChangeManagedDependencyGroupIdAndArtifactId) that it also does not change the existing local POM properties.

[sambsnyd]

I don't like this doNotAlterAnyProperties flag. Perhaps the recipe should use the change tag visitor directly rather than this helper method if the behavior of this helper method is not desired.