Fix UpgradeDependencyVersion not upgrading parent dependencyManagement after ChangeDependency in multi-module projects#8155
Draft
timtebeek wants to merge 1 commit into
Draft
Fix UpgradeDependencyVersion not upgrading parent dependencyManagement after ChangeDependency in multi-module projects#8155timtebeek wants to merge 1 commit into
timtebeek wants to merge 1 commit into
Conversation
…t after ChangeDependency in multi-module projects Two distinct multi-module bugs caused the chained `ChangeDependency` -> `UpgradeDependencyVersion` scenario from #8145 to fail: 1. `UpdateMavenModel` discarded a parent POM's own valid model update when re-resolving one of its aggregated child modules threw. After `ChangeDependency` renames a managed dependency in the parent, the child module still references the old (now unmanaged) coordinates and cannot be resolved mid-recipe. The resulting `MavenDownloadingExceptions` caused the entire parent update to be thrown away, leaving the parent's resolved model stale, so the following `UpgradeDependencyVersion` could not find the managed dependency to upgrade. Module re-resolution is now best-effort: a failing module keeps its previous resolution (it is refreshed when its own source file is updated) and no longer invalidates the parent's update. 2. `ChangeDependencyGroupIdAndArtifactId` added a spurious explicit `<version>` to a child dependency whose version is managed by a local parent when the dependency uses `provided` (or `test`) scope. The managed-dependency check used `Scope.isInClasspathOf`, which models transitive classpath visibility and returns `false` for `provided`/`provided` and `test`/`test`. Version management applies whenever coordinates match regardless of scope, so an identical scope now correctly counts as managed.
steve-aom-elliott
approved these changes
Jun 30, 2026
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
| } | ||
|
|
||
| private boolean canAffectManagedDependency(MavenResolutionResult result, Scope scope, String groupId, String artifactId) { | ||
| // We're only going to be able to effect managed dependencies that are either direct or are brought in as direct via a local parent |
Contributor
There was a problem hiding this comment.
Should probably be the word affect rather than effect, but not worth fixing separately.
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
UpgradeDependencyVersiondoes not update a<dependencyManagement>entry in a parent POM followingChangeDependencyin the same run #8145. In a multi-module Maven project, chainingChangeDependencyfollowed byUpgradeDependencyVersionin the same run left the parent POM's<dependencyManagement>entry un-upgraded and added a spurious explicit<version>to the child. Two distinct bugs were responsible:1.
UpdateMavenModeldiscarded a parent's valid update when a child module failed to re-resolveAfter
ChangeDependencyrenames a managed dependency in the parent POM,maybeUpdateModel()re-resolves the parent — including, recursively, its aggregated child modules. The child still references the old (now unmanaged) coordinates, so it cannot resolve a version mid-recipe and throwsMavenDownloadingExceptions. That exception caused the entire parent update to be thrown away, leaving the parent's resolvedMavenResolutionResultstale. The subsequentUpgradeDependencyVersionthen couldn't find the managed dependency and left the parent at the old version.Module re-resolution is now best-effort: a module that can't be resolved keeps its previous resolution (it is refreshed when its own source file is updated later in the run) and no longer invalidates the parent POM's own valid update.
2.
ChangeDependencyGroupIdAndArtifactIdadded an explicit version toprovided/testdependencies managed by a local parentThe managed-dependency detection used
Scope.isInClasspathOf, which models transitive classpath visibility and returnsfalseforprovided/providedandtest/test. As a result, aprovidedchild dependency managed by aprovidedentry in a local parent was treated as unmanaged, and an explicit<version>was added. Maven applies a managed version whenever coordinates match regardless of scope, so an identical scope now correctly counts as managed.Testing
UpgradeDependencyVersionTest.changeDependencyThenUpgradeManagedVersionInParentOfMultiModulereproducing the exactUpgradeDependencyVersiondoes not update a<dependencyManagement>entry in a parent POM followingChangeDependencyin the same run #8145 scenario (parent upgrades to EE10, child stays version-less).ChangeDependencyGroupIdAndArtifactIdTest.providedDependencyManagedByLocalParentDoesNotGetExplicitVersioncovering bug #2 in isolation.:rewrite-maven:testsuite passes.