Expand star imports in ChangePackage when they would create ambiguity

rewrite-java-test/build.gradle.kts

@@ -4,6 +4,9 @@ plugins {
 
 recipeDependencies {
     parserClasspath("jakarta.persistence:jakarta.persistence-api:3.1.0")
+    testParserClasspath("jakarta.validation:jakarta.validation-api:3.0.2")
+    testParserClasspath("javax.validation:validation-api:1.1.0.Final")
+    testParserClasspath("org.hibernate:hibernate-validator:5.4.3.Final")
 }
 
 dependencies {

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

@@ -18,6 +18,7 @@
 import org.intellij.lang.annotations.Language;
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.InMemoryExecutionContext;
 import org.openrewrite.Issue;
 import org.openrewrite.java.search.FindTypes;
 import org.openrewrite.java.tree.J;
@@ -28,11 +29,16 @@
 import org.openrewrite.test.RewriteTest;
 import org.openrewrite.test.SourceSpec;
 
+import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.List;
 
+import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
+import static org.openrewrite.java.Assertions.addTypesToSourceSet;
 import static org.openrewrite.java.Assertions.java;
+import static org.openrewrite.java.Assertions.srcMainJava;
 import static org.openrewrite.properties.Assertions.properties;
 import static org.openrewrite.xml.Assertions.xml;
 import static org.openrewrite.yaml.Assertions.yaml;
@@ -665,6 +671,92 @@ <T extends Test> T method(T t) {
         );
     }
 
+    @Test
+    void changePackageExpandsStarImportWhenItWouldCreateAmbiguity() {
+        InMemoryExecutionContext ctx = new InMemoryExecutionContext();
+        List<Path> classpath = JavaParser.dependenciesFromResources(ctx,
+          "validation-api", "jakarta.validation-api", "hibernate-validator");
+        rewriteRun(
+          spec -> spec.recipe(new ChangePackage("javax.validation.constraints", "jakarta.validation.constraints", true))
+                  .parser(JavaParser.fromJavaVersion().classpathFromResources(ctx,
+                    "validation-api", "hibernate-validator"))
+                  .beforeRecipe(addTypesToSourceSet("main",
+                    emptyList(), classpath)),
+          srcMainJava(
+            java(
+              """
+                package xyz;
+
+                import javax.validation.constraints.*;
+                import org.hibernate.validator.constraints.*;
+
+                class A {
+                    @NotNull
+                    private String someField;
+                    @NotEmpty
+                    private String otherField;
+                }
+                """,
+              """
+                package xyz;
+
+                import jakarta.validation.constraints.NotNull;
+                import org.hibernate.validator.constraints.*;
+
+                class A {
+                    @NotNull
+                    private String someField;
+                    @NotEmpty
+                    private String otherField;
+                }
+                """
+            )
+          )
+        );
+    }
+
+    @Test
+    void changePackagePreservesStarImportWhenNoAmbiguity() {
+        InMemoryExecutionContext ctx = new InMemoryExecutionContext();
+        List<Path> classpath = JavaParser.dependenciesFromResources(ctx,
+          "validation-api", "jakarta.validation-api");
+        rewriteRun(
+          spec -> spec.recipe(new ChangePackage("javax.validation.constraints", "jakarta.validation.constraints", true))
+                  .parser(JavaParser.fromJavaVersion().classpathFromResources(ctx,
+                    "validation-api"))
+                  .beforeRecipe(addTypesToSourceSet("main",
+                    emptyList(), classpath)),
+          srcMainJava(
+            java(
+              """
+                package xyz;
+
+                import javax.validation.constraints.*;
+
+                class A {
+                    @NotNull
+                    private String someField;
+                    @Size(max = 100)
+                    private String otherField;
+                }
+                """,
+              """
+                package xyz;
+
+                import jakarta.validation.constraints.*;
+
+                class A {
+                    @NotNull
+                    private String someField;
+                    @Size(max = 100)
+                    private String otherField;
+                }
+                """
+            )
+          )
+        );
+    }
+
     @Test
     void annotation() {
         rewriteRun(
@@ -1912,4 +2004,5 @@ public class Importer {
           )
         );
     }
+
 }

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

