Skip to content

UpgradeDependencyVersion (Gradle): respect shared version variables #7491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MBoegers merged 4 commits into from
May 5, 2026
Merged

UpgradeDependencyVersion (Gradle): respect shared version variables #7491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85df2be
Add failing reproducers for shared-variable corruption (customer-requ…
MBoegers Apr 27, 2026
3456fe3
Do not corrupt shared version variables when new version does not res…
MBoegers Apr 27, 2026
5ac35b3
Simplify scanner: single trait walk, drop redundant gatherVariables a…
MBoegers Apr 27, 2026
ed13c8f
Merge remote-tracking branch 'origin/main'
MBoegers Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 117 additions & 70 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,21 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);

// Check for any dependency (single or varargs with literal strings)
// Resolve the multi-dependency once and apply the recipe's matcher inline. The
// unfiltered pass also records variable usages for unmatched neighbours so a
// shared variable backing dependencies the upgrade does not target is detected
// before we corrupt those neighbours.
GradleMultiDependency.matcher()
.matcher(new DependencyMatcher(groupId, artifactId, getVersionComparator()))
.get(getCursor())
.ifPresent(multiDependency ->
multiDependency.forEach(dep -> scanDependency(dep, ctx)));
.ifPresent(multiDependency -> {
DependencyMatcher matcher = new DependencyMatcher(groupId, artifactId, getVersionComparator());
multiDependency.forEach(dep -> {
trackVariableUsage(dep);
if (matcher.matches(dep.getGroupId(), dep.getArtifactId())) {
scanDependency(dep, ctx);
}
});
});

new SpringDependencyManagementPluginEntry.Matcher()
.groupId(groupId)
Expand All @@ -202,6 +211,26 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
return m;
}

private void trackVariableUsage(GradleDependency gradleDependency) {
String versionVariableName = gradleDependency.getVersionVariable();
if (versionVariableName == null) {
return;
}
String depGroupId = gradleDependency.getGroupId();
String depArtifactId = gradleDependency.getArtifactId();
if (depGroupId == null || depArtifactId == null) {
return;
}
GroupArtifact ga = new GroupArtifact(depGroupId, depArtifactId);
Set<String> configs = acc.variableNames
.computeIfAbsent(versionVariableName, k -> new HashMap<>())
.computeIfAbsent(ga, k -> new HashSet<>());
String configName = gradleDependency.getConfigurationName();
if (configName != null) {
configs.add(configName);
}
}

