Always add import for Record classes not from java.lang package, …

…to avoid conflicts on Java 14+ (#4787) * force Import No Java Record * Apply formatter * Minor polish --------- Co-authored-by: Tim te Beek <tim@moderne.io>
openrewrite · Dec 15, 2024 · 36acd59 · 36acd59
1 parent 601fec7
commit 36acd59
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 18 deletions.
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java
@@ -186,10 +186,11 @@ class A {
 
     @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
     @Test
-    void forceImportNoJavaRecord() {
+    void forceImportNonJavaLangRecord() {
         // Add import for a class named `Record`, even within the same package, to avoid conflicts with java.lang.Record
         rewriteRun(
-          spec -> spec.recipe(toRecipe(() -> new AddImport<>("com.acme.bank.Record", null, false))),
+          spec -> spec.recipe(toRecipe(() -> new AddImport<>("com.acme.bank.Record", null, false)))
+            .parser(JavaParser.fromJavaVersion().dependsOn("package com.acme.bank; public class Record {}")),
           //language=java
           java(
             """
@@ -211,6 +212,38 @@ class Foo {
         );
     }
 
+    @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
+    @Test
+    void forceImportNonJavaLangRecordFromWildcardImport() {
+        // Add import for a class named `Record`, even within the same package, to avoid conflicts with java.lang.Record
+        rewriteRun(
+          spec -> spec.recipe(toRecipe(() -> new AddImport<>("com.acme.bank.Record", null, false)))
+            .parser(JavaParser.fromJavaVersion().dependsOn("package com.acme.bank; public class Record {}")),
+          //language=java
+          java(
+            """
+              package com.acme.bank;
+              
+              import com.acme.bank.*;
+              
+              class Foo {
+              }
+              """,
+            """
+              package com.acme.bank;
+              
+              import com.acme.bank.*;
+              
+              import com.acme.bank.Record;
+              
+              class Foo {
+              }
+              """,
+            spec -> spec.markers(javaVersion(11))
+          )
+        );
+    }
+
     @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
     @Test
     void notForceImportJavaRecord() {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java b/rewrite-java/src/main/java/org/openrewrite/java/AddImport.java
@@ -110,8 +110,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
                 return cu;
             }
             // Nor if the classes are within the same package
-            if (!"Record".equals(typeName) && // Record's late addition to `java.lang` might conflict with user class
-                    cu.getPackageDeclaration() != null &&
+            if (!"Record".equals(typeName) && cu.getPackageDeclaration() != null &&
                     packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor()))) {
                 return cu;
             }
@@ -120,7 +119,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
                 return cu;
             }
 
-            if (cu.getImports().stream().anyMatch(i -> {
+            if (!"Record".equals(typeName) && cu.getImports().stream().anyMatch(i -> {
                 String ending = i.getQualid().getSimpleName();
                 if (member == null) {
                     return !i.isStatic() && i.getPackageName().equals(packageName) &&
@@ -143,19 +142,17 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
 
             List<JRightPadded<J.Import>> imports = new ArrayList<>(cu.getPadding().getImports());
 
-            if (imports.isEmpty() && !cu.getClasses().isEmpty()) {
-                if (cu.getPackageDeclaration() == null) {
-                    // leave javadocs on the class and move other comments up to the import
-                    // (which could include license headers and the like)
-                    Space firstClassPrefix = cu.getClasses().get(0).getPrefix();
-                    importToAdd = importToAdd.withPrefix(firstClassPrefix
-                            .withComments(ListUtils.map(firstClassPrefix.getComments(), comment -> comment instanceof Javadoc ? null : comment))
-                            .withWhitespace(""));
-
-                    cu = cu.withClasses(ListUtils.mapFirst(cu.getClasses(), clazz ->
-                            clazz.withComments(ListUtils.map(clazz.getComments(), comment -> comment instanceof Javadoc ? comment : null))
-                    ));
-                }
+            if (imports.isEmpty() && !cu.getClasses().isEmpty() && cu.getPackageDeclaration() == null) {
+                // leave javadocs on the class and move other comments up to the import
+                // (which could include license headers and the like)
+                Space firstClassPrefix = cu.getClasses().get(0).getPrefix();
+                importToAdd = importToAdd.withPrefix(firstClassPrefix
+                        .withComments(ListUtils.map(firstClassPrefix.getComments(), comment -> comment instanceof Javadoc ? null : comment))
+                        .withWhitespace(""));
+
+                cu = cu.withClasses(ListUtils.mapFirst(cu.getClasses(), clazz ->
+                        clazz.withComments(ListUtils.map(clazz.getComments(), comment -> comment instanceof Javadoc ? comment : null))
+                ));
             }
 
             ImportLayoutStyle layoutStyle = Optional.ofNullable(((SourceFile) cu).getStyle(ImportLayoutStyle.class))