diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java index 401b0a132b..3598149839 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeDependencyVersion.java @@ -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) @@ -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 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()) { @@ -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(); @@ -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()); - } - } }; } @@ -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 finalVersion = versionComparator.upgrade(entry.getValue().getText(), singletonList((String) result)); + Optional finalVersion = versionComparator.upgrade(entry.getValue().getText(), singletonList(agreedVersion)); return finalVersion.map(v -> entry.withValue(entry.getValue().withText(v))).orElse(entry); } return entry; @@ -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> usages = acc.variableNames.get(varName); + if (usages == null || usages.isEmpty()) { + return null; + } + DependencyVersionSelector selector = null; + String agreedVersion = null; + for (Map.Entry> 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 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 { final DependencyVersionState acc; @@ -630,40 +693,19 @@ public J postVisit(J tree, ExecutionContext ctx) { if (!acc.variableNames.containsKey(variableName)) { return prop.getTree(); } - - Map.Entry> 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 finalVersion = versionComparator.upgrade(prop.getValue(), singletonList(selectedVersion)); + if (!finalVersion.isPresent()) { return prop.getTree(); } - VersionComparator versionComparator = getVersionComparator(); - if (versionComparator != null) { - Optional 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); } @@ -711,16 +753,12 @@ public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ex if (!(a.getVariable() instanceof J.Identifier)) { return a; } - Map> 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; } @@ -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(); } diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java index 31e2492a28..478c195e91 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradleDependency.java @@ -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)); } } } diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeDependencyVersionTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeDependencyVersionTest.java index 44ebaf2dc4..02d99c59a7 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeDependencyVersionTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/UpgradeDependencyVersionTest.java @@ -884,6 +884,135 @@ void upgradesVariablesDefinedInExtraProperties() { ); } + @Test + void doesNotCorruptSharedKotlinValVariable() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("com.fasterxml.jackson.core", "jackson-annotations", "2.21", null)), + buildGradleKts( + """ + plugins { + `java-library` + } + + repositories { + mavenCentral() + } + + val jacksonVersion = "2.17.3" + + dependencies { + implementation("com.fasterxml.jackson.core", "jackson-annotations", jacksonVersion) + implementation("com.fasterxml.jackson.core", "jackson-core", jacksonVersion) + } + """, + """ + plugins { + `java-library` + } + + repositories { + mavenCentral() + } + + val jacksonVersion = "2.17.3" + + dependencies { + implementation("com.fasterxml.jackson.core", "jackson-annotations", "2.21") + implementation("com.fasterxml.jackson.core", "jackson-core", jacksonVersion) + } + """ + ) + ); + } + + @Test + void doesNotCorruptSharedExtProperty() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("com.fasterxml.jackson.core", "jackson-annotations", "2.21", null)), + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + ext { + jacksonVersion = "2.17.3" + } + + dependencies { + implementation "com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}" + implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + ext { + jacksonVersion = "2.17.3" + } + + dependencies { + implementation "com.fasterxml.jackson.core:jackson-annotations:2.21" + implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" + } + """ + ) + ); + } + + @Test + void doesNotCorruptSharedGradleProperty() { + rewriteRun( + spec -> spec.recipe(new UpgradeDependencyVersion("com.fasterxml.jackson.core", "jackson-annotations", "2.21", null)), + properties( + """ + jacksonVersion=2.17.3 + """, + spec -> spec.path("gradle.properties") + ), + buildGradle( + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation "com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}" + implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" + } + """, + """ + plugins { + id "java" + } + + repositories { + mavenCentral() + } + + dependencies { + implementation "com.fasterxml.jackson.core:jackson-annotations:2.21" + implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" + } + """ + ) + ); + } + @Test void upgradesVersionInExtSubscriptNotation() { rewriteRun(