From e042a67fd527565822c14fee690c7af143f982ef Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 26 Feb 2026 14:15:11 +0100 Subject: [PATCH 1/3] Preserve GString/StringTemplate structure in ChangeDependency when version is unchanged When `ChangeDependency` renames a dependency that uses a GString or Kotlin StringTemplate (e.g. `"group:artifact:${version}"`), and the version is not being changed, the GString/StringTemplate structure is now preserved instead of being collapsed to a literal with a pinned version. Previously, the GString and Kotlin StringTemplate code paths unconditionally applied `newVersion` without checking the `overrideManagedVersion` guard that the literal path already had. This caused recipes like the Spring Boot 4 migration to pin hardcoded versions on dependencies that used property references for their version (e.g. `${springBootVersion}`). The fix adds the same `overrideManagedVersion` guard to both the GString and Kotlin StringTemplate paths, and when only group/artifact changes (not version), updates only the literal prefix while preserving the interpolated structure. --- .../openrewrite/gradle/ChangeDependency.java | 46 +++++++++++++++---- .../gradle/ChangeDependencyTest.java | 4 +- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java index 738c595b0ff..e8bfa900530 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java @@ -302,7 +302,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte if (!StringUtils.isBlank(newArtifactId) && !updated.getArtifactId().equals(newArtifactId)) { updated = updated.withGav(updated.getGav().withArtifactId(newArtifactId)); } - if (!StringUtils.isBlank(newVersion)) { + if (!StringUtils.isBlank(newVersion) && (!StringUtils.isBlank(original.getVersion()) || Boolean.TRUE.equals(overrideManagedVersion))) { String resolvedVersion; try { resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, null) @@ -315,10 +315,23 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte } } if (original != updated) { - String replacement = DependencyNotation.toStringNotation(updated); - J.Literal newLiteral = literal.withValue(replacement) - .withValueSource(gstring.getDelimiter() + replacement + gstring.getDelimiter()); - m = m.withArguments(singletonList(newLiteral)); + if (Objects.equals(original.getVersion(), updated.getVersion())) { + // Version unchanged: preserve GString structure, only update the literal prefix + String oldGav = original.getGroupId() + ":" + original.getArtifactId(); + String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); + String oldValue = (String) literal.getValue(); + String updatedValue = oldValue.replace(oldGav, newGav); + J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); + m = m.withArguments(singletonList( + gstring.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) + )); + } else { + // Version changed: collapse GString to a literal + String replacement = DependencyNotation.toStringNotation(updated); + J.Literal newLiteral = literal.withValue(replacement) + .withValueSource(gstring.getDelimiter() + replacement + gstring.getDelimiter()); + m = m.withArguments(singletonList(newLiteral)); + } } } } @@ -595,7 +608,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte if (!StringUtils.isBlank(newArtifactId) && !updated.getArtifactId().equals(newArtifactId)) { updated = updated.withGav(updated.getGav().withArtifactId(newArtifactId)); } - if (!StringUtils.isBlank(newVersion)) { + if (!StringUtils.isBlank(newVersion) && (!StringUtils.isBlank(original.getVersion()) || Boolean.TRUE.equals(overrideManagedVersion))) { String resolvedVersion; try { resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, null) @@ -608,10 +621,23 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte } } if (original != updated) { - String replacement = DependencyNotation.toStringNotation(updated); - J.Literal newLiteral = literal.withValue(replacement) - .withValueSource(template.getDelimiter() + replacement + template.getDelimiter()); - m = m.withArguments(singletonList(newLiteral)); + if (Objects.equals(original.getVersion(), updated.getVersion())) { + // Version unchanged: preserve StringTemplate structure, only update the literal prefix + String oldGav = original.getGroupId() + ":" + original.getArtifactId(); + String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); + String oldValue = (String) literal.getValue(); + String updatedValue = oldValue.replace(oldGav, newGav); + J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); + m = m.withArguments(singletonList( + template.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) + )); + } else { + // Version changed: collapse StringTemplate to a literal + String replacement = DependencyNotation.toStringNotation(updated); + J.Literal newLiteral = literal.withValue(replacement) + .withValueSource(template.getDelimiter() + replacement + template.getDelimiter()); + m = m.withArguments(singletonList(newLiteral)); + } } } } diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java index 6f5503aa174..b91891aecda 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java @@ -264,7 +264,7 @@ implementation platform("commons-lang:commons-lang:${version}") def version = '2.6' dependencies { - implementation platform("org.apache.commons:commons-lang3:3.11") + implementation platform("org.apache.commons:commons-lang3:${version}") } """ ) @@ -574,7 +574,7 @@ void kotlinDslStringInterpolation() { dependencies { val commonsLangVersion = "2.6" - implementation("org.apache.commons:commons-lang3:3.11") + implementation("org.apache.commons:commons-lang3:${commonsLangVersion}") } """ ) From 64d4dd3edd4fea5f3b5acb607d85bce6f51fe7c7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 26 Feb 2026 22:42:31 +0100 Subject: [PATCH 2/3] Use gradle.properties for version in GString/StringTemplate tests Move version variables from inline definitions to gradle.properties in worksWithGString and kotlinDslStringInterpolation tests, making them closer to real-world usage patterns. The properties file remains unchanged, confirming the GString/StringTemplate structure is preserved. --- .../gradle/ChangeDependencyTest.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java index b91891aecda..bd47847672e 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java @@ -24,6 +24,7 @@ import static org.openrewrite.gradle.Assertions.buildGradle; import static org.openrewrite.gradle.Assertions.buildGradleKts; import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; +import static org.openrewrite.properties.Assertions.properties; class ChangeDependencyTest implements RewriteTest { @Override @@ -238,6 +239,12 @@ implementation platform("org.apache.commons:commons-lang3:3.11") void worksWithGString() { rewriteRun( spec -> spec.recipe(new ChangeDependency("commons-lang", "commons-lang", "org.apache.commons", "commons-lang3", "3.11.x", null, null, true)), + properties( + """ + commonsLangVersion=2.6 + """, + spec -> spec.path("gradle.properties") + ), buildGradle( """ plugins { @@ -248,9 +255,8 @@ void worksWithGString() { mavenCentral() } - def version = '2.6' dependencies { - implementation platform("commons-lang:commons-lang:${version}") + implementation platform("commons-lang:commons-lang:${commonsLangVersion}") } """, """ @@ -262,9 +268,8 @@ implementation platform("commons-lang:commons-lang:${version}") mavenCentral() } - def version = '2.6' dependencies { - implementation platform("org.apache.commons:commons-lang3:${version}") + implementation platform("org.apache.commons:commons-lang3:${commonsLangVersion}") } """ ) @@ -548,6 +553,12 @@ void kotlinDsl() { void kotlinDslStringInterpolation() { rewriteRun( spec -> spec.recipe(new ChangeDependency("commons-lang", "commons-lang", "org.apache.commons", "commons-lang3", "3.11.x", null, null, true)), + properties( + """ + commonsLangVersion=2.6 + """, + spec -> spec.path("gradle.properties") + ), buildGradleKts( """ plugins { @@ -558,8 +569,8 @@ void kotlinDslStringInterpolation() { mavenCentral() } + val commonsLangVersion: String by project dependencies { - val commonsLangVersion = "2.6" implementation("commons-lang:commons-lang:${commonsLangVersion}") } """, @@ -572,8 +583,8 @@ void kotlinDslStringInterpolation() { mavenCentral() } + val commonsLangVersion: String by project dependencies { - val commonsLangVersion = "2.6" implementation("org.apache.commons:commons-lang3:${commonsLangVersion}") } """ From 6d271cd22460de36dc6219dbc21bb5ba8635cdba Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 27 Feb 2026 16:27:24 +0100 Subject: [PATCH 3/3] Update version property in gradle.properties when preserving GString When ChangeDependency renames a GString dependency and resolves a new version, the version property in gradle.properties is now updated to match. This preserves the GString structure while ensuring the property value is correct for the new artifact. For example, renaming commons-lang:commons-lang to org.apache.commons:commons-lang3 with newVersion 3.11.x now: - Preserves "org.apache.commons:commons-lang3:${commonsLangVersion}" - Updates gradle.properties: commonsLangVersion=3.11 The composite visitor handles both Gradle build scripts and properties files in a single recipe cycle. --- .../openrewrite/gradle/ChangeDependency.java | 115 ++++++++++++------ .../gradle/ChangeDependencyTest.java | 6 + 2 files changed, 86 insertions(+), 35 deletions(-) diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java index e8bfa900530..2931e91435e 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java @@ -36,10 +36,13 @@ import org.openrewrite.kotlin.tree.K; import org.openrewrite.maven.MavenDownloadingException; import org.openrewrite.maven.table.MavenMetadataFailures; +import org.openrewrite.properties.PropertiesVisitor; +import org.openrewrite.properties.tree.Properties; import org.openrewrite.semver.DependencyMatcher; import org.openrewrite.semver.Semver; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; @@ -59,6 +62,9 @@ public class ChangeDependency extends Recipe { @EqualsAndHashCode.Exclude transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this); + @EqualsAndHashCode.Exclude + transient Map pendingPropertyUpdates = new ConcurrentHashMap<>(); + @Option(displayName = "Old groupId", description = "The old groupId to replace. The groupId is the first part of a dependency coordinate 'com.google.guava:guava:VERSION'. Supports glob expressions.", example = "org.openrewrite.recipe") @@ -161,7 +167,7 @@ public Validated validate() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new FindGradleProject(FindGradleProject.SearchCriteria.Marker).getVisitor(), new JavaIsoVisitor() { + TreeVisitor gradleVisitor = Preconditions.check(new FindGradleProject(FindGradleProject.SearchCriteria.Marker).getVisitor(), new JavaIsoVisitor() { final DependencyMatcher depMatcher = requireNonNull(DependencyMatcher.build(oldGroupId + ":" + oldArtifactId).getValue()); final DependencyMatcher existingMatcher = requireNonNull(DependencyMatcher.build(newGroupId + ":" + newArtifactId + (newVersion == null ? "" : ":" + newVersion)).getValue()); @@ -302,7 +308,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte if (!StringUtils.isBlank(newArtifactId) && !updated.getArtifactId().equals(newArtifactId)) { updated = updated.withGav(updated.getGav().withArtifactId(newArtifactId)); } - if (!StringUtils.isBlank(newVersion) && (!StringUtils.isBlank(original.getVersion()) || Boolean.TRUE.equals(overrideManagedVersion))) { + if (!StringUtils.isBlank(newVersion)) { String resolvedVersion; try { resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, null) @@ -315,22 +321,21 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte } } if (original != updated) { - if (Objects.equals(original.getVersion(), updated.getVersion())) { - // Version unchanged: preserve GString structure, only update the literal prefix - String oldGav = original.getGroupId() + ":" + original.getArtifactId(); - String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); - String oldValue = (String) literal.getValue(); - String updatedValue = oldValue.replace(oldGav, newGav); - J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); - m = m.withArguments(singletonList( - gstring.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) - )); - } else { - // Version changed: collapse GString to a literal - String replacement = DependencyNotation.toStringNotation(updated); - J.Literal newLiteral = literal.withValue(replacement) - .withValueSource(gstring.getDelimiter() + replacement + gstring.getDelimiter()); - m = m.withArguments(singletonList(newLiteral)); + // Always preserve GString structure, only update the literal prefix + String oldGav = original.getGroupId() + ":" + original.getArtifactId(); + String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); + String oldValue = (String) literal.getValue(); + String updatedValue = oldValue.replace(oldGav, newGav); + J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); + m = m.withArguments(singletonList( + gstring.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) + )); + // If version was resolved, schedule property update + if (updated.getVersion() != null && !Objects.equals(original.getVersion(), updated.getVersion())) { + String varName = extractVersionVariableName(strings); + if (varName != null) { + pendingPropertyUpdates.put(varName, updated.getVersion()); + } } } } @@ -608,7 +613,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte if (!StringUtils.isBlank(newArtifactId) && !updated.getArtifactId().equals(newArtifactId)) { updated = updated.withGav(updated.getGav().withArtifactId(newArtifactId)); } - if (!StringUtils.isBlank(newVersion) && (!StringUtils.isBlank(original.getVersion()) || Boolean.TRUE.equals(overrideManagedVersion))) { + if (!StringUtils.isBlank(newVersion)) { String resolvedVersion; try { resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, null) @@ -621,22 +626,21 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, ExecutionConte } } if (original != updated) { - if (Objects.equals(original.getVersion(), updated.getVersion())) { - // Version unchanged: preserve StringTemplate structure, only update the literal prefix - String oldGav = original.getGroupId() + ":" + original.getArtifactId(); - String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); - String oldValue = (String) literal.getValue(); - String updatedValue = oldValue.replace(oldGav, newGav); - J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); - m = m.withArguments(singletonList( - template.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) - )); - } else { - // Version changed: collapse StringTemplate to a literal - String replacement = DependencyNotation.toStringNotation(updated); - J.Literal newLiteral = literal.withValue(replacement) - .withValueSource(template.getDelimiter() + replacement + template.getDelimiter()); - m = m.withArguments(singletonList(newLiteral)); + // Always preserve StringTemplate structure, only update the literal prefix + String oldGav = original.getGroupId() + ":" + original.getArtifactId(); + String newGav = updated.getGroupId() + ":" + updated.getArtifactId(); + String oldValue = (String) literal.getValue(); + String updatedValue = oldValue.replace(oldGav, newGav); + J.Literal updatedLiteral = literal.withValue(updatedValue).withValueSource(updatedValue); + m = m.withArguments(singletonList( + template.withStrings(ListUtils.mapFirst(strings, s -> updatedLiteral)) + )); + // If version was resolved, schedule property update + if (updated.getVersion() != null && !Objects.equals(original.getVersion(), updated.getVersion())) { + String varName = extractVersionVariableName(strings); + if (varName != null) { + pendingPropertyUpdates.put(varName, updated.getVersion()); + } } } } @@ -720,5 +724,46 @@ private GradleProject updateGradleModel(GradleProject gp, ExecutionContext ctx) return gp; } }); + + return new TreeVisitor() { + @Override + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + if (tree instanceof Properties.File && !pendingPropertyUpdates.isEmpty()) { + return new PropertiesVisitor() { + @Override + public Properties.Entry visitEntry(Properties.Entry entry, ExecutionContext ctx) { + String version = pendingPropertyUpdates.get(entry.getKey()); + if (version != null && !version.equals(entry.getValue().getText())) { + return entry.withValue(entry.getValue().withText(version)); + } + return entry; + } + }.visit(tree, ctx); + } + return gradleVisitor.visit(tree, ctx); + } + }; + } + + private static @Nullable String extractVersionVariableName(List strings) { + if (strings.size() >= 2) { + J versionPart = strings.get(strings.size() - 1); + // Groovy GString: strings[1] is G.GString.Value wrapping J.Identifier + if (versionPart instanceof G.GString.Value) { + J tree = ((G.GString.Value) versionPart).getTree(); + if (tree instanceof J.Identifier) { + return ((J.Identifier) tree).getSimpleName(); + } + } + // Kotlin StringTemplate: strings[1] is K.StringTemplate.Expression wrapping J.Identifier + if (versionPart instanceof K.StringTemplate.Expression) { + J tree = ((K.StringTemplate.Expression) versionPart).getTree(); + if (tree instanceof J.Identifier) { + return ((J.Identifier) tree).getSimpleName(); + } + } + } + return null; } + } diff --git a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java index bd47847672e..d344eb01479 100644 --- a/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java +++ b/rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java @@ -243,6 +243,9 @@ void worksWithGString() { """ commonsLangVersion=2.6 """, + """ + commonsLangVersion=3.11 + """, spec -> spec.path("gradle.properties") ), buildGradle( @@ -557,6 +560,9 @@ void kotlinDslStringInterpolation() { """ commonsLangVersion=2.6 """, + """ + commonsLangVersion=3.11 + """, spec -> spec.path("gradle.properties") ), buildGradleKts(