Skip to content

Skip redundant property override in child when parent declares same dependency#6955

Merged
sambsnyd merged 4 commits intomainfrom
MBoegers/upgrade-dep-local-parent
Mar 31, 2026
Merged

Skip redundant property override in child when parent declares same dependency#6955
sambsnyd merged 4 commits intomainfrom
MBoegers/upgrade-dep-local-parent

Conversation

@MBoegers
Copy link
Copy Markdown
Contributor

@MBoegers MBoegers commented Mar 12, 2026

Summary

  • When UpgradeDependencyVersion processes a managed dependency whose version is a property (e.g. ${flyway.version}), it now checks if a local parent POM also declares the same <dependency>. If so, the parent will be processed by the same recipe and will handle the property change — the child skips the redundant override.

  • Adds isDeclaredByLocalParent() helper that walks up the local parent chain checking for declared dependencies (not just managed ones), complementing the existing isManagedByLocalParent() method.

  • Fixes UpgradeDependencyVersion with overrideManagedVersion does not add property if <relativePath> is missing #6945

Test plan

  • UpgradeDependencyVersionTest.upgradeVersionDefinedViaImplicitPropertyInRemoteParentOfLocalParent_withoutRelativePath — parent and child both declare flyway-core, property only added to parent
  • Full UpgradeDependencyVersionTest suite passes (no regressions)
  • ChangePropertyValueTest and AddPropertyTest suites pass

…declares dependency

When UpgradeDependencyVersion processes a managed dependency whose
version is a property, and a local parent POM also declares the same
dependency, the parent will handle the property change. The child
should skip it to avoid a redundant property override.

Fixes #6945
@MBoegers MBoegers marked this pull request as ready for review March 12, 2026 12:29
@timtebeek timtebeek self-requested a review March 23, 2026 21:49
dm.getRequested().getVersion().length() - 1),
newerVersion, overrideManagedVersion, false).getVisitor());
// if a local parent also declares this dependency, it will handle the property change
if (!isDeclaredByLocalParent(d.getGroupId(), d.getArtifactId())) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe only skip if the version is null here? To avoid that we keep a version that then pins back a newer version from a parent? Or is that concern already handled separately?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes good call applied

Addresses review feedback: if the local parent declares the same
dependency with an explicit version, it will update the version tag
directly and won't handle the property change. The child must still
add the property override in that case.
@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Mar 31, 2026
@sambsnyd sambsnyd merged commit 22fd6f5 into main Mar 31, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Mar 31, 2026
@sambsnyd sambsnyd deleted the MBoegers/upgrade-dep-local-parent branch March 31, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

UpgradeDependencyVersion with overrideManagedVersion does not add property if <relativePath> is missing

3 participants