Skip to content

Commit cd00f8c

Browse files
steve-aom-elliottsteve-aom-elliott
committed
Replace JAR scanning on dependency change with a JavaSourceSet dirty flag
When dependency-mutating recipes (ChangeDependency, AddDependency, UpgradeDependencyVersion) run, they previously asked JavaSourceSetUpdater to download the new dependency's JAR and scan it for types so that downstream ambiguity checks (ChangePackage, ChangeType, OrderImports, AddImport) could see the post-change classpath. On large multi-module projects driven by recipes like UpgradeSpringBoot_4_0, that JAR retrieval dominates recipe wall time with a large fraction of the run's wall time spent there even after prior fetch optimizations. Introduce a `dirty` flag on the JavaSourceSet marker instead. JavaSourceSetUpdater now flips that flag on every change — no downloads, no classpath reshaping. Ambiguity-sensitive recipes read the flag and take the safe path when it is set: - ImportLayoutStyle.isPackageFoldable returns false for a dirty source set, so OrderImports never folds imports into a star it cannot prove is unambiguous. - AddImport passes the dirty flag through to the same conflict detector, so added imports are not folded either. - ChangePackage.hasAmbiguity returns true for a dirty source set, expanding a star into explicit imports before rewriting the package. - ChangeType.maybeAddExplicitImportForAmbiguity adds the explicit import unconditionally for a dirty source set. Parser-built markers are never dirty; the flag is sticky within a recipe run and cleared only by re-parsing. `withDirty(boolean)` is Lombok-generated. Tests cover the marker round-trip, the dirty-flipping in the updater, and the four consumer recipes' safe-path behavior. A new `markSourceSetDirty()` helper in rewrite-java Assertions simulates the post-dependency-change state.
1 parent e156df4 commit cd00f8c

15 files changed

Lines changed: 274 additions & 94 deletions

File tree

‎rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import static org.assertj.core.api.Assertions.fail;
3939
import static org.openrewrite.java.Assertions.addTypesToSourceSet;
4040
import static org.openrewrite.java.Assertions.java;
41+
import static org.openrewrite.java.Assertions.markSourceSetDirty;
4142
import static org.openrewrite.java.Assertions.srcMainJava;
4243
import static org.openrewrite.java.Assertions.withSourceTypesOnClasspath;
4344
import static org.openrewrite.properties.Assertions.properties;
@@ -717,6 +718,51 @@ class A {
717718
);
718719
}
719720

721+
@Test
722+
void changePackageExpandsStarImportWhenSourceSetIsDirty() {
723+
// Classpath shows no ambiguity — only the validation API is on the classpath — but the
724+
// source set has been marked dirty by an earlier in-run dependency mutation. The recipe
725+
// must not rely on the stale classpath and should fall back to the safe path: expand the star.
726+
InMemoryExecutionContext ctx = new InMemoryExecutionContext();
727+
List<Path> classpath = JavaParser.dependenciesFromResources(ctx,
728+
"validation-api", "jakarta.validation-api");
729+
rewriteRun(
730+
spec -> spec.recipe(new ChangePackage("javax.validation.constraints", "jakarta.validation.constraints", true))
731+
.parser(JavaParser.fromJavaVersion().classpathFromResources(ctx,
732+
"validation-api"))
733+
.beforeRecipe(sfs -> {
734+
addTypesToSourceSet("main", emptyList(), classpath).accept(sfs);
735+
markSourceSetDirty().accept(sfs);
736+
}),
737+
srcMainJava(
738+
java(
739+
"""
740+
package xyz;
741+
742+
import javax.validation.constraints.*;
743+
import org.hibernate.validator.constraints.*;
744+
745+
class A {
746+
@NotNull
747+
private String someField;
748+
}
749+
""",
750+
"""
751+
package xyz;
752+
753+
import jakarta.validation.constraints.NotNull;
754+
import org.hibernate.validator.constraints.*;
755+
756+
class A {
757+
@NotNull
758+
private String someField;
759+
}
760+
"""
761+
)
762+
)
763+
);
764+
}
765+
720766
@Test
721767
void changePackagePreservesStarImportWhenNoAmbiguity() {
722768
InMemoryExecutionContext ctx = new InMemoryExecutionContext();

‎rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static org.assertj.core.api.Assertions.assertThat;
3232
import static org.junit.jupiter.api.Assertions.assertEquals;
3333
import static org.openrewrite.java.Assertions.java;
34+
import static org.openrewrite.java.Assertions.markSourceSetDirty;
3435
import static org.openrewrite.java.Assertions.withSourceTypesOnClasspath;
3536
import static org.openrewrite.properties.Assertions.properties;
3637
import static org.openrewrite.test.SourceSpecs.text;
@@ -2598,6 +2599,51 @@ void changeTypeInServiceProviderFileName() {
25982599
);
25992600
}
26002601

