Skip to content

Route addToRootPom=true pins to the reactor root in UpgradeTransitiveDependencyVersion - #8319

Open
steve-aom-elliott wants to merge 1 commit into
mainfrom
feat/utdv-add-to-root-pom-multi-module
Open

Route addToRootPom=true pins to the reactor root in UpgradeTransitiveDependencyVersion#8319
steve-aom-elliott wants to merge 1 commit into
mainfrom
feat/utdv-add-to-root-pom-multi-module

Conversation

@steve-aom-elliott

Copy link
Copy Markdown
Contributor

Summary

UpgradeTransitiveDependencyVersion(..., addToRootPom=true) silently dropped every pin in a multi-module reactor whenever the vulnerable transitive was held only by a child module. Two things combined to hide the write:

  1. UpgradeTransitiveDependencyVersion.visitDocument returned early on the reactor root because the root itself has no matching transitive of its own (matchingDependencies.isEmpty()).
  2. AddManagedDependency.visitDocument's write gate (!Boolean.TRUE.equals(addToRootPom) || acc.rootPoms.contains(document)) refused to write on the child pom because only reactor-root poms are in acc.rootPoms.

Result: the recipe produced no changes at all, even though a child module clearly held the transitive.

Change

Give UpgradeTransitiveDependencyVersion a real scanner: for every pom that holds a matching transitive, walk up parentPomIsProjectPom() to find the reactor root's GAV and record the coordinate against it in a new AddManagedDependency.Scanned#reactorRootTransitives map.

In the visitor phase, branch on addToRootPom:

  • true — fire only on reactor roots (!parentPomIsProjectPom()) and pin every coordinate the scanner collected against that root's GAV.
  • null/false — existing behavior unchanged (per-pom matching with parent-preference dedup).

AddManagedDependency.visitDocument's guard already routes correctly when the root pom is visited (root is in acc.rootPoms), so no changes needed there.

Behavior change

Only the addToRootPom=true path is affected. The null/false path — the common case, including everything UpgradeTransitiveDependencyVersionTest covered before — is unchanged.

Callers that were relying on addToRootPom=true silently no-op-ing in reactors will now see the recipe produce edits.

Known scope limitation

If a child has its own competing <dependencyManagement> entry for the same coordinate, Maven's inheritance rule means the child's local entry shadows the parent's — the root-scoped pin won't take effect for that specific module until the child entry is removed. This PR does not touch child poms. Filed as follow-up.

Test plan

  • New addsManagedDependencyToReactorRootWhenAddToRootPomAndOnlyChildHoldsTheTransitive in UpgradeTransitiveDependencyVersionTest covers the reactor-root write.
  • Full UpgradeTransitiveDependencyVersionTest and AddManagedDependencyTest suites pass.
  • Full rewrite-maven:test module passes.

Draft — soliciting feedback on the accumulator shape and the child-cleanup boundary before finalizing.

…DependencyVersion

Previously addToRootPom=true dropped every pin in a multi-module reactor when the
transitive was held only by a child module. Two things combined to hide the write:
UpgradeTransitiveDependencyVersion bailed at the reactor root because the root
itself has no matching transitive of its own, and AddManagedDependency's write
gate refused to write on the child because the child was not in the rootPoms set.

Give UpgradeTransitiveDependencyVersion a real scanner: for every pom that holds
a matching transitive, walk up parentPomIsProjectPom to find the reactor root's
GAV and record the coordinate against it in AddManagedDependency.Scanned. In the
visitor phase, when addToRootPom=true, fire only on reactor roots and pin every
coordinate the scanner collected against that root. Non-root behavior stays
identical for addToRootPom=null/false.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jul 23, 2026
@steve-aom-elliott steve-aom-elliott added bug Something isn't working test provided Already replicated with a unit test, using JUnit pioneer's ExpectedToFail maven labels Jul 23, 2026
@steve-aom-elliott
steve-aom-elliott marked this pull request as ready for review July 23, 2026 19:52
@steve-aom-elliott steve-aom-elliott moved this from In Progress to Ready to Review in OpenRewrite Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maven test provided Already replicated with a unit test, using JUnit pioneer's ExpectedToFail

Projects

Status: Ready to Review

Development

Successfully merging this pull request may close these issues.

1 participant