Skip to content

Commit 48f89f5

Browse files
Add star import ambiguity detection to ChangeType
When ChangeType moves a type into a package covered by a star import, and another star import provides a type with the same simple name, add an explicit import to disambiguate. This mirrors the ambiguity handling already present in ChangePackage.
1 parent e1d2fd1 commit 48f89f5

2 files changed

Lines changed: 169 additions & 0 deletions

File tree

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,20 @@
1919
import org.junit.jupiter.api.Disabled;
2020
import org.junit.jupiter.api.Test;
2121
import org.openrewrite.*;
22+
import org.openrewrite.java.marker.JavaSourceSet;
2223
import org.openrewrite.java.search.FindTypes;
2324
import org.openrewrite.java.tree.J;
25+
import org.openrewrite.java.tree.JavaSourceFile;
2426
import org.openrewrite.java.tree.JavaType;
2527
import org.openrewrite.java.tree.NameTree;
2628
import org.openrewrite.java.tree.TypeUtils;
2729
import org.openrewrite.test.RecipeSpec;
2830
import org.openrewrite.test.RewriteTest;
2931
import org.openrewrite.test.SourceSpec;
32+
import org.openrewrite.test.UncheckedConsumer;
33+
34+
import java.util.ArrayList;
35+
import java.util.Optional;
3036

3137
import static org.assertj.core.api.Assertions.assertThat;
3238
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -2596,4 +2602,89 @@ void changeTypeInServiceProviderFileName() {
25962602
)
25972603
);
25982604
}
2605+
2606+
@Test
2607+
void changeTypeAddsExplicitImportWhenStarImportsWouldBeAmbiguous() {
2608+
rewriteRun(
2609+
spec -> spec.recipe(new ChangeType("a.Ambiguous", "b.Ambiguous", true))
2610+
.beforeRecipe(withSourceTypesOnClasspath()),
2611+
java(
2612+
"""
2613+
package a;
2614+
public class Ambiguous {}
2615+
"""
2616+
),
2617+
java(
2618+
"""
2619+
package b;
2620+
public class Ambiguous {}
2621+
"""
2622+
),
2623+
java(
2624+
"""
2625+
package b;
2626+
public class Other {}
2627+
"""
2628+
),
2629+
java(
2630+
"""
2631+
package c;
2632+
public class Ambiguous {}
2633+
"""
2634+
),
2635+
java(
2636+
"""
2637+
import a.Ambiguous;
2638+
import b.*;
2639+
import c.*;
2640+
2641+
class Test {
2642+
Ambiguous a;
2643+
Other o;
2644+
}
2645+
""",
2646+
"""
2647+
import b.Ambiguous;
2648+
import b.*;
2649+
import c.*;
2650+
2651+
class Test {
2652+
Ambiguous a;
2653+
Other o;
2654+
}
2655+
"""
2656+
)
2657+
);
2658+
}
2659+
2660+
private static UncheckedConsumer<java.util.List<SourceFile>> withSourceTypesOnClasspath() {
2661+
return sourceFiles -> {
2662+
java.util.List<JavaType.FullyQualified> sourceTypes = new ArrayList<>();
2663+
for (SourceFile sf : sourceFiles) {
2664+
if (sf instanceof JavaSourceFile) {
2665+
for (J.ClassDeclaration classDecl : ((JavaSourceFile) sf).getClasses()) {
2666+
JavaType.FullyQualified type = classDecl.getType();
2667+
if (type != null) {
2668+
sourceTypes.add(type);
2669+
}
2670+
}
2671+
}
2672+
}
2673+
for (int i = 0; i < sourceFiles.size(); i++) {
2674+
SourceFile sf = sourceFiles.get(i);
2675+
Optional<JavaSourceSet> maybeSourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
2676+
JavaSourceSet ss;
2677+
if (maybeSourceSet.isPresent()) {
2678+
ss = maybeSourceSet.get();
2679+
java.util.List<JavaType.FullyQualified> enriched = new ArrayList<>(ss.getClasspath());
2680+
enriched.addAll(sourceTypes);
2681+
ss = ss.withClasspath(enriched);
2682+
} else {
2683+
ss = new JavaSourceSet(java.util.UUID.randomUUID(), "main", sourceTypes, java.util.Collections.emptyMap());
2684+
}
2685+
sourceFiles.set(i, sf.withMarkers(
2686+
sf.getMarkers().computeByType(ss, (orig, upd) -> upd)));
2687+
}
2688+
};
2689+
}
25992690
}

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.*;
2222
import org.openrewrite.internal.ListUtils;
23+
import org.openrewrite.java.marker.JavaSourceSet;
2324
import org.openrewrite.java.search.UsesType;
2425
import org.openrewrite.java.tree.*;
2526
import org.openrewrite.marker.Markers;
@@ -276,6 +277,12 @@ private void addImport(JavaType.FullyQualified owningClass) {
276277
setCursor(cursor);
277278
}
278279
}));
280+
281+
// If the new type is covered by a star import and another star import
282+
// provides a type with the same simple name, add an explicit import
283+
if (fullyQualifiedTarget != null) {
284+
j = maybeAddExplicitImportForAmbiguity((JavaSourceFile) j, fullyQualifiedTarget);
285+
}
279286
}
280287