2602+
@Test
2603+
void changeTypeAddsExplicitImportWhenSourceSetIsDirty() {
2604+
// Classpath knows of no other Ambiguous type in b.* or c.*, so ambiguity cannot be proven.
2605+
// A dirty source set must force the safe path: add an explicit import anyway.
2606+
rewriteRun(
2607+
spec -> spec.recipe(new ChangeType("a.Ambiguous", "b.Ambiguous", true))
2608+
.beforeRecipe(sfs -> {
2609+
withSourceTypesOnClasspath().accept(sfs);
2610+
markSourceSetDirty().accept(sfs);
2611+
}),
2612+
java(
2613+
"""
2614+
package a;
2615+
public class Ambiguous {}
2616+
"""
2617+
),
2618+
java(
2619+
"""
2620+
package b;
2621+
public class Ambiguous {}
2622+
"""
2623+
),
2624+
java(
2625+
"""
2626+
import a.Ambiguous;
2627+
import b.*;
2628+
import c.*;
2629+
2630+
class Test {
2631+
Ambiguous a;
2632+
}
2633+
""",
2634+
"""
2635+
import b.Ambiguous;
2636+
import b.*;
2637+
import c.*;
2638+
2639+
class Test {
2640+
Ambiguous a;
2641+
}
2642+
"""
2643+
)
2644+
);
2645+
}
2646+
26012647
@Test
26022648
void changeTypeAddsExplicitImportWhenStarImportsWouldBeAmbiguous() {
26032649
rewriteRun(

‎rewrite-java-test/src/test/java/org/openrewrite/java/OrderImportsTest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,31 @@ void foldIntoStar() {
5959
);
6060
}
6161

62+
@Test
63+
void doNotFoldIntoStarWhenSourceSetIsDirty() {
64+
rewriteRun(
65+
spec -> spec.beforeRecipe(markSourceSetDirty()),
66+
java(
67+
"""
68+
import java.util.List;
69+
import java.util.ArrayList;
70+
import java.util.regex.Pattern;
71+
import java.util.Objects;
72+
import java.util.Set;
73+
import java.util.Map;
74+
""",
75+
"""
76+
import java.util.ArrayList;
77+
import java.util.List;
78+
import java.util.Map;
79+
import java.util.Objects;
80+
import java.util.Set;
81+
import java.util.regex.Pattern;
82+
"""
83+
)
84+
);
85+
}
86+
6287
@Test
6388
void sortInnerAndOuterClassesInTheSamePackage() {
6489
rewriteRun(

‎rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,21 @@ void gavCoordinateFromTypeTable() {
8383
void tolerateWeirdClassNames(){
8484
assertThat(JavaSourceSet.isDeclarable("fj.data.$")).isFalse();
8585
}
86+
87+
@Test
88+
void parserBuiltSourceSetIsNotDirty() {
89+
JavaSourceSet jss = JavaSourceSet.build("main", emptyList());
90+
assertThat(jss.isDirty()).isFalse();
91+
}
92+
93+
@Test
94+
void withDirtyTogglesFlagWithoutOtherChanges() {
95+
JavaSourceSet clean = JavaSourceSet.build("main", emptyList());
96+
JavaSourceSet dirty = clean.withDirty(true);
97+
98+
assertThat(dirty.isDirty()).isTrue();
99+
assertThat(dirty.getClasspath()).isEqualTo(clean.getClasspath());
100+
assertThat(dirty.getGavToTypes()).isEqualTo(clean.getGavToTypes());
101+
assertThat(dirty.getId()).isEqualTo(clean.getId());
102+
}
86103
}

‎rewrite-java/src/main/java/org/openrewrite/java/AddImport.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,11 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
175175
ImportLayoutStyle layoutStyle = Optional.ofNullable(Style.from(ImportLayoutStyle.class, ((SourceFile) cu)))
176176
.orElse(IntelliJ.importLayout());
177177

178-
List<JavaType.FullyQualified> classpath = cu.getMarkers().findFirst(JavaSourceSet.class)
179-
.map(JavaSourceSet::getClasspath)
180-
.orElse(emptyList());
178+
Optional<JavaSourceSet> maybeSourceSet = cu.getMarkers().findFirst(JavaSourceSet.class);
179+
List<JavaType.FullyQualified> classpath = maybeSourceSet.map(JavaSourceSet::getClasspath).orElse(emptyList());
180+
boolean classpathDirty = maybeSourceSet.map(JavaSourceSet::isDirty).orElse(false);
181181

182-
List<JRightPadded<J.Import>> newImports = layoutStyle.addImport(cu.getPadding().getImports(), importToAdd, cu.getPackageDeclaration(), classpath);
182+
List<JRightPadded<J.Import>> newImports = layoutStyle.addImport(cu.getPadding().getImports(), importToAdd, cu.getPackageDeclaration(), classpath, classpathDirty);
183183

184184
if (member != null && typeReference.isPresent()) {
185185
cu = (JavaSourceFile) new ShortenFullyQualifiedMemberReferences(typeReference.get()).visit(cu, p);

‎rewrite-java/src/main/java/org/openrewrite/java/Assertions.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,28 @@ public static UncheckedConsumer<List<SourceFile>> addTypesToSourceSet(String sou
266266
return addTypesToSourceSet(sourceSetName, emptyList(), emptyList());
267267
}
268268

269+
/**
270+
* Mark every Java {@link SourceFile} with a dirty {@link JavaSourceSet} marker, simulating the state
271+
* after a dependency-mutating recipe has modified one of the project's dependencies earlier in the recipe run.
272+
* If a source file already has a {@link JavaSourceSet} marker, the existing marker's {@code dirty} flag is
273+
* toggled on; otherwise a fresh dirty marker (empty classpath) is attached.
274+
*/
275+
public static UncheckedConsumer<List<SourceFile>> markSourceSetDirty() {
276+
return sourceFiles -> {
277+
for (int i = 0; i < sourceFiles.size(); i++) {
278+
SourceFile sf = sourceFiles.get(i);
279+
if (!(sf instanceof JavaSourceFile)) {
280+
continue;
281+
}
282+
Optional<JavaSourceSet> maybeSourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
283+
JavaSourceSet dirty = maybeSourceSet
284+
.map(js -> js.withDirty(true))
285+
.orElseGet(() -> JavaSourceSet.build("main", emptyList()).withDirty(true));
286+
sourceFiles.set(i, sf.withMarkers(sf.getMarkers().setByType(dirty)));
287+
}
288+
};
289+
}
290+
269291
/**
270292
* Enrich each source file's JavaSourceSet marker with types declared in other source files,
271293
* so that classpath-based ambiguity detection works in tests where types come from source
@@ -294,7 +316,7 @@ public static UncheckedConsumer<List<SourceFile>> withSourceTypesOnClasspath() {
294316
enriched.addAll(sourceTypes);
295317
ss = ss.withClasspath(enriched);
296318
} else {
297-
ss = new JavaSourceSet(Tree.randomId(), "main", sourceTypes, emptyMap());
319+
ss = new JavaSourceSet(Tree.randomId(), "main", sourceTypes, emptyMap(), false);
298320
}
299321
sourceFiles.set(i, sf.withMarkers(
300322
sf.getMarkers().computeByType(ss, (orig, upd) -> upd)));
@@ -315,6 +337,6 @@ private static JavaProject javaProject(String projectName) {
315337

316338
private static JavaSourceSet javaSourceSet(String sourceSet) {
317339
return javaSourceSets.computeIfAbsent(sourceSet, name ->
318-
new JavaSourceSet(Tree.randomId(), name, emptyList(), emptyMap()));
340+
new JavaSourceSet(Tree.randomId(), name, emptyList(), emptyMap(), false));
319341
}
320342
}

‎rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,11 @@ private boolean hasAmbiguity(JavaSourceFile sf, String changedPackage, String or
377377
if (!sourceSet.isPresent()) {
378378
return false;
379379
}
380+
if (sourceSet.get().isDirty()) {
381+
// An earlier dependency mutation in this run means the classpath cannot be trusted to
382+
// prove non-ambiguity; fall back to the safe path and expand the star.
383+
return true;
384+
}
380385

381386
Set<String> typesInChangedPackage = new HashSet<>();
382387
Set<String> typesInOtherPackages = new HashSet<>();

‎rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -638,11 +638,18 @@ private JavaSourceFile maybeAddExplicitImportForAmbiguity(JavaSourceFile sf, Jav
638638
if (!sourceSet.isPresent()) {
639639
return sf;
640640
}
641-
boolean ambiguous = false;
642-
for (JavaType.FullyQualified fq : sourceSet.get().getClasspath()) {
643-
if (fq.getClassName().equals(simpleName) && otherStarPackages.contains(fq.getPackageName())) {
644-
ambiguous = true;
645-
break;
641+
boolean ambiguous;
642+
if (sourceSet.get().isDirty()) {
643+
// An earlier dependency mutation in this run means the classpath cannot be trusted to
644+
// prove non-ambiguity; fall back to the safe path and add the explicit import.
645+
ambiguous = true;
646+
} else {
647+
ambiguous = false;
648+
for (JavaType.FullyQualified fq : sourceSet.get().getClasspath()) {
649+
if (fq.getClassName().equals(simpleName) && otherStarPackages.contains(fq.getPackageName())) {
650+
ambiguous = true;
651+
break;
652+
}
646653
}
647654
}
648655

‎rewrite-java/src/main/java/org/openrewrite/java/OrderImports.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,14 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
9595
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
9696
Optional<JavaSourceSet> sourceSet = cu.getMarkers().findFirst(JavaSourceSet.class);
9797
List<JavaType.FullyQualified> classpath = emptyList();
98+
boolean classpathDirty = false;
9899
if (sourceSet.isPresent()) {
99100
classpath = sourceSet.get().getClasspath();
101+
classpathDirty = sourceSet.get().isDirty();
100102
}
101103

102104
ImportLayoutStyle importLayoutStyle = importLayoutStyle(cu, namedStyles);
103-
List<JRightPadded<J.Import>> orderedImports = importLayoutStyle.orderImports(cu.getPadding().getImports(), classpath);
105+
List<JRightPadded<J.Import>> orderedImports = importLayoutStyle.orderImports(cu.getPadding().getImports(), classpath, classpathDirty);
104106

105107
boolean changed = false;
106108
if (orderedImports.size() != cu.getImports().size()) {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ public class JavaSourceSet implements SourceSet {
5858
*/
5959
Map<String, List<JavaType.FullyQualified>> gavToTypes;
6060

61+
/**
62+
* When {@code true}, the {@link #classpath} and {@link #gavToTypes} are known to be stale with respect to
63+
* dependency mutations performed earlier in this recipe run. Recipes making ambiguity-sensitive decisions
64+
* from the classpath (e.g. whether to expand a star import or fold a group of imports into a star) must
65+
* treat a dirty source set as "ambiguity possible" and take the safe path: expand stars, never fold.
66+
* <p>
67+
* Cleared only by re-parsing — sticky within a recipe run. Parser-built markers are never dirty.
68+
*/
69+
boolean dirty;
70+
6171
/**
6272
* Add types for the given GAV key to this source set's classpath and gavToTypes mapping.
6373
*
@@ -237,7 +247,7 @@ public static JavaSourceSet build(String sourceSetName, Collection<Path> classpa
237247

238248
// Peculiarly, Classgraph will not return a ClassInfo for java.lang.Object, although it does for all other java.lang types
239249
typeNames.add("java.lang.Object");
240-
return new JavaSourceSet(randomId(), sourceSetName, typesFrom(typeNames), emptyMap());
250+
return new JavaSourceSet(randomId(), sourceSetName, typesFrom(typeNames), emptyMap(), false);
241251
}
242252

243253

@@ -351,7 +361,7 @@ public static JavaSourceSet build(String sourceSetName, Collection<Path> classpa
351361
gavToTypes.put(gav, typesFromPath);
352362
}
353363
}
354-
return new JavaSourceSet(randomId(), sourceSetName, types, gavToTypes);
364+
return new JavaSourceSet(randomId(), sourceSetName, types, gavToTypes, false);
355365
}
356366

357367
/**

0 commit comments

Comments
 (0)
⚡