Skip to content

Prevent UpgradeDependencyVersion from downgrading ext property versions#7290

Merged
steve-aom-elliott merged 1 commit intomainfrom
fix-upgrade-dep-version-property-downgrade
Apr 6, 2026
Merged

Prevent UpgradeDependencyVersion from downgrading ext property versions#7290
steve-aom-elliott merged 1 commit intomainfrom
fix-upgrade-dep-version-property-downgrade

Conversation

@steve-aom-elliott
Copy link
Copy Markdown
Contributor

Summary

  • The postVisit ExtraProperty update path in UpgradeDependencyVersion resolved a candidate version via DependencyVersionSelector.select() using only a GroupArtifact (no current version), then applied it unconditionally — unlike the literal-version path and the gradle.properties path, which both use VersionComparator.upgrade() to reject downgrades
  • Added the same VersionComparator.upgrade() guard to the ext property path, consistent with the other two paths in the same file
  • This prevents incorrect behavior when a composite recipe includes an UpgradeDependencyVersion targeting an older major version range (e.g. 4.x) while the ext property already holds a newer version (e.g. 5.x)

Test plan

  • Added doesNotDowngradeExtPropertyVersion test — ext property at 30.1.1-jre is not downgraded when recipe targets 29.0-jre
  • All existing doesNotDowngrade* tests pass
  • All existing ext property upgrade tests pass (upgradesVersionInExtBlock, upgradesVersionInExtSubscriptNotation, upgradesVersionInExtBlockSetMethod)

The postVisit ExtraProperty update path in UpgradeDependencyVersion
resolved a candidate version via DependencyVersionSelector.select()
using only a GroupArtifact (no current version), then applied it
unconditionally. Unlike the literal-version and gradle.properties
paths which both use VersionComparator.upgrade() to reject downgrades,
this path had no such check.

This caused incorrect behavior when a composite recipe included an
UpgradeDependencyVersion targeting an older major version range (e.g.
4.x) while the ext property already held a newer version (e.g. 5.x).
The property would be silently downgraded.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Apr 6, 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 labels Apr 6, 2026
@steve-aom-elliott steve-aom-elliott moved this from In Progress to Ready to Review in OpenRewrite Apr 6, 2026
@steve-aom-elliott steve-aom-elliott merged commit 23c1afd into main Apr 6, 2026
1 check passed
@steve-aom-elliott steve-aom-elliott deleted the fix-upgrade-dep-version-property-downgrade branch April 6, 2026 20:43
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Apr 6, 2026
Jenson3210 added a commit that referenced this pull request Apr 7, 2026
… is unresolvable

DependencyVersionSelector.select() can return null when no matching version
is found. The downgrade prevention code added in #7290 passes this null into
versionComparator.upgrade() via singletonList(selectedVersion), which then
calls RELEASE_PATTERN.matcher(null) and throws NullPointerException.

Add a null guard consistent with the pattern used elsewhere in the same file
(e.g., line 480).

Fixes downstream failure in moderneinc/rewrite-spring.
timtebeek pushed a commit that referenced this pull request Apr 7, 2026
… is unresolvable (#7299)

DependencyVersionSelector.select() can return null when no matching version
is found. The downgrade prevention code added in #7290 passes this null into
versionComparator.upgrade() via singletonList(selectedVersion), which then
calls RELEASE_PATTERN.matcher(null) and throws NullPointerException.

Add a null guard consistent with the pattern used elsewhere in the same file
(e.g., line 480).

Fixes downstream failure in moderneinc/rewrite-spring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants