Skip to content

Commit 13fe9bd

Browse files
Prevent marker-only changes from producing phantom diffs
Dependency-modifying recipes now update JavaSourceSet markers (added in #7202), which can produce Results with identical before/after text. This caused downstream test failures in rewrite-dropwizard, rewrite-hibernate, rewrite-migrate-java, and rewrite-spring. Three fixes: 1. JavaSourceSet.addTypesForGav returns `this` when the gavKey already exists with the same types, preventing unnecessary allocations for LSTs with empty gavToTypes maps. 2. The cached overload of JavaSourceSet.updateOnSourceFile now checks whether the source file's current marker is already the cached instance before replacing it, preventing O(n) phantom diffs across files in the same source set. 3. RewriteTest accepts marker-only changes (identical printed text) instead of failing with "An empty diff was generated", since these represent legitimate internal state updates. Closes #7349
1 parent 1758f7b commit 13fe9bd

2 files changed

Lines changed: 17 additions & 1 deletion

File tree

rewrite-java/src/main/java/org/openrewrite/java/marker/JavaSourceSet.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ public class JavaSourceSet implements SourceSet {
6666
* @return a new JavaSourceSet with the types added
6767
*/
6868
public JavaSourceSet addTypesForGav(String gavKey, List<JavaType.FullyQualified> types) {
69+
List<JavaType.FullyQualified> existing = gavToTypes.get(gavKey);
70+
if (existing != null && existing.equals(types)) {
71+
return this;
72+
}
73+
6974
List<JavaType.FullyQualified> newClasspath = new ArrayList<>(classpath);
7075
newClasspath.addAll(types);
7176

@@ -178,6 +183,9 @@ public static SourceFile updateOnSourceFile(SourceFile sf, Map<String, JavaSourc
178183
String cacheKey = maybeJp.get().getId().toString() + ":" + maybeSourceSet.get().getName();
179184
JavaSourceSet cached = cache.get(cacheKey);
180185
if (cached != null) {
186+
if (cached == maybeSourceSet.get()) {
187+
return sf;
188+
}
181189
return sf.withMarkers(sf.getMarkers().setByType(cached));
182190
}
183191
JavaSourceSet updated = transform.apply(maybeSourceSet.get());

rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,15 @@ default void rewriteRun(Consumer<RecipeSpec> spec, SourceSpec<?>... sourceSpecs)
562562
boolean isRemote = result.getAfter() instanceof Remote;
563563
if (!isRemote && Objects.equals(result.getBefore().getSourcePath(), result.getAfter().getSourcePath()) &&
564564
Objects.equals(before, actualAfter)) {
565-
fail("An empty diff was generated. The recipe incorrectly changed a reference without changing its contents.");
565+
// Marker-only change (e.g. updated JavaSourceSet classpath) — no visible diff.
566+
// Still call afterRecipe so marker assertions work, then skip.
567+
allResults.remove(result);
568+
try {
569+
//noinspection unchecked
570+
((Consumer<SourceFile>) sourceSpec.afterRecipe).accept(result.getAfter());
571+
} catch (ClassCastException ignored) {
572+
}
573+
continue nextSourceFile;
566574
}
567575

568576
assert result.getBefore() != null;

0 commit comments

Comments
 (0)