From cd9a76184b7a97ed5fdbb4cb061419782c068ab5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 4 Jul 2025 00:30:06 +0200 Subject: [PATCH 1/4] Do not add explicit `Record` class import if using `var` keyword --- .../migrate/lang/ExplicitRecordImport.java | 22 +++---- .../lang/ExplicitRecordImportTest.java | 63 +++++++++++++++---- 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java b/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java index e5576cb5ef..f949e5f519 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java @@ -22,8 +22,8 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JavaSourceFile; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.JavaVarKeyword; public class ExplicitRecordImport extends Recipe { @Override @@ -42,19 +42,17 @@ public TreeVisitor getVisitor() { new UsesType<>("*..Record", false), new JavaIsoVisitor() { @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - JavaSourceFile javaSourceFile = getCursor().firstEnclosing(JavaSourceFile.class); - if (javaSourceFile != null) { - for (JavaType type : cu.getTypesInUse().getTypesInUse()) { - if (type instanceof JavaType.FullyQualified) { - JavaType.FullyQualified ref = (JavaType.FullyQualified) type; - if ("Record".equals(ref.getClassName()) && !ref.getPackageName().startsWith("java.lang")) { - maybeAddImport(ref.getFullyQualifiedName()); - } - } + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + J.VariableDeclarations vd = super.visitVariableDeclarations(multiVariable, ctx); + if (vd.getType() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified type = (JavaType.FullyQualified) vd.getType(); + if ("Record".equals(type.getClassName()) && + !type.getPackageName().startsWith("java.lang") && + (vd.getTypeExpression() == null || !vd.getTypeExpression().getMarkers().findFirst(JavaVarKeyword.class).isPresent())) { + maybeAddImport(type.getFullyQualifiedName()); } } - return cu; + return vd; } } ); diff --git a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java index f7da06d7f0..9f523eeb03 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.java.migrate.lang; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -24,6 +23,7 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; class ExplicitRecordImportTest implements RewriteTest { @@ -32,12 +32,19 @@ public void defaults(RecipeSpec spec) { spec.recipe(new ExplicitRecordImport()) //language=java .parser(JavaParser.fromJavaVersion().dependsOn(""" - package com.acme.music; - public class Record { - String name; - } - """ - ) + package com.acme.music; + public class Record { + public String name; + } + """, + """ + package com.acme.music; + import java.util.List; + public class RecordList { + public List records; + } + """ + ) ); } @@ -51,7 +58,7 @@ void addImportFromSamePackage() { """ package com.acme.music; - public class Test { + class Test { Record record; } """, @@ -60,7 +67,7 @@ public class Test { import com.acme.music.Record; - public class Test { + class Test { Record record; } """ @@ -68,9 +75,29 @@ public class Test { ); } + @Test + void noChangeIfUsingVar() { + rewriteRun( + //language=java + java( + """ + package com.acme.music; + + import com.acme.music.RecordList; + + class Test { + void test() { + for (var record : new RecordList().records) { + } + } + } + """, + s -> s.markers(javaVersion(17)) + ) + ); + } @Test - @Disabled("Not handled yet; deemed unlikely") void noChangeIfAlreadyFullyQualified() { rewriteRun( //language=java @@ -78,9 +105,19 @@ void noChangeIfAlreadyFullyQualified() { """ package com.acme.music; - public class Test { + class Test { com.acme.music.Record record; } + """, + // Perhaps undesired, but also unlikely, so not worth changing + """ + package com.acme.music; + + import com.acme.music.Record; + + class Test { + Record record; + } """ ) ); @@ -97,7 +134,7 @@ void noChangeIfAlreadyImported() { import com.acme.music.Record; - public class Test { + class Test { Record record; } """ @@ -113,7 +150,7 @@ void noImportAddedForJavaLangRecord() { """ package foo.bar; - public class Test { + class Test { Record record; } """ From e270cd3b6c802496ac08e6d048c86e132537faa5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 4 Jul 2025 00:36:57 +0200 Subject: [PATCH 2/4] Show new now problematic case --- .../lang/ExplicitRecordImportTest.java | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java index 9f523eeb03..2ffe00d177 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java @@ -98,7 +98,37 @@ void test() { } @Test - void noChangeIfAlreadyFullyQualified() { + void genericUseOfRecordClass() { + rewriteRun( + //language=java + java( + """ + package com.acme.music; + + import java.util.List; + + class Test { + List records; + } + """, + """ + package com.acme.music; + + import com.acme.music.Record; + + import java.util.List; + + class Test { + List records; + } + """ + ) + ); + } + + + @Test + void documentChangeWhenFullyQualified() { rewriteRun( //language=java java( From 60777d65997837d49eca19e439c2411a0dcc8846 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 4 Jul 2025 00:51:12 +0200 Subject: [PATCH 3/4] Switch to using `FindTypes.findAssignable` --- .../migrate/lang/ExplicitRecordImport.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java b/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java index f949e5f519..f7adf22cde 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/ExplicitRecordImport.java @@ -20,10 +20,9 @@ import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.search.FindTypes; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.JavaVarKeyword; +import org.openrewrite.java.tree.*; public class ExplicitRecordImport extends Recipe { @Override @@ -42,17 +41,21 @@ public TreeVisitor getVisitor() { new UsesType<>("*..Record", false), new JavaIsoVisitor() { @Override - public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { - J.VariableDeclarations vd = super.visitVariableDeclarations(multiVariable, ctx); - if (vd.getType() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified type = (JavaType.FullyQualified) vd.getType(); - if ("Record".equals(type.getClassName()) && - !type.getPackageName().startsWith("java.lang") && - (vd.getTypeExpression() == null || !vd.getTypeExpression().getMarkers().findFirst(JavaVarKeyword.class).isPresent())) { - maybeAddImport(type.getFullyQualifiedName()); + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + JavaSourceFile javaSourceFile = getCursor().firstEnclosing(JavaSourceFile.class); + if (javaSourceFile != null) { + for (NameTree nameTree : FindTypes.findAssignable(cu, "*..Record")) { + if (nameTree.getType() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified ref = (JavaType.FullyQualified) nameTree.getType(); + if ("Record".equals(ref.getClassName()) && + !ref.getPackageName().startsWith("java.lang") && + !nameTree.getMarkers().findFirst(JavaVarKeyword.class).isPresent()) { + maybeAddImport(ref.getFullyQualifiedName()); + } + } } } - return vd; + return cu; } } ); From 2dc5659adb32eb953ec0498bc2339d14e02699d0 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 4 Jul 2025 00:53:08 +0200 Subject: [PATCH 4/4] No need for marker --- .../java/migrate/lang/ExplicitRecordImportTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java index 2ffe00d177..803bb8dc0f 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExplicitRecordImportTest.java @@ -23,7 +23,6 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; -import static org.openrewrite.java.Assertions.javaVersion; class ExplicitRecordImportTest implements RewriteTest { @@ -91,8 +90,7 @@ void test() { } } } - """, - s -> s.markers(javaVersion(17)) + """ ) ); }