Avoid duplicate dependencies in ChangeDependency - #8129
Merged
timtebeek merged 5 commits intoJul 10, 2026
Conversation
When the new coordinates were already declared directly and the existing declaration was at a lower version than the requested one, the recipe renamed the old dependency in place, producing two declarations of the same dependency. Now the old declaration is removed whenever an existing direct dependency already matches the new coordinates on groupId, artifactId, classifier, type, and scope, and the surviving declaration is upgraded in place to the requested version. Declarations that differ on classifier, type, or scope are left untouched as distinct dependencies.
…ching Address review feedback: - Dedupe now applies when only newGroupId or only newArtifactId changes, not just when both are provided. - Match duplicates on full renamed coordinates (groupId/artifactId plus classifier/type/scope) rather than assuming exact pre-filtering, and rename the helpers to make the intent explicit.
- Remove explanatory comments now covered by method/variable names - Collapse the dead first conjunct in dedupeEnabled - Delete shadowing effectiveNew* locals in the exclusion pass - Centralize the rename rule in renamedGroupId/renamedArtifactId helpers - Rely on ResolvedDependency.getType() and the null-safe Scope.fromName instead of hand-rolled defaulting
timtebeek
marked this pull request as ready for review
July 10, 2026 16:21
steve-aom-elliott
approved these changes
Jul 10, 2026
steve-aom-elliott
left a comment
steve-aom-elliott
left a comment
Contributor
There was a problem hiding this comment.
I think it ended up correct, it's still just denser logic than I think ideally we'd be able to describe it as. Not sure how to cleanly simplify further, though.
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
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
2 participants
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.
What's changed
ChangeDependencyGroupIdAndArtifactId(the Maven engine behindorg.openrewrite.java.dependencies.ChangeDependency) could produce a duplicate dependency: when the new coordinates were already declared directly at a lower version than the requested one, the recipe renamed the old declaration in place, leaving two declarations of the same dependency.The recipe already removed the old declaration when the existing copy was at a version
>=the requested one, but the version guard meant the lower-version case fell through to a rename and produced a duplicate.This change:
groupId+artifactId+classifier+type+scope) — independent of version.Why
Concretely, migrating a POM that declares both
jakarta.jws:jakarta.jws-api:3.0.0andjakarta.xml.ws:jakarta.xml.ws-api:3.0.1withChangeDependency(jws-api → xml.ws-api, version4.0.x) previously yielded twojakarta.xml.ws-apientries. It now dropsjakarta.jws-apiand upgrades the existingjakarta.xml.ws-apito4.0.x— a single, correct entry.RemoveDuplicateDependencies.Tests
shouldAddNewIfDependencyAlreadyExistsInOlderVersion(which asserted the duplicate) intoshouldUpgradeExistingInPlaceIfDependencyAlreadyExistsInOlderVersion, asserting a single upgraded entry.shouldNotLeaveDuplicateWhenExistingNewDependencyIsAtLowerVersioncovering the jws/ws scenario.shouldNotDeduplicateWhenClassifierDiffersconfirming distinct coordinates are preserved.ChangeManagedDependencyGroupIdAndArtifactIdTestandUpgradeDependencyVersionTest.The Gradle
ChangeDependencypath is out of scope here, matching the Maven-only scope of the downstream compensation.