Skip to content

Commit fcf54ab

Browse files
Set type attribution when unfolding star imports (#6614)
* Set type attribution when unfolding star imports When RemoveImport unfolds a star import (e.g., import org.junit.* to import org.junit.Test), the resulting import was missing type attribution on its qualid field. This caused downstream recipes like ChangeType or UpdateTestAnnotation to fail to properly match and transform the import. The fix adds type attribution (JavaType.ShallowClass) to the unfolded import's qualid for non-static imports, allowing downstream recipes to work correctly. Fixes moderneinc/customer-requests#1521 * Expect a single cycle going forward --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 624656f commit fcf54ab

3 files changed

Lines changed: 86 additions & 6 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.openrewrite.Issue;
2121
import org.openrewrite.Recipe;
2222
import org.openrewrite.java.style.ImportLayoutStyle;
23+
import org.openrewrite.java.tree.JavaType;
2324
import org.openrewrite.style.NamedStyles;
2425
import org.openrewrite.test.RewriteTest;
2526

@@ -786,6 +787,45 @@ class Test {
786787
);
787788
}
788789

790+
@Issue("https://github.com/moderneinc/customer-requests/issues/1521")
791+
@Test
792+
void unfoldedStarImportHasTypeAttribution() {
793+
rewriteRun(
794+
spec -> spec.recipes(
795+
removeImport("java.util.List"),
796+
new ChangeType("java.util.Set", "java.util.HashSet", null)
797+
),
798+
java(
799+
"""
800+
package a;
801+
802+
import java.util.*;
803+
804+
public class A {
805+
Set<Integer> s;
806+
}
807+
""",
808+
"""
809+
package a;
810+
811+
import java.util.HashSet;
812+
813+
public class A {
814+
HashSet<Integer> s;
815+
}
816+
""",
817+
spec -> spec.afterRecipe(cu -> {
818+
// Verify the unfolded import has proper type attribution
819+
assertThat(cu.getImports()).hasSize(1);
820+
assertThat(cu.getImports().getFirst().getQualid().getType())
821+
.isInstanceOf(JavaType.FullyQualified.class);
822+
assertThat(((JavaType.FullyQualified) cu.getImports().getFirst().getQualid().getType())
823+
.getFullyQualifiedName()).isEqualTo("java.util.HashSet");
824+
})
825+
)
826+
);
827+
}
828+
789829
@Test
790830
void unfoldSubpackage() {
791831
rewriteRun(

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

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.internal.ListUtils;
2222
import org.openrewrite.java.internal.FormatFirstClassPrefix;
23+
import org.openrewrite.java.marker.JavaSourceSet;
2324
import org.openrewrite.java.style.ImportLayoutStyle;
2425
import org.openrewrite.java.style.IntelliJ;
2526
import org.openrewrite.java.tree.*;
@@ -139,7 +140,7 @@ public RemoveImport(String type, boolean force) {
139140
} else if (!isPackageAlwaysFolded(importLayoutStyle.getPackagesToFold(), import_) &&
140141
methodsAndFieldsUsed.size() + otherMethodsAndFieldsInTypeUsed.size() < importLayoutStyle.getNameCountToUseStarImport()) {
141142
methodsAndFieldsUsed.addAll(otherMethodsAndFieldsInTypeUsed);
142-
return unfoldStarImport(import_, methodsAndFieldsUsed);
143+
return unfoldStarImport(import_, methodsAndFieldsUsed, cu);
143144
}
144145
} else if (TypeUtils.fullyQualifiedNamesAreEqual(typeName, type) && !methodsAndFieldsUsed.contains(imported)) {
145146
// e.g. remove java.util.Collections.emptySet when type is java.util.Collections
@@ -157,7 +158,7 @@ public RemoveImport(String type, boolean force) {
157158
spaceForNextImport.set(import_.getPrefix());
158159
return null;
159160
} else {
160-
return unfoldStarImport(import_, otherTypesInPackageUsed);
161+
return unfoldStarImport(import_, otherTypesInPackageUsed, cu);
161162
}
162163
}
163164
return import_;
@@ -178,14 +179,53 @@ private long countTrailingLinebreaks(Space space) {
178179
return space.getLastWhitespace().chars().filter(s -> s == '\n').count();
179180
}
180181

181-
private Object unfoldStarImport(J.Import starImport, Set<String> otherImportsUsed) {
182+
private Object unfoldStarImport(J.Import starImport, Set<String> otherImportsUsed, JavaSourceFile cu) {
182183
List<J.Import> unfoldedImports = new ArrayList<>(otherImportsUsed.size());
183184
int i = 0;
184185
for (String other : otherImportsUsed) {
185-
J.Import unfolded = starImport.withQualid(starImport.getQualid().withName(starImport
186-
.getQualid().getName().withSimpleName(other))).withId(randomId());
186+
J.FieldAccess newQualid = starImport.getQualid().withName(starImport
187+
.getQualid().getName().withSimpleName(other));
188+
189+
// Set type attribution on the unfolded import so downstream recipes
190+
// can properly match and transform it
191+
if (!starImport.isStatic()) {
192+
String fqn = starImport.getPackageName() + "." + other;
193+
JavaType.FullyQualified typeForImport = findType(fqn, cu);
194+
newQualid = newQualid.withType(typeForImport);
195+
}
196+
197+
J.Import unfolded = starImport.withQualid(newQualid).withId(randomId());
187198
unfoldedImports.add(i++ == 0 ? unfolded : unfolded.withPrefix(Space.format("\n")));
188199
}
189200
return unfoldedImports;
190201
}
202+
203+
/**
204+
* Find a fully qualified type by name. First checks TypesInUse for a fully hydrated type,
205+
* then checks the JavaSourceSet classpath, and falls back to creating a ShallowClass.
206+
*/
207+
private JavaType.FullyQualified findType(String fqn, JavaSourceFile cu) {
208+
// First, check if the type is already used in the source file (fully hydrated)
209+
for (JavaType javaType : cu.getTypesInUse().getTypesInUse()) {
210+
if (javaType instanceof JavaType.FullyQualified) {
211+
JavaType.FullyQualified fq = (JavaType.FullyQualified) javaType;
212+
if (TypeUtils.fullyQualifiedNamesAreEqual(fq.getFullyQualifiedName(), fqn)) {
213+
return fq;
214+
}
215+
}
216+
}
217+
218+
// Check the JavaSourceSet classpath
219+
Optional<JavaSourceSet> sourceSet = cu.getMarkers().findFirst(JavaSourceSet.class);
220+
if (sourceSet.isPresent()) {
221+
for (JavaType.FullyQualified fq : sourceSet.get().getClasspath()) {
222+
if (TypeUtils.fullyQualifiedNamesAreEqual(fq.getFullyQualifiedName(), fqn)) {
223+
return fq;
224+
}
225+
}
226+
}
227+
228+
// Fall back to creating a ShallowClass
229+
return JavaType.ShallowClass.build(fqn);
230+
}
191231
}

rewrite-kotlin/src/test/java/org/openrewrite/kotlin/RemoveImportTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class A
7575
@Test
7676
void removeStarFoldPackage() {
7777
rewriteRun(
78-
spec -> spec.recipe(removeTypeImportRecipe("java.io.OutputStream")).expectedCyclesThatMakeChanges(2),
78+
spec -> spec.recipe(removeTypeImportRecipe("java.io.OutputStream")),
7979
kotlin(
8080
"""
8181
import java.io.*

0 commit comments

Comments
 (0)