@@ -21,24 +21,24 @@
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.*;
 import org.openrewrite.internal.ListUtils;
+import org.openrewrite.java.marker.JavaSourceSet;
 import org.openrewrite.java.tree.*;
 import org.openrewrite.marker.SearchResult;
 import org.openrewrite.trait.Reference;
 
 import java.nio.file.Paths;
-import java.util.HashMap;
-import java.util.IdentityHashMap;
-import java.util.Map;
+import java.util.*;
 
 import static java.util.Objects.requireNonNull;
+import static org.openrewrite.Tree.randomId;
 
 /**
  * A recipe that will rename a package name in package statements, imports, and fully-qualified types (see: NOTE).
  * <p>
  * NOTE: Does not currently transform all possible type references, and accomplishing this would be non-trivial.
- * For example, a method invocation select might refer to field `A a` whose type has now changed to `A2`, and so the type
- * on the select should change as well. But how do we identify the set of all method selects which refer to `a`? Suppose
- * it were prefixed like `this.a`, or `MyClass.this.a`, or indirectly via a separate method call like `getA()` where `getA()`
+ * For example, a method invocation select might refer to field {@code A a} whose type has now changed to {@code A2}, and so the type
+ * on the select should change as well. But how do we identify the set of all method selects which refer to {@code a}? Suppose
+ * it were prefixed like {@code this.a}, or {@code MyClass.this.a}, or indirectly via a separate method call like {@code getA()} where {@code getA()}
  * is defined on the super class.
  */
 @Value
@@ -271,12 +271,147 @@ public J postVisit(J tree, ExecutionContext ctx) {
                     }
                 }
 
+                // Expand changed star imports that would create ambiguity with other star imports
+                sf = maybeExpandStarImport(sf, newPackageName, oldPackageName);
+                if (changingTo != null && !changingTo.equals(newPackageName)) {
+                    String oldSubPkg = oldPackageName + changingTo.substring(newPackageName.length());
+                    sf = maybeExpandStarImport(sf, changingTo, oldSubPkg);
+                }
+                if (Boolean.TRUE.equals(recursive)) {
+                    for (J.Import anImport : sf.getImports()) {
+                        if (!anImport.isStatic() && "*".equals(anImport.getQualid().getSimpleName())) {
+                            String pkg = anImport.getPackageName();
+                            if (pkg.startsWith(newPackageName + ".")) {
+                                String oldPkg = oldPackageName + pkg.substring(newPackageName.length());
+                                sf = maybeExpandStarImport(sf, pkg, oldPkg);
+                            }
+                        }
+                    }
+                }
+
                 j = sf;
             }
             //noinspection DataFlowIssue
             return j;
         }
 
+        /**
+         * If a star import for {@code changedPackage} exists alongside other star imports,
+         * and types from {@code changedPackage} share simple names with types from those
+         * other packages, expand the star import into explicit imports to avoid ambiguity.
+         *
+         * @param changedPackage the new package name (after rename)
+         * @param originalPackage the old package name (before rename), used to find types on classpath
+         */
+        private JavaSourceFile maybeExpandStarImport(JavaSourceFile sf, String changedPackage, String originalPackage) {
+            J.Import changedStarImport = null;
+            Set<String> otherStarPackages = new LinkedHashSet<>();
+            for (J.Import anImport : sf.getImports()) {
+                if (anImport.isStatic() || !"*".equals(anImport.getQualid().getSimpleName())) {
+                    continue;
+                }
+                if (anImport.getPackageName().equals(changedPackage)) {
+                    changedStarImport = anImport;
+                } else {
+                    otherStarPackages.add(anImport.getPackageName());
+                }
+            }
+
+            if (changedStarImport == null || otherStarPackages.isEmpty()) {
+                return sf;
+            }
+
+            if (!hasAmbiguity(sf, changedPackage, originalPackage, otherStarPackages)) {
+                return sf;
+            }
+
+            // Collect simple names of types used from the changed package
+            Set<String> usedFromChangedPackage = new TreeSet<>();
+            for (JavaType type : sf.getTypesInUse().getTypesInUse()) {
+                if (type instanceof JavaType.FullyQualified) {
+                    JavaType.FullyQualified fq = (JavaType.FullyQualified) type;
+                    if (fq.getPackageName().equals(changedPackage)) {
+                        usedFromChangedPackage.add(fq.getClassName());
+                    }
+                }
+            }
+
+            if (usedFromChangedPackage.isEmpty()) {
+                return sf;
+            }
+
+            // Expand the changed star import into explicit imports for used types
+            J.Import starImport = changedStarImport;
+            return sf.withImports(ListUtils.flatMap(sf.getImports(), anImport -> {
+                if (anImport == starImport) {
+                    List<J.Import> expanded = new ArrayList<>(usedFromChangedPackage.size());
+                    int i = 0;
+                    for (String simpleName : usedFromChangedPackage) {
+                        J.FieldAccess newQualid = starImport.getQualid()
+                                .withName(starImport.getQualid().getName().withSimpleName(simpleName));
+                        String fqn = changedPackage + "." + simpleName;
+                        newQualid = newQualid.withType(findType(fqn, sf));
+                        J.Import explicit = starImport.withQualid(newQualid).withId(randomId());
+                        expanded.add(i++ == 0 ? explicit : explicit.withPrefix(Space.format("\n")));
+                    }
+                    return expanded;
+                }
+                return anImport;
+            }));
+        }
+
+        /**
+         * Checks whether types in the changed package share simple names with types in
+         * any of the other star-imported packages, using the JavaSourceSet classpath.
+         * Checks both the new package name and the original package name, since the
+         * classpath may still have types under the old package name.
+         */
+        private boolean hasAmbiguity(JavaSourceFile sf, String changedPackage, String originalPackage, Set<String> otherStarPackages) {
+            Optional<JavaSourceSet> sourceSet = sf.getMarkers().findFirst(JavaSourceSet.class);
+            if (!sourceSet.isPresent()) {
+                return false;
+            }
+
+            Set<String> typesInChangedPackage = new HashSet<>();
+            Set<String> typesInOtherPackages = new HashSet<>();
+            for (JavaType.FullyQualified fq : sourceSet.get().getClasspath()) {
+                String pkg = fq.getPackageName();
+                String className = fq.getClassName();
+                if (pkg.equals(changedPackage) || pkg.equals(originalPackage)) {
+                    typesInChangedPackage.add(className);
+                } else if (otherStarPackages.contains(pkg)) {
+                    typesInOtherPackages.add(className);
+                }
+            }
+
+            for (String typeName : typesInChangedPackage) {
+                if (typesInOtherPackages.contains(typeName)) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        private JavaType.FullyQualified findType(String fqn, JavaSourceFile cu) {
+            for (JavaType type : cu.getTypesInUse().getTypesInUse()) {
+                if (type instanceof JavaType.FullyQualified) {
+                    JavaType.FullyQualified fq = (JavaType.FullyQualified) type;
+                    if (TypeUtils.fullyQualifiedNamesAreEqual(fq.getFullyQualifiedName(), fqn)) {
+                        return fq;
+                    }
+                }
+            }
+            Optional<JavaSourceSet> sourceSet = cu.getMarkers().findFirst(JavaSourceSet.class);
+            if (sourceSet.isPresent()) {
+                for (JavaType.FullyQualified fq : sourceSet.get().getClasspath()) {
+                    if (TypeUtils.fullyQualifiedNamesAreEqual(fq.getFullyQualifiedName(), fqn)) {
+                        return fq;
+                    }
+                }
+            }
+            return JavaType.ShallowClass.build(fqn);
+        }
+
         private @Nullable JavaType updateType(@Nullable JavaType oldType) {
             if (oldType == null || oldType instanceof JavaType.Unknown) {
                 return oldType;