Skip to content

Commit e6daa51

Browse files
steve-aom-elliottsteve-aom-elliott
authored
Fix JavaSourceSet phantom diffs and use cached updates in all wrapping visitors (#7376)
1 parent 839bfb7 commit e6daa51

5 files changed

Lines changed: 68 additions & 60 deletions

File tree

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
337337
return new TreeVisitor<Tree, ExecutionContext>() {
338338
@Nullable
339339
private JavaSourceSetUpdater updater;
340+
private final Map<String, JavaSourceSet> updatedSourceSets = new HashMap<>();
340341

341342
@Override
342343
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
@@ -363,12 +364,15 @@ private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
363364
if (!maybeJp.isPresent() || !acc.configurationsByProject.containsKey(maybeJp.get())) {
364365
return sf;
365366
}
366-
if (updater == null) {
367-
updater = new JavaSourceSetUpdater(ctx);
368-
}
369-
return JavaSourceSet.updateOnSourceFile(sf, sourceSet ->
370-
sourceSet.getGavToTypes().isEmpty() ? sourceSet :
371-
updater.addDependency(sourceSet, groupId, artifactId, acc.resolvedVersion, acc.repositories));
367+
return JavaSourceSet.updateOnSourceFile(sf, updatedSourceSets, sourceSet -> {
368+
if (sourceSet.getGavToTypes().isEmpty()) {
369+
return sourceSet;
370+
}
371+
if (updater == null) {
372+
updater = new JavaSourceSetUpdater(ctx);
373+
}
374+
return updater.addDependency(sourceSet, groupId, artifactId, acc.resolvedVersion, acc.repositories);
375+
});
372376
}
373377
};
374378
}

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

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor(DependencyVersionState acc) {
317317
private final UpdateProperties updateProperties = new UpdateProperties(acc);
318318
@Nullable
319319
private JavaSourceSetUpdater jssUpdater;
320+
private final Map<String, JavaSourceSet> updatedSourceSets = new HashMap<>();
320321

321322
@Override
322323
public boolean isAcceptable(SourceFile sf, ExecutionContext ctx) {
@@ -403,33 +404,31 @@ private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
403404
if (oldDeps == null || oldDeps.isEmpty()) {
404405
return sf;
405406
}
406-
Optional<JavaSourceSet> maybeSourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
407-
if (!maybeSourceSet.isPresent() || maybeSourceSet.get().getGavToTypes().isEmpty()) {
408-
return sf;
409-
}
410-
if (jssUpdater == null) {
411-
jssUpdater = new JavaSourceSetUpdater(ctx);
412-
}
413-
JavaSourceSet sourceSet = maybeSourceSet.get();
414-
for (ResolvedDependency oldDep : oldDeps) {
415-
GroupArtifact ga = new GroupArtifact(oldDep.getGroupId(), oldDep.getArtifactId());
416-
Object newVersionObj = acc.gaToNewVersion.get(ga);
417-
if (!(newVersionObj instanceof String)) {
418-
continue;
407+
return JavaSourceSet.updateOnSourceFile(sf, updatedSourceSets, sourceSet -> {
408+
if (sourceSet.getGavToTypes().isEmpty()) {
409+
return sourceSet;
419410
}
420-
String resolvedNewVersion = (String) newVersionObj;
421-
ResolvedGroupArtifactVersion newGav = new ResolvedGroupArtifactVersion(
422-
oldDep.getGav().getRepository(),
423-
oldDep.getGroupId(), oldDep.getArtifactId(), resolvedNewVersion, null);
424-
ResolvedDependency newDep = oldDep
425-
.withGav(newGav)
426-
.withRepository(MavenRepository.MAVEN_CENTRAL);
427-
sourceSet = jssUpdater.changeDependency(sourceSet, oldDep, newDep);
428-
}
429-
if (sourceSet != maybeSourceSet.get()) {
430-
return sf.withMarkers(sf.getMarkers().setByType(sourceSet));
431-
}
432-
return sf;
411+
if (jssUpdater == null) {
412+
jssUpdater = new JavaSourceSetUpdater(ctx);
413+
}
414+
JavaSourceSet result = sourceSet;
415+
for (ResolvedDependency oldDep : oldDeps) {
416+
GroupArtifact ga = new GroupArtifact(oldDep.getGroupId(), oldDep.getArtifactId());
417+
Object newVersionObj = acc.gaToNewVersion.get(ga);
418+
if (!(newVersionObj instanceof String)) {
419+
continue;
420+
}
421+
String resolvedNewVersion = (String) newVersionObj;
422+
ResolvedGroupArtifactVersion newGav = new ResolvedGroupArtifactVersion(
423+
oldDep.getGav().getRepository(),
424+
oldDep.getGroupId(), oldDep.getArtifactId(), resolvedNewVersion, null);
425+
ResolvedDependency newDep = oldDep
426+
.withGav(newGav)
427+
.withRepository(MavenRepository.MAVEN_CENTRAL);
428+
result = jssUpdater.changeDependency(result, oldDep, newDep);
429+
}
430+
return result;
431+
});
433432
}
434433
};
435434
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ private boolean isAggregatorNotUsedAsParent() {
321321
return new TreeVisitor<Tree, ExecutionContext>() {
322322
@Nullable
323323
private JavaSourceSetUpdater updater;
324+
private final Map<String, JavaSourceSet> updatedSourceSets = new HashMap<>();
324325

325326
@Override
326327
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
@@ -347,12 +348,15 @@ private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
347348
if (!maybeJp.isPresent() || !acc.scopeByProject.containsKey(maybeJp.get())) {
348349
return sf;
349350
}
350-
if (updater == null) {
351-
updater = new JavaSourceSetUpdater(ctx);
352-
}
353-
return JavaSourceSet.updateOnSourceFile(sf, sourceSet ->
354-
sourceSet.getGavToTypes().isEmpty() ? sourceSet :
355-
updater.addDependency(sourceSet, groupId, artifactId, acc.resolvedVersion, acc.repositories));
351+
return JavaSourceSet.updateOnSourceFile(sf, updatedSourceSets, sourceSet -> {
352+
if (sourceSet.getGavToTypes().isEmpty()) {
353+
return sourceSet;
354+
}
355+
if (updater == null) {
356+
updater = new JavaSourceSetUpdater(ctx);
357+
}
358+
return updater.addDependency(sourceSet, groupId, artifactId, acc.resolvedVersion, acc.repositories);
359+
});
356360
}
357361
};
358362
}

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

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,7 @@ null, null, getResolutionResult().getPom().getRepositories()
616616
return new TreeVisitor<Tree, ExecutionContext>() {
617617
@Nullable
618618
private JavaSourceSetUpdater updater;
619+
private final Map<String, JavaSourceSet> updatedSourceSets = new HashMap<>();
619620

620621
@Override
621622
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
@@ -650,24 +651,21 @@ private SourceFile updateJavaSourceSet(SourceFile sf, ExecutionContext ctx) {
650651
if (newVersion == null) {
651652
return sf;
652653
}
653-
Optional<JavaSourceSet> maybeSourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
654-
if (!maybeSourceSet.isPresent() || maybeSourceSet.get().getGavToTypes().isEmpty()) {
655-
return sf;
656-
}
657-
if (updater == null) {
658-
updater = new JavaSourceSetUpdater(ctx);
659-
}
660-
ResolvedGroupArtifactVersion newGav = new ResolvedGroupArtifactVersion(
661-
oldDep.getGav().getRepository(),
662-
oldDep.getGroupId(), oldDep.getArtifactId(), newVersion, null);
663-
ResolvedDependency newDep = oldDep
664-
.withGav(newGav)
665-
.withRepository(findRemoteRepository(maybeJp.get()));
666-
JavaSourceSet updated = updater.changeDependency(maybeSourceSet.get(), oldDep, newDep);
667-
if (updated != maybeSourceSet.get()) {
668-
return sf.withMarkers(sf.getMarkers().setByType(updated));
669-
}
670-
return sf;
654+
return JavaSourceSet.updateOnSourceFile(sf, updatedSourceSets, sourceSet -> {
655+
if (sourceSet.getGavToTypes().isEmpty()) {
656+
return sourceSet;
657+
}
658+
if (updater == null) {
659+
updater = new JavaSourceSetUpdater(ctx);
660+
}
661+
ResolvedGroupArtifactVersion newGav = new ResolvedGroupArtifactVersion(
662+
oldDep.getGav().getRepository(),
663+
oldDep.getGroupId(), oldDep.getArtifactId(), newVersion, null);
664+
ResolvedDependency newDep = oldDep
665+
.withGav(newGav)
666+
.withRepository(findRemoteRepository(maybeJp.get()));
667+
return updater.changeDependency(sourceSet, oldDep, newDep);
668+
});
671669
}
672670

673671
private MavenRepository findRemoteRepository(JavaProject jp) {

‎rewrite-maven/src/main/java/org/openrewrite/maven/utilities/JavaSourceSetUpdater.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,13 @@ public JavaSourceSet changeDependency(JavaSourceSet sourceSet,
7272
ResolvedDependency newDep) {
7373
String oldGavKey = gavKey(oldDep);
7474
String newGavKey = gavKey(newDep);
75-
// Idempotent: if already changed (old absent, new present), skip
76-
if (!sourceSet.getGavToTypes().containsKey(oldGavKey) &&
77-
sourceSet.getGavToTypes().containsKey(newGavKey)) {
78-
return sourceSet;
75+
if (!sourceSet.getGavToTypes().containsKey(oldGavKey)) {
76+
// Old dependency not tracked. If gavToTypes is non-empty, other dependencies
77+
// are being tracked but this one wasn't — nothing to change. If gavToTypes is
78+
// empty, we're in initial population mode (first call on this source set).
79+
if (!sourceSet.getGavToTypes().isEmpty() || sourceSet.getGavToTypes().containsKey(newGavKey)) {
80+
return sourceSet;
81+
}
7982
}
8083
sourceSet = sourceSet.removeTypesForGav(oldGavKey);
8184
List<JavaType.FullyQualified> newTypes = downloadAndScanTypes(newDep);

0 commit comments

Comments
 (0)
⚡