Add RemoveRedundantDependencyVersions.Skip marker#6852
Draft
Jenson3210 wants to merge 7 commits intomainfrom
Draft
Add RemoveRedundantDependencyVersions.Skip marker#6852Jenson3210 wants to merge 7 commits intomainfrom
Jenson3210 wants to merge 7 commits intomainfrom
Conversation
retainExplicitVersion option to UpgradeDependencyVersion
timtebeek
reviewed
Mar 2, 2026
rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java
Outdated
Show resolved
Hide resolved
Comment on lines
309
to
312
| doAfterVisit(new RemoveRedundantDependencyVersions(null, null, null, retainVersions).getVisitor()); | ||
| doAfterVisit(upgradeManagedDependency); | ||
| maybeUpdateModel(); | ||
| doAfterVisit(new RemoveRedundantDependencyVersions(null, null, null, null).getVisitor()); |
Contributor
There was a problem hiding this comment.
Is there not the possibility that their explicit versions will be wiped out anyway at some point if something like this is reached (possibly even for a different invocation of UpgradeDependencyVersion that is unrelated)?
retainExplicitVersion option to UpgradeDependencyVersionWhen set to true, this flag prevents the recipe from removing explicit version tags that match the parent-managed version after an upgrade. This is useful when organizations want to keep version tags in child POMs for clarity or policy reasons. A backward-compatible constructor is preserved alongside the new @JsonCreator constructor that accepts the additional parameter. Fixes moderneinc/customer-requests#1942
Co-authored-by: Tim te Beek <tim@moderne.io>
Remove the retainExplicitVersion field, constructors, and conditional logic from UpgradeDependencyVersion in favor of a more flexible Skip marker on RemoveRedundantDependencyVersions.
Add a Skip marker that can be placed on a Maven dependency tag to prevent RemoveRedundantDependencyVersions from removing its explicit <version>. This allows other recipes to preserve intentionally set versions by adding the marker before RemoveRedundantDependencyVersions runs. Fixes moderneinc/customer-requests#1942
Remove unnecessary expectedCyclesThatMakeChanges and add mixed-state test that verifies the Skip marker only affects the marked dependency while unmarked dependencies still have their redundant versions removed.
ad39cb7 to
a4a176d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Skipmarker nested class inRemoveRedundantDependencyVersionsthat prevents version removal when present on a dependency tagisDependencyTag()andisManagedDependencyTag()branches ofvisitTagretainExplicitVersionflag approach in favor of this marker-based designProblem
When
UpgradeDependencyVersionupgrades a dependency to match the parent-managed version,RemoveRedundantDependencyVersionssubsequently removes the explicit<version>tag. In some cases, users want to retain the explicit version even when it matches the managed version, but there was no way for an earlier recipe to signal this intent.Solution
Instead of adding a configuration flag to
UpgradeDependencyVersion, introduce aRemoveRedundantDependencyVersions.Skipmarker that any recipe can place on a<dependency>tag. WhenRemoveRedundantDependencyVersionsencounters this marker, it skips version removal for that tag. This keeps the mechanism generic and avoids adding recipe-specific boolean flags.Test plan
RemoveRedundantDependencyVersionsTesttests pass (56 tests)SkipMarkernested test class with two tests:dependencyWithSkipMarkerKeepsVersion- version preserved when marker presentdependencyWithoutSkipMarkerHasVersionRemoved- default behavior unchangedUpgradeDependencyVersionTesttests passFixes moderneinc/customer-requests#1942