private void scanSpringDependencyManagementEntry(SpringDependencyManagementPluginEntry entry, ExecutionContext ctx) {
String entryGroup = entry.getGroup();
for (String entryArtifact : entry.getArtifacts()) {
Expand Down Expand Up @@ -242,7 +271,6 @@ private void scanSpringDependencyManagementEntry(SpringDependencyManagementPlugi
* Scans a single dependency and records its information for later processing.
*/
private void scanDependency(GradleDependency gradleDependency, ExecutionContext ctx) {
gatherVariables(gradleDependency);
String groupId = gradleDependency.getGroupId();
String artifactId = gradleDependency.getArtifactId();

Expand Down Expand Up @@ -287,25 +315,6 @@ private boolean shouldResolveVersion(String declaredGroupId, String declaredArti
new DependencyMatcher(groupId, artifactId, null).matches(declaredGroupId, declaredArtifactId);
}

/**
* Gathers version variable names for dependencies
*/
private void gatherVariables(GradleDependency gradleDependency) {
String versionVariableName = gradleDependency.getVersionVariable();
if (versionVariableName == null) {
return;
}

String groupId = gradleDependency.getGroupId();
String artifactId = gradleDependency.getArtifactId();

if (shouldResolveVersion(groupId, artifactId)) {
J.MethodInvocation method = gradleDependency.getTree();
acc.variableNames.computeIfAbsent(versionVariableName, it -> new HashMap<>())
.computeIfAbsent(new GroupArtifact(groupId, artifactId), it -> new HashSet<>())
.add(method.getSimpleName());
}
}
};
}

Expand Down Expand Up @@ -449,19 +458,15 @@ public Properties visitFile(Properties.File file, ExecutionContext ctx) {
@Override
public org.openrewrite.properties.tree.Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) {
if (acc.versionPropNameToGA.containsKey(entry.getKey())) {
GroupArtifact ga = acc.versionPropNameToGA.get(entry.getKey()).keySet().stream().findFirst().orElse(null);
if (ga == null || !dependencyMatcher.matches(ga.getGroupId(), ga.getArtifactId())) {
return entry;
}
Object result = acc.gaToNewVersion.get(ga);
if (result == null || result instanceof Exception) {
String agreedVersion = safeUpdatedVersion(entry.getKey(), dependencyMatcher, acc, null, ctx);
if (agreedVersion == null) {
return entry;
}
VersionComparator versionComparator = getVersionComparator();
if (versionComparator == null) {
return entry;
}
Optional<String> finalVersion = versionComparator.upgrade(entry.getValue().getText(), singletonList((String) result));
Optional<String> finalVersion = versionComparator.upgrade(entry.getValue().getText(), singletonList(agreedVersion));
return finalVersion.map(v -> entry.withValue(entry.getValue().withText(v))).orElse(entry);
}
return entry;
Expand All @@ -472,6 +477,64 @@ public org.openrewrite.properties.tree.Properties visitEntry(Properties.Entry en
return Semver.validate(StringUtils.isBlank(newVersion) ? "latest.release" : newVersion, versionPattern).getValue();
}

/**
* Returns the agreed-upon resolved version when it is safe to update the shared variable,
* or null otherwise. A variable can only be updated when every dependency using it is
* targeted by the recipe's matcher AND every targeted dependency resolves to the same
* concrete version. Otherwise the matched dependency is detached to a literal version
* via {@code updateDependency} and the variable is left untouched so other neighbours
* sharing it stay on a working version.
*/
private @Nullable String safeUpdatedVersion(
String varName,
DependencyMatcher dependencyMatcher,
DependencyVersionState acc,
@Nullable GradleProject gradleProject,
ExecutionContext ctx) {
Map<GroupArtifact, Set<String>> usages = acc.variableNames.get(varName);
if (usages == null || usages.isEmpty()) {
return null;
}
DependencyVersionSelector selector = null;
String agreedVersion = null;
for (Map.Entry<GroupArtifact, Set<String>> entry : usages.entrySet()) {
GroupArtifact ga = entry.getKey();
if (!dependencyMatcher.matches(ga.getGroupId(), ga.getArtifactId())) {
return null;
}
Object cached = acc.gaToNewVersion.get(ga);
if (cached instanceof Exception) {
return null;
}
String selected;
if (cached instanceof String) {
selected = (String) cached;
} else {
// Not cached: happens for GAs the matcher targets but which were not encountered
// in a regular dependencies {} block (only via a shared variable). Resolve now.
Set<String> configs = entry.getValue();
String configName = configs == null ? null : configs.stream().findFirst().orElse(null);
if (selector == null) {
selector = new DependencyVersionSelector(metadataFailures, gradleProject, null);
}
try {
selected = selector.select(ga, configName, newVersion, versionPattern, ctx);
} catch (MavenDownloadingException e) {
return null;
}
}
if (selected == null) {
return null;
}
if (agreedVersion == null) {
agreedVersion = selected;
} else if (!agreedVersion.equals(selected)) {
return null;
}
}
return agreedVersion;
}

@RequiredArgsConstructor
private class UpdateGradle extends JavaVisitor<ExecutionContext> {
final DependencyVersionState acc;
Expand Down Expand Up @@ -630,40 +693,19 @@ public J postVisit(J tree, ExecutionContext ctx) {
if (!acc.variableNames.containsKey(variableName)) {
return prop.getTree();
}

Map.Entry<GroupArtifact, Set<String>> gaWithConfigs =
acc.variableNames.get(variableName).entrySet().iterator().next();

try {
GroupArtifact ga = gaWithConfigs.getKey();
DependencyVersionSelector dependencyVersionSelector =
new DependencyVersionSelector(metadataFailures, gradleProject, null);

String selectedVersion;
try {
selectedVersion = dependencyVersionSelector.select(ga, null, newVersion, versionPattern, execCtx);
} catch (MavenDownloadingException e) {
if (!gaWithConfigs.getValue().contains("classpath")) {
throw e;
}
selectedVersion = dependencyVersionSelector.select(ga, "classpath", newVersion, versionPattern, execCtx);
}
if (selectedVersion == null) {
String selectedVersion = safeUpdatedVersion(variableName, dependencyMatcher, acc, gradleProject, execCtx);
if (selectedVersion == null) {
return prop.getTree();
}
VersionComparator versionComparator = getVersionComparator();
if (versionComparator != null) {
Optional<String> finalVersion = versionComparator.upgrade(prop.getValue(), singletonList(selectedVersion));
if (!finalVersion.isPresent()) {
return prop.getTree();
}
VersionComparator versionComparator = getVersionComparator();
if (versionComparator != null) {
Optional<String> finalVersion = versionComparator.upgrade(prop.getValue(), singletonList(selectedVersion));
if (!finalVersion.isPresent()) {
return prop.getTree();
}
selectedVersion = finalVersion.get();
}
return prop.withValue(selectedVersion).getTree();
} catch (MavenDownloadingException e) {
// No change on error
return prop.getTree();
selectedVersion = finalVersion.get();
}
return prop.withValue(selectedVersion).getTree();
})
.visitNonNull(cu, ctx);
}
Expand Down Expand Up @@ -711,16 +753,12 @@ public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ex
if (!(a.getVariable() instanceof J.Identifier)) {
return a;
}
Map<GroupArtifact, Set<String>> groupArtifactSetMap = acc.versionPropNameToGA.get("gradle." + a.getVariable());
String varName = "gradle." + a.getVariable();
// Guard to ensure that an unsupported notation doesn't throw an exception
if (groupArtifactSetMap == null) {
if (!acc.versionPropNameToGA.containsKey(varName)) {
return a;
}
GroupArtifact ga = groupArtifactSetMap.entrySet().stream().findFirst().map(Map.Entry::getKey).orElse(null);
if (ga == null) {
return a;
}
String newVersion = (String) acc.gaToNewVersion.get(ga);
String newVersion = safeUpdatedVersion(varName, dependencyMatcher, acc, gradleProject, executionContext);
if (newVersion == null) {
return a;
}
Expand Down Expand Up @@ -785,7 +823,16 @@ private J.MethodInvocation updateDependency(GradleDependency dependency, Executi
// Handle variable references
String versionVariable = dependency.getVersionVariable();
if (versionVariable != null) {
// Variable updates are handled separately
if (safeUpdatedVersion(versionVariable, dependencyMatcher, acc, gradleProject, ctx) != null) {
// Variable is safe to update — leave the call site referencing it; postVisit / UpdateProperties will rewrite the variable.
return dependency.getTree();
}
// The variable is shared with another dependency that this upgrade does not target,
// or the new version does not resolve uniformly across all sharers. Detach this
// call site to a literal version so its neighbours stay on a working version.
if (scanResult instanceof String) {
return dependency.withDeclaredVersion((String) scanResult).getTree();
}
return dependency.getTree();
}

Expand Down
26 changes: 21 additions & 5 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1240,12 +1240,28 @@ public GradleDependency withDeclaredVersion(@Nullable String newVersion) {
updated = m.withArguments(ListUtils.mapFirst(m.getArguments(),
arg -> ChangeStringLiteral.withStringValue((J.Literal) arg, DependencyNotation.toStringNotation(updatedDep))));
} else if (dep == null && isMultiComponentDefinition(m.getArguments())) {
// Multi-component form: update literal version; variable versions are handled by UpdateProperties/UpdateVariable
if (m.getArguments().size() >= 3 && m.getArguments().get(2) instanceof J.Literal) {
String currentVersion = (String) ((J.Literal) m.getArguments().get(2)).getValue();
if (!newVersion.equals(currentVersion)) {
// Multi-component form: update literal version, or detach a variable reference to a literal.
if (m.getArguments().size() >= 3) {
Expression versionArg = m.getArguments().get(2);
if (versionArg instanceof J.Literal) {
String currentVersion = (String) ((J.Literal) versionArg).getValue();
if (!newVersion.equals(currentVersion)) {
updated = m.withArguments(ListUtils.map(m.getArguments(), (i, arg) ->
i == 2 ? ChangeStringLiteral.withStringValue((J.Literal) arg, newVersion) : arg));
}
} else if (versionArg instanceof J.Identifier) {
String delimiter = "\"";
if (m.getArguments().get(1) instanceof J.Literal) {
String src = ((J.Literal) m.getArguments().get(1)).getValueSource();
if (src != null && !src.isEmpty()) {
delimiter = src.substring(0, 1);
}
}
J.Literal replacement = new J.Literal(
Tree.randomId(), versionArg.getPrefix(), versionArg.getMarkers(),
newVersion, delimiter + newVersion + delimiter, null, JavaType.Primitive.String);
updated = m.withArguments(ListUtils.map(m.getArguments(), (i, arg) ->
i == 2 ? ChangeStringLiteral.withStringValue((J.Literal) arg, newVersion) : arg));
i == 2 ? replacement : arg));
}
}
}
Expand Down
Loading
⚡