diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java index 0c27837f49..d5e8caeb42 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java @@ -2505,7 +2505,8 @@ A test() { } } """, - """ + """ + import bar.A.Builder; import foo.A; class Test { @@ -2519,6 +2520,135 @@ A test() { ); } + @Test + void renameInnerClassWithinSameOuterClass() { + rewriteRun( + spec -> spec.recipe(new ChangeType("foo.A$Builder", "foo.A$Creator", null)) + .parser(JavaParser.fromJavaVersion().dependsOn( + """ + package foo; + + public class A { + public static class Builder {} + public static class Creator {} + } + """ + ) + ), + java( + """ + import foo.A; + + class Test { + void test() { + A.Builder x = new A.Builder(); + } + } + """, + """ + import foo.A; + + class Test { + void test() { + A.Creator x = new A.Creator(); + } + } + """ + ) + ); + } + + @Test + void changeTypeOfInnerCompletely() { + rewriteRun( + spec -> spec.recipe(new ChangeType("foo.A$Builder", "bar.B$Builder", null)) + .parser(JavaParser.fromJavaVersion().dependsOn( + """ + package foo; + + public class A { + public static class Builder {} + } + """, + """ + package bar; + + public class B { + public static class Builder {} + } + """ + ) + ), + java( + """ + import foo.A; + import foo.A.Builder; + + class Test { + void test() { + A.Builder x = new A.Builder(); + } + } + """, + """ + import bar.B; + + class Test { + void test() { + B.Builder x = new B.Builder(); + } + } + """ + ) + ); + } + + @Test + void changeTypeOfInnerClassImplicitly() { + rewriteRun( + spec -> spec.recipe(new ChangeType("foo.A", "bar.B", null)) + .parser(JavaParser.fromJavaVersion().dependsOn( + """ + package foo; + + public class A { + public static class Builder {} + } + """, + """ + package bar; + + public class B { + public static class Builder {} + } + """ + ) + ), + java( + """ + import foo.A; + import foo.A.Builder; + + class Test { + void test() { + A.Builder x = new A.Builder(); + } + } + """, + """ + import bar.B; + import bar.B.Builder; + + class Test { + void test() { + B.Builder x = new B.Builder(); + } + } + """ + ) + ); + } + @Test void inheritedTypesUpdated() { rewriteRun( diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java index 50eb6be890..69c51f7364 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java @@ -193,12 +193,12 @@ public J visitImport(J.Import import_, ExecutionContext ctx) { return updateType(javaType); } - private void addImport(JavaType.FullyQualified owningClass) { + private void addImport(JavaType.FullyQualified owningClass, boolean onlyIfUsed) { if (importAlias != null) { - maybeAddImport(owningClass.getPackageName(), owningClass.getClassName(), null, importAlias.getSimpleName(), true); + maybeAddImport(owningClass.getPackageName(), owningClass.getClassName(), null, importAlias.getSimpleName(), onlyIfUsed); } - maybeAddImport(owningClass.getPackageName(), owningClass.getClassName(), null, null, true); + maybeAddImport(owningClass.getPackageName(), owningClass.getClassName(), null, null, onlyIfUsed); } @Override @@ -225,6 +225,7 @@ private void addImport(JavaType.FullyQualified owningClass) { j = ((TypedTree) tree).withType(updateType(((TypedTree) tree).getType())); } else if (tree instanceof JavaSourceFile) { JavaSourceFile sf = (JavaSourceFile) tree; + boolean outerClassImportRemoved = false; if (targetType instanceof JavaType.FullyQualified) { for (J.Import anImport : sf.getImports()) { if (anImport.isStatic()) { @@ -242,7 +243,10 @@ private void addImport(JavaType.FullyQualified owningClass) { if (originalType.getFullyQualifiedName().equals(type.getFullyQualifiedName())) { sf = (JavaSourceFile) new RemoveImport(originalType.getFullyQualifiedName()).visitNonNull(sf, ctx, getCursor().getParentOrThrow()); } else if (originalType.getOwningClass() != null && originalType.getOwningClass().getFullyQualifiedName().equals(type.getFullyQualifiedName())) { + JavaSourceFile sfBefore = sf; sf = (JavaSourceFile) new RemoveImport(originalType.getOwningClass().getFullyQualifiedName()).visitNonNull(sf, ctx, getCursor().getParentOrThrow()); + // Track whether the outer class import was actually removed (not retained because still in use) + outerClassImportRemoved |= (sf != sfBefore); } } } @@ -253,13 +257,21 @@ private void addImport(JavaType.FullyQualified owningClass) { if (fullyQualifiedTarget != null) { JavaType.FullyQualified owningClass = fullyQualifiedTarget.getOwningClass(); if (!topLevelClassnames.contains(getTopLevelClassName(fullyQualifiedTarget).getFullyQualifiedName())) { - if (hasNoConflictingImport(sf)) { - if (owningClass != null && !"java.lang".equals(fullyQualifiedTarget.getPackageName())) { - addImport(owningClass); - } - if (!"java.lang".equals(fullyQualifiedTarget.getPackageName())) { - addImport(fullyQualifiedTarget); + if (!"java.lang".equals(fullyQualifiedTarget.getPackageName()) && hasNoConflictingImport(sf)) { + if (owningClass != null) { + // Force-add the outer class import when the original outer class import was + // explicitly removed (meaning code used the outer class by name, e.g. "B" in "B.Builder"). + // When the outer class import was absent, the code uses only the simple inner class + // name (e.g. just "Y"), so AddImport can decide based on actual references. + addImport(owningClass, !outerClassImportRemoved); } + // Force-add the inner class import only when the outer class was NOT removed: + // if the outer class was removed and is being re-added, `import bar.B` already + // makes `B.Builder` accessible — adding `import bar.B.Builder` would be redundant. + // When only the inner class import existed (e.g. code uses simple name "Y"), + // force-add when the owning class changed so FindTypes can locate the new type. + boolean forceAddInnerImport = !outerClassImportRemoved && !owningClassSame(owningClass); + addImport(fullyQualifiedTarget, !forceAddInnerImport); } } } @@ -278,6 +290,16 @@ private void addImport(JavaType.FullyQualified owningClass) { return j; } + private boolean owningClassSame(JavaType.@Nullable FullyQualified owningClass) { + JavaType.FullyQualified originalOwningClass = originalType.getOwningClass(); + if (originalOwningClass == null || owningClass == null) { + return true; + } + + return originalOwningClass.getClassName().equals(owningClass.getClassName()) && + originalOwningClass.getPackageName().equals(owningClass.getPackageName()); + } + @Override public J visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) { if (fieldAccess.isFullyQualifiedClassReference(originalType.getFullyQualifiedName())) { @@ -400,7 +422,8 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) method.getSimpleName().equals(anImport.getQualid().getSimpleName())) { JavaType.FullyQualified targetFqn = (JavaType.FullyQualified) targetType; - addImport(targetFqn); + // onlyIfUsed=true: the static import already proves the type is referenced. + addImport(targetFqn, true); maybeAddImport((targetFqn).getFullyQualifiedName(), method.getName().getSimpleName()); break; }