From 13b1307694a0bdc0d917af9246fef54103b8ab51 Mon Sep 17 00:00:00 2001 From: Anshuman Mishra Date: Sun, 1 Dec 2024 14:41:05 -0800 Subject: [PATCH 1/2] Fix multiple issues in JodaTime to JavaTime migration recipe --- .../java/migrate/joda/JodaTimeVisitor.java | 30 +++++++--- .../migrate/joda/templates/VarTemplates.java | 18 ++++-- .../migrate/joda/JodaTimeVisitorTest.java | 60 +++++++++++++++++++ 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java index 647dac37b7..2e13b17bae 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -19,6 +19,7 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavadocVisitor; import org.openrewrite.java.migrate.joda.templates.AllTemplates; import org.openrewrite.java.migrate.joda.templates.MethodTemplate; import org.openrewrite.java.migrate.joda.templates.TimeClassMap; @@ -42,6 +43,19 @@ public JodaTimeVisitor(JodaTimeRecipe.Accumulator acc, boolean safeMigration, Li this.acc = acc; this.safeMigration = safeMigration; } + + @Override + protected JavadocVisitor getJavadocVisitor() { + return new JavadocVisitor(this) { + /** + * Do not visit the method referenced from the Javadoc, may cause recipe to fail. + */ + @Override + public Javadoc visitReference(Javadoc.Reference reference, ExecutionContext ctx) { + return reference; + } + }; + } @Override public @NonNull J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) { @@ -70,17 +84,17 @@ public JodaTimeVisitor(JodaTimeRecipe.Accumulator acc, boolean safeMigration, Li @Override public @NonNull J visitVariableDeclarations(@NonNull J.VariableDeclarations multiVariable, @NonNull ExecutionContext ctx) { - if (!multiVariable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + if (multiVariable.getTypeExpression() == null || !multiVariable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { return super.visitVariableDeclarations(multiVariable, ctx); } if (multiVariable.getVariables().stream().anyMatch(acc.getUnsafeVars()::contains)) { return multiVariable; } - multiVariable = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx); - return VarTemplates.getTemplate(multiVariable).apply( - updateCursor(multiVariable), - multiVariable.getCoordinates().replace(), - VarTemplates.getTemplateArgs(multiVariable)); + J.VariableDeclarations m = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx); + return VarTemplates.getTemplate(multiVariable).map(t -> t.apply( + updateCursor(m), + m.getCoordinates().replace(), + VarTemplates.getTemplateArgs(m))).orElse(multiVariable); } @Override @@ -111,11 +125,11 @@ public JodaTimeVisitor(JodaTimeRecipe.Accumulator acc, boolean safeMigration, Li if (!mayBeVar.isPresent() || acc.getUnsafeVars().contains(mayBeVar.get())) { return assignment; } - return VarTemplates.getTemplate(a).apply( + return VarTemplates.getTemplate(assignment).map(t -> t.apply( updateCursor(a), a.getCoordinates().replace(), varName, - a.getAssignment()); + a.getAssignment())).orElse(assignment); } @Override diff --git a/src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java b/src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java index c85b30293f..d555492a33 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Optional; import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; @@ -37,9 +38,12 @@ public class VarTemplates { } }; - public static JavaTemplate getTemplate(J.VariableDeclarations variable) { + public static Optional getTemplate(J.VariableDeclarations variable) { JavaType.Class type = (JavaType.Class) variable.getTypeExpression().getType(); String typeName = JodaToJavaTimeType.get(type.getFullyQualifiedName()); + if (typeName == null) { + return Optional.empty(); // unhandled type + } StringBuilder template = new StringBuilder(); String varName; try { @@ -60,19 +64,21 @@ public static JavaTemplate getTemplate(J.VariableDeclarations variable) { template.append(")}"); } } - return JavaTemplate.builder(template.toString()) + return Optional.of(JavaTemplate.builder(template.toString()) .imports(typeName) - .build(); + .build()); } - public static JavaTemplate getTemplate(J.Assignment assignment) { + public static Optional getTemplate(J.Assignment assignment) { JavaType.Class type = (JavaType.Class) assignment.getAssignment().getType(); JavaType.Class varType = (JavaType.Class) assignment.getVariable().getType(); String typeName = JodaToJavaTimeType.get(type.getFullyQualifiedName()); String varTypeName = JodaToJavaTimeType.get(varType.getFullyQualifiedName()); + if (typeName == null || varTypeName == null) { + return Optional.empty(); // unhandled type + } String template = "#{any(" + varTypeName + ")} = #{any(" + typeName + ")}"; - return JavaTemplate.builder(template) - .build(); + return Optional.of(JavaTemplate.builder(template).build()); } public static Object[] getTemplateArgs(J.VariableDeclarations variable) { diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java index 86fecef8ac..5d461f8b57 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java @@ -733,4 +733,64 @@ public void foo() { ) ); } + + @Test + void lambdaVarDeclaration() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import java.util.List; + import java.util.ArrayList; + + class A { + public void foo() { + List list = new ArrayList<>(); + list.add(100L); + list.add(200L); + list.forEach(millis -> { + System.out.println(new DateTime().withMillis(millis)); + }); + } + } + """, + """ + import java.util.List; + import java.time.Instant; + import java.time.ZonedDateTime; + import java.util.ArrayList; + + class A { + public void foo() { + List list = new ArrayList<>(); + list.add(100L); + list.add(200L); + list.forEach(millis -> { + System.out.println(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZonedDateTime.now().getZone())); + }); + } + } + """ + ) + ); + } + + @Test + void unhandledVarDeclaration() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.Interval; + + class A { + public void foo(Interval interval) { + interval = new Interval(100, 50); + } + } + """ + ) + ); + } } From 67b3b0c6014b1e09f05da3c077a49e183c30d9ef Mon Sep 17 00:00:00 2001 From: Anshuman Mishra Date: Sun, 1 Dec 2024 20:21:12 -0800 Subject: [PATCH 2/2] formatting --- .../org/openrewrite/java/migrate/joda/JodaTimeVisitor.java | 2 +- .../openrewrite/java/migrate/joda/JodaTimeVisitorTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java index 2e13b17bae..be5e5652f3 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -43,7 +43,7 @@ public JodaTimeVisitor(JodaTimeRecipe.Accumulator acc, boolean safeMigration, Li this.acc = acc; this.safeMigration = safeMigration; } - + @Override protected JavadocVisitor getJavadocVisitor() { return new JavadocVisitor(this) { diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java index 5d461f8b57..71118e3a88 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java @@ -733,7 +733,7 @@ public void foo() { ) ); } - + @Test void lambdaVarDeclaration() { //language=java @@ -775,7 +775,7 @@ public void foo() { ) ); } - + @Test void unhandledVarDeclaration() { //language=java