Skip to content

Commit ccb7d23

Browse files
steve-aom-elliottsteve-aom-elliott
authored
ChangeDependency (when using glob) skips dependencies whose new coordinates don't resolve (#7321)
1 parent d9459d0 commit ccb7d23

4 files changed

Lines changed: 81 additions & 6 deletions

File tree

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/ChangeDependency.java‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,15 @@ public Validated<Object> validate() {
162162
));
163163
}
164164

165+
private boolean isGlobPattern() {
166+
return oldGroupId.contains("*") || oldGroupId.contains("?") ||
167+
oldArtifactId.contains("*") || oldArtifactId.contains("?");
168+
}
169+
165170
public static class Accumulator {
166171
Map<String, Object> versionVariableUpdates = new HashMap<>();
167172
Map<String, Set<GroupArtifact>> versionVariableUsages = new HashMap<>();
173+
Set<GroupArtifact> failedResolutions = new HashSet<>();
168174
}
169175

170176
@Override
@@ -232,7 +238,14 @@ private void resolveAndRecordVersion(String varName, J.MethodInvocation m, Gradl
232238
acc.versionVariableUpdates.put(varName, resolvedVersion);
233239
}
234240
} catch (MavenDownloadingException e) {
235-
acc.versionVariableUpdates.put(varName, e);
241+
if (isGlobPattern()) {
242+
acc.failedResolutions.add(new GroupArtifact(dep.getGroupId(), dep.getArtifactId()));
243+
// Don't overwrite a successful resolution with a failure from a different artifact
244+
// sharing the same version variable (e.g. hibernate-core resolved but hibernate-validator didn't)
245+
acc.versionVariableUpdates.putIfAbsent(varName, e);
246+
} else {
247+
acc.versionVariableUpdates.put(varName, e);
248+
}
236249
}
237250
}
238251
};
@@ -329,8 +342,8 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations
329342
return v;
330343
}
331344
Object scanResult = acc.versionVariableUpdates.get(v.getSimpleName());
332-
if (scanResult instanceof Exception) {
333-
return Markup.warn(v, (Exception) scanResult);
345+
if (scanResult instanceof MavenDownloadingException) {
346+
return Markup.warn(v, (MavenDownloadingException) scanResult);
334347
}
335348
if (scanResult instanceof String && v.getInitializer() instanceof J.Literal) {
336349
String resolvedVersion = (String) scanResult;
@@ -347,6 +360,10 @@ private boolean canUpdateVariable(String varName) {
347360
}
348361

349362
private J.MethodInvocation updateDependency(J.MethodInvocation m, GradleDependency dep, ExecutionContext ctx) {
363+
if (acc.failedResolutions.contains(new GroupArtifact(dep.getGroupId(), dep.getArtifactId()))) {
364+
return m;
365+
}
366+
350367
GradleDependency updated = dep;
351368

352369
if (!StringUtils.isBlank(newGroupId)) {
@@ -359,7 +376,7 @@ private J.MethodInvocation updateDependency(J.MethodInvocation m, GradleDependen
359376
String varName = dep.getVersionVariable();
360377
if (varName != null && !canUpdateVariable(varName)) {
361378
Object scanResult = acc.versionVariableUpdates.get(varName);
362-
if (scanResult instanceof Exception) {
379+
if (scanResult instanceof MavenDownloadingException) {
363380
return ((MavenDownloadingException) scanResult).warn(m);
364381
}
365382
if (scanResult instanceof String) {
@@ -485,8 +502,8 @@ public Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) {
485502
return entry;
486503
}
487504
Object scanResult = acc.versionVariableUpdates.get(entry.getKey());
488-
if (scanResult instanceof Exception) {
489-
return Markup.warn(entry, (Exception) scanResult);
505+
if (scanResult instanceof MavenDownloadingException) {
506+
return Markup.warn(entry, (MavenDownloadingException) scanResult);
490507
}
491508
if (scanResult instanceof String) {
492509
String resolvedVersion = (String) scanResult;
@@ -514,6 +531,9 @@ private boolean canUpdateVariable(String varName, DependencyMatcher depMatcher,
514531
if (!depMatcher.matches(ga.getGroupId(), ga.getArtifactId())) {
515532
return false;
516533
}
534+
if (acc.failedResolutions.contains(ga)) {
535+
return false;
536+
}
517537
}
518538
return true;
519539
}

‎rewrite-gradle/src/test/java/org/openrewrite/gradle/ChangeDependencyTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,4 +1104,43 @@ void isNotAcceptableForPlainTextWithGradleProjectMarker() {
11041104
.build())));
11051105
assertThat(visitor.isAcceptable(sourceFile, new InMemoryExecutionContext())).isFalse();
11061106
}
1107+
1108+
@Test
1109+
void doesNotChangeGroupIdWhenNewCoordinatesDontResolve() {
1110+
rewriteRun(
1111+
spec -> spec.recipe(new ChangeDependency("org.hibernate", "hibernate-*", "org.hibernate.orm", null, "6.0.x", null, null, true)),
1112+
buildGradle(
1113+
"""
1114+
plugins {
1115+
id "java-library"
1116+
}
1117+
1118+
repositories {
1119+
mavenCentral()
1120+
}
1121+
1122+
def hibernateVersion = '5.6.15.Final'
1123+
dependencies {
1124+
implementation "org.hibernate:hibernate-core:${hibernateVersion}"
1125+
implementation "org.hibernate:hibernate-validator:${hibernateVersion}"
1126+
}
1127+
""",
1128+
"""
1129+
plugins {
1130+
id "java-library"
1131+
}
1132+
1133+
repositories {
1134+
mavenCentral()
1135+
}
1136+
1137+
def hibernateVersion = '5.6.15.Final'
1138+
dependencies {
1139+
implementation "org.hibernate.orm:hibernate-core:6.0.2.Final"
1140+
implementation "org.hibernate:hibernate-validator:${hibernateVersion}"
1141+
}
1142+
"""
1143+
)
1144+
);
1145+
}
11071146
}

‎rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ public ChangeDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifac
114114
this.changeManagedDependency = changeManagedDependency;
115115
}
116116

117+
private boolean isGlobPattern() {
118+
return oldGroupId.contains("*") || oldGroupId.contains("?") ||
119+
oldArtifactId.contains("*") || oldArtifactId.contains("?");
120+
}
121+
117122
String displayName = "Change Maven dependency";
118123

119124
@Override
@@ -391,6 +396,9 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
391396
deferUpdate = true;
392397
}
393398
} catch (MavenDownloadingException e) {
399+
if (isGlobPattern()) {
400+
return tag;
401+
}
394402
return e.warn(tag);
395403
}
396404
}

‎rewrite-maven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String old
101101
this.versionPattern = versionPattern;
102102
}
103103

104+
private boolean isGlobPattern() {
105+
return oldGroupId.contains("*") || oldGroupId.contains("?") ||
106+
oldArtifactId.contains("*") || oldArtifactId.contains("?");
107+
}
108+
104109
@Override
105110
public Validated<Object> validate() {
106111
Validated<Object> validated = super.validate();
@@ -177,6 +182,9 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
177182
}
178183
}
179184
} catch (MavenDownloadingException e) {
185+
if (isGlobPattern()) {
186+
return tag;
187+
}
180188
return e.warn(t);
181189
}
182190
}

0 commit comments

Comments
 (0)
⚡