281288
return j;
@@ -596,6 +603,77 @@ private JavaType.FullyQualified updateNestedType(JavaType.FullyQualified nestedT
596603
return JavaType.ShallowClass.build(newNestedFqn);
597604
}
598605

606+
/**
607+
* When the new type is provided by a star import and another star import provides
608+
* a type with the same simple name, add an explicit import to disambiguate.
609+
*/
610+
private JavaSourceFile maybeAddExplicitImportForAmbiguity(JavaSourceFile sf, JavaType.FullyQualified newType) {
611+
String newPkg = newType.getPackageName();
612+
String simpleName = newType.getClassName();
613+
614+
// Check if the new type is covered by a star import
615+
boolean coveredByStar = false;
616+
Set<String> otherStarPackages = new LinkedHashSet<>();
617+
for (J.Import anImport : sf.getImports()) {
618+
if (anImport.isStatic() || !"*".equals(anImport.getQualid().getSimpleName())) {
619+
// Check if there's already an explicit import for this type
620+
if (!anImport.isStatic() && anImport.getTypeName().replace('$', '.').equals(newType.getFullyQualifiedName())) {
621+
return sf; // Already has explicit import, no ambiguity possible
622+
}
623+
continue;
624+
}
625+
if (anImport.getPackageName().equals(newPkg)) {
626+
coveredByStar = true;
627+
} else {
628+
otherStarPackages.add(anImport.getPackageName());
629+
}
630+
}
631+
632+
if (!coveredByStar || otherStarPackages.isEmpty()) {
633+
return sf;
634+
}
635+
636+
// Check if any other star-imported package has a type with the same simple name
637+
Optional<JavaSourceSet> sourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
638+
if (!sourceSet.isPresent()) {
639+
return sf;
640+
}
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;
646+
}
647+
}
648+
649+
if (!ambiguous) {
650+
return sf;
651+
}
652+
653+
// Add an explicit import to resolve the ambiguity
654+
J.Import explicitImport = new J.Import(Tree.randomId(),
655+
Space.EMPTY,
656+
Markers.EMPTY,
657+
new JLeftPadded<>(Space.EMPTY, false, Markers.EMPTY),
658+
TypeTree.build(newType.getFullyQualifiedName()).withPrefix(Space.SINGLE_SPACE),
659+
null);
660+
661+
List<JRightPadded<J.Import>> imports = new ArrayList<>(sf.getPadding().getImports());
662+
// Insert the explicit import right before the star import for its package
663+
for (int i = 0; i < imports.size(); i++) {
664+
J.Import imp = imports.get(i).getElement();
665+
if (!imp.isStatic() && "*".equals(imp.getQualid().getSimpleName()) && imp.getPackageName().equals(newPkg)) {
666+
// Give the explicit import the star import's prefix (which has the leading newline)
667+
JRightPadded<J.Import> padded = JRightPadded.build(explicitImport.withPrefix(imp.getPrefix()));
668+
// Give the star import a fresh newline prefix
669+
imports.set(i, imports.get(i).map(starImp -> starImp.withPrefix(Space.format("\n"))));
670+
imports.add(i, padded);
671+
break;
672+
}
673+
}
674+
return sf.getPadding().withImports(imports);
675+
}
676+
599677
private boolean hasNoConflictingImport(@Nullable JavaSourceFile sf) {
600678
JavaType.FullyQualified oldType = TypeUtils.asFullyQualified(originalType);
601679
JavaType.FullyQualified newType = TypeUtils.asFullyQualified(targetType);

0 commit comments

Comments
 (0)