diff --git a/rewrite-java-test/build.gradle.kts b/rewrite-java-test/build.gradle.kts index 427a9f6c582..54aa4ba0892 100644 --- a/rewrite-java-test/build.gradle.kts +++ b/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 { diff --git a/rewrite-java-test/src/main/resources/META-INF/rewrite/classpath.tsv.gz b/rewrite-java-test/src/main/resources/META-INF/rewrite/classpath.tsv.gz index b15940eecca..45dd8332c3b 100644 Binary files a/rewrite-java-test/src/main/resources/META-INF/rewrite/classpath.tsv.gz and b/rewrite-java-test/src/main/resources/META-INF/rewrite/classpath.tsv.gz differ diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java index 59dae182ee2..b127adbec91 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java +++ b/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 method(T t) { ); } + @Test + void changePackageExpandsStarImportWhenItWouldCreateAmbiguity() { + InMemoryExecutionContext ctx = new InMemoryExecutionContext(); + List 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 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 { ) ); } + } diff --git a/rewrite-java-test/src/test/resources/META-INF/rewrite/classpath.tsv.gz b/rewrite-java-test/src/test/resources/META-INF/rewrite/classpath.tsv.gz new file mode 100644 index 00000000000..78735adfd16 Binary files /dev/null and b/rewrite-java-test/src/test/resources/META-INF/rewrite/classpath.tsv.gz differ diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java index e0598d22d52..95a8f09b593 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java +++ b/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). *

* 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 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 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 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 otherStarPackages) { + Optional sourceSet = sf.getMarkers().findFirst(JavaSourceSet.class); + if (!sourceSet.isPresent()) { + return false; + } + + Set typesInChangedPackage = new HashSet<>(); + Set 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 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;