From ec8663cea6cfa82d065972e231507bf2c4b7acb7 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Fri, 25 Oct 2024 00:29:43 -0700 Subject: [PATCH] [3/3] Implement joda to java time migration recipe (#591) * [3/3] Implement joda to java time migration recipe * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix autosuggestions * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix unit test --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/migrate/joda/JodaTimeRecipe.java | 50 +++++ .../java/migrate/joda/JodaTimeScanner.java | 70 ++---- .../java/migrate/joda/JodaTimeVisitor.java | 74 +++++-- .../java/migrate/joda/ScopeAwareVisitor.java | 95 ++++++++ .../migrate/joda/templates/TimeClassMap.java | 10 + .../migrate/joda/templates/VarTemplates.java | 91 ++++++++ .../java/migrate/joda/JodaTimeRecipeTest.java | 206 ++++++++++++++++++ .../migrate/joda/JodaTimeScannerTest.java | 22 +- .../migrate/joda/JodaTimeVisitorTest.java | 32 +-- 9 files changed, 542 insertions(+), 108 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/ScopeAwareVisitor.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java create mode 100644 src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java new file mode 100644 index 000000000..faf34af26 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java @@ -0,0 +1,50 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.joda; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; + +import java.util.HashSet; +import java.util.Set; + +public class JodaTimeRecipe extends ScanningRecipe> { + @Override + public String getDisplayName() { + return "Migrate Joda Time to Java Time"; + } + + @Override + public String getDescription() { + return "Prefer the Java standard library over third-party usage of Joda Time."; + } + + @Override + public Set getInitialValue(ExecutionContext ctx) { + return new HashSet<>(); + } + + @Override + public JodaTimeScanner getScanner(Set acc) { + return new JodaTimeScanner(acc); + } + + @Override + public JodaTimeVisitor getVisitor(Set acc) { + return new JodaTimeVisitor(acc); + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java index 9df8f6141..2559e3fe5 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -19,7 +19,6 @@ import lombok.Getter; import lombok.NonNull; import lombok.RequiredArgsConstructor; -import lombok.Value; import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.analysis.dataflow.Dataflow; @@ -35,18 +34,25 @@ import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN; -public class JodaTimeScanner extends JavaIsoVisitor { +public class JodaTimeScanner extends ScopeAwareVisitor { @Getter - private final Set unsafeVars = new HashSet<>(); - - private final LinkedList scopes = new LinkedList<>(); + private final Set unsafeVars; private final Map> varDependencies = new HashMap<>(); + public JodaTimeScanner(Set unsafeVars, LinkedList scopes) { + super(scopes); + this.unsafeVars = unsafeVars; + } + + public JodaTimeScanner(Set unsafeVars) { + this(unsafeVars, new LinkedList<>()); + } + @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - cu = super.visitCompilationUnit(cu, ctx); + public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + super.visitCompilationUnit(cu, ctx); Set allReachable = new HashSet<>(); for (NamedVariable var : unsafeVars) { dfs(var, allReachable); @@ -55,18 +61,8 @@ public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionCon return cu; } - @Override - public J.Block visitBlock(J.Block block, ExecutionContext ctx) { - scopes.push(new VariablesInScope(getCursor())); - J.Block b = super.visitBlock(block, ctx); - scopes.pop(); - return b; - } - @Override public NamedVariable visitVariable(NamedVariable variable, ExecutionContext ctx) { - assert !scopes.isEmpty(); - scopes.peek().variables.add(variable); if (!variable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { return variable; } @@ -75,14 +71,14 @@ public NamedVariable visitVariable(NamedVariable variable, ExecutionContext ctx) unsafeVars.add(variable); return variable; } - variable = super.visitVariable(variable, ctx); + variable = (NamedVariable) super.visitVariable(variable, ctx); if (!variable.getType().isAssignableFrom(JODA_CLASS_PATTERN) || variable.getInitializer() == null) { return variable; } List sinks = findSinks(variable.getInitializer()); - assert !scopes.isEmpty(); - Cursor currentScope = scopes.peek().getScope(); + + Cursor currentScope = getCurrentScope(); J.Block block = currentScope.getValue(); new AddSafeCheckMarker(sinks).visit(block, ctx, currentScope.getParent()); processMarkersOnExpression(sinks, variable); @@ -149,27 +145,6 @@ private boolean isLocalVar(NamedVariable variable) { return j instanceof J.Block; } - // Returns the variable in the closest scope - private Optional findVarInScope(String varName) { - for (VariablesInScope scope : scopes) { - for (NamedVariable var : scope.variables) { - if (var.getSimpleName().equals(varName)) { - return Optional.of(var); - } - } - } - return Optional.empty(); - } - - private Cursor findScope(NamedVariable variable) { - for (VariablesInScope scope : scopes) { - if (scope.variables.contains(variable)) { - return scope.scope; - } - } - return null; - } - private void dfs(NamedVariable root, Set visited) { if (visited.contains(root)) { return; @@ -180,17 +155,6 @@ private void dfs(NamedVariable root, Set visited) { } } - @Value - private static class VariablesInScope { - Cursor scope; - Set variables; - - public VariablesInScope(Cursor scope) { - this.scope = scope; - this.variables = new HashSet<>(); - } - } - @RequiredArgsConstructor private class AddSafeCheckMarker extends JavaIsoVisitor { @@ -221,7 +185,7 @@ private SafeCheckMarker getMarker(Expression expr, ExecutionContext ctx) { isSafe = false; } Expression boundaryExpr = boundary.getValue(); - J j = new JodaTimeVisitor(true).visit(boundaryExpr, ctx, boundary.getParentTreeCursor()); + J j = new JodaTimeVisitor(new HashSet<>(), scopes).visit(boundaryExpr, ctx, boundary.getParentTreeCursor()); Set referencedVars = new HashSet<>(); new FindVarReferences().visit(expr, referencedVars, getCursor().getParentTreeCursor()); AtomicBoolean hasJodaType = new AtomicBoolean(); 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 e54b95344..85414793e 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -19,17 +19,17 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.migrate.joda.templates.*; import org.openrewrite.java.tree.*; +import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; +import org.openrewrite.java.tree.MethodCall; -import java.util.List; -import java.util.Optional; +import java.util.*; import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; -public class JodaTimeVisitor extends JavaVisitor { +public class JodaTimeVisitor extends ScopeAwareVisitor { private final MethodMatcher anyNewDateTime = new MethodMatcher(JODA_DATE_TIME + "(..)"); private final MethodMatcher anyDateTime = new MethodMatcher(JODA_DATE_TIME + " *(..)"); @@ -40,16 +40,20 @@ public class JodaTimeVisitor extends JavaVisitor { private final MethodMatcher anyDuration = new MethodMatcher(JODA_DURATION + " *(..)"); private final MethodMatcher anyAbstractInstant = new MethodMatcher(JODA_ABSTRACT_INSTANT + " *(..)"); - private boolean scanMode; + private final Set unsafeVars; - public JodaTimeVisitor(boolean scanMode) { - this.scanMode = scanMode; + public JodaTimeVisitor(Set unsafeVars, LinkedList scopes) { + super(scopes); + this.unsafeVars = unsafeVars; } - public JodaTimeVisitor() { - this(false); + public JodaTimeVisitor(Set unsafeVars) { + this(unsafeVars, new LinkedList<>()); } + public JodaTimeVisitor() { + this(new HashSet<>()); + } @Override public @NonNull J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) { @@ -75,19 +79,54 @@ public JodaTimeVisitor() { return super.visitCompilationUnit(cu, ctx); } + @Override + public @NonNull J visitVariableDeclarations(@NonNull J.VariableDeclarations multiVariable, @NonNull ExecutionContext ctx) { + if (!multiVariable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return super.visitVariableDeclarations(multiVariable, ctx); + } + if (multiVariable.getVariables().stream().anyMatch(unsafeVars::contains)) { + return multiVariable; + } + multiVariable = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx); + return VarTemplates.getTemplate(multiVariable).apply( + updateCursor(multiVariable), + multiVariable.getCoordinates().replace(), + VarTemplates.getTemplateArgs(multiVariable)); + } + @Override public @NonNull J visitVariable(@NonNull J.VariableDeclarations.NamedVariable variable, @NonNull ExecutionContext ctx) { - // TODO implement logic for safe variable migration - if (variable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + if (!variable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return super.visitVariable(variable, ctx); + } + if (unsafeVars.contains(variable) || ! (variable.getType() instanceof JavaType.Class)) { return variable; } - return super.visitVariable(variable, ctx); - } + JavaType.Class jodaType = (JavaType.Class) variable.getType(); + return variable + .withType(TimeClassMap.getJavaTimeType(jodaType.getFullyQualifiedName())) + .withInitializer((Expression) visit(variable.getInitializer(), ctx)); + } @Override public @NonNull J visitAssignment(@NonNull J.Assignment assignment, @NonNull ExecutionContext ctx) { J.Assignment a = (J.Assignment) super.visitAssignment(assignment, ctx); - return a.withType(a.getVariable().getType()); + if (!a.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return a; + } + if (!(a.getVariable() instanceof J.Identifier)) { + return assignment; + } + J.Identifier varName = (J.Identifier) a.getVariable(); + Optional mayBeVar = findVarInScope(varName.getSimpleName()); + if (!mayBeVar.isPresent() || unsafeVars.contains(mayBeVar.get())) { + return assignment; + } + return VarTemplates.getTemplate(a).apply( + updateCursor(a), + a.getCoordinates().replace(), + varName, + a.getAssignment()); } @Override @@ -155,12 +194,11 @@ public JodaTimeVisitor() { @Override public @NonNull J visitIdentifier(@NonNull J.Identifier ident, @NonNull ExecutionContext ctx) { - if (!(isJodaVarRef(ident) && scanMode)) { + if (!isJodaVarRef(ident)) { return super.visitIdentifier(ident, ctx); } - - // TODO: support migration for class variables - if (!(ident.getType() instanceof JavaType.Class)) { + Optional mayBeVar = findVarInScope(ident.getSimpleName()); + if (!mayBeVar.isPresent() || unsafeVars.contains(mayBeVar.get())) { return ident; } diff --git a/src/main/java/org/openrewrite/java/migrate/joda/ScopeAwareVisitor.java b/src/main/java/org/openrewrite/java/migrate/joda/ScopeAwareVisitor.java new file mode 100644 index 000000000..257e90ea0 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/ScopeAwareVisitor.java @@ -0,0 +1,95 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.joda; + +import lombok.AllArgsConstructor; +import lombok.Value; +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; + +import java.util.HashSet; +import java.util.LinkedList; +import java.util.Optional; +import java.util.Set; + +@AllArgsConstructor +public class ScopeAwareVisitor extends JavaVisitor { + protected final LinkedList scopes; + + @Override + public J preVisit(J j, ExecutionContext ctx) { + if (j instanceof J.Block) { + scopes.push(new VariablesInScope(getCursor())); + } + if (j instanceof J.MethodDeclaration) { + scopes.push(new VariablesInScope(getCursor())); + } + if (j instanceof J.VariableDeclarations.NamedVariable) { + assert !scopes.isEmpty(); + NamedVariable variable = (NamedVariable) j; + scopes.peek().variables.add(variable); + } + return super.preVisit(j, ctx); + } + + @Override + public J postVisit(J j, ExecutionContext ctx) { + if (j instanceof J.Block) { + scopes.pop(); + } + return super.postVisit(j, ctx); + } + + Cursor findScope(NamedVariable variable) { + for (VariablesInScope scope : scopes) { + if (scope.variables.contains(variable)) { + return scope.scope; + } + } + return null; + } + + // Returns the variable in the closest scope + Optional findVarInScope(String varName) { + for (VariablesInScope scope : scopes) { + for (NamedVariable var : scope.variables) { + if (var.getSimpleName().equals(varName)) { + return Optional.of(var); + } + } + } + return Optional.empty(); + } + + Cursor getCurrentScope() { + assert !scopes.isEmpty(); + return scopes.peek().scope; + } + + @Value + public static class VariablesInScope { + Cursor scope; + Set variables; + + public VariablesInScope(Cursor scope) { + this.scope = scope; + this.variables = new HashSet<>(); + } + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java b/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java index 08c2f6678..80fbaeb52 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java @@ -37,6 +37,12 @@ public class TimeClassMap { } }; + private final Map jodaToJavaTimeShortName = new HashMap() { + { + put(JODA_DATE_TIME, "ZonedDateTime"); + } + }; + private static JavaType.Class javaTypeClass(String fqn, JavaType.Class superType) { return new JavaType.Class(null, 0, fqn, JavaType.FullyQualified.Kind.Class, null, superType, null, null, null, null, null); @@ -45,4 +51,8 @@ private static JavaType.Class javaTypeClass(String fqn, JavaType.Class superType public static JavaType.Class getJavaTimeType(String typeFqn) { return new TimeClassMap().jodaToJavaTimeMap.get(typeFqn); } + + public static String getJavaTimeShortName(String typeFqn) { + return new TimeClassMap().jodaToJavaTimeShortName.get(typeFqn); + } } 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 new file mode 100644 index 000000000..2d0259b06 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/VarTemplates.java @@ -0,0 +1,91 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.joda.templates; + +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; + +import java.util.HashMap; +import java.util.Map; + +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; + +public class VarTemplates { + + private static Map JodaToJavaTimeType = new HashMap() { + { + put(JODA_DATE_TIME, JAVA_DATE_TIME); + put(JODA_TIME_FORMATTER, JAVA_TIME_FORMATTER); + put(JODA_LOCAL_DATE, JAVA_LOCAL_DATE); + put(JODA_LOCAL_TIME, JAVA_LOCAL_TIME); + put(JODA_DATE_TIME_ZONE, JAVA_ZONE_ID); + put(JODA_DURATION, JAVA_DURATION); + } + }; + + public static JavaTemplate getTemplate(J.VariableDeclarations variable) { + JavaType.Class type = (JavaType.Class) variable.getTypeExpression().getType(); + String typeName = JodaToJavaTimeType.get(type.getFullyQualifiedName()); + StringBuilder template = new StringBuilder(); + String varName; + try { + varName = Class.forName(typeName).getSimpleName(); + } catch (ClassNotFoundException e) { + throw new IllegalArgumentException("Unknown type " + typeName); + } + template.append(varName); + template.append(" "); + for (int i = 0; i < variable.getVariables().size(); i++) { + if (i > 0) { + template.append(", "); + } + template.append("#{}"); + if (variable.getVariables().get(i).getInitializer() != null) { + template.append(" = #{any("); + template.append(typeName); + template.append(")}"); + } + } + return JavaTemplate.builder(template.toString()) + .imports(typeName) + .build(); + } + + public static JavaTemplate 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()); + String template = "#{any(" + varTypeName + ")} = #{any(" + typeName +")}"; + return JavaTemplate.builder(template) + .build(); + } + + public static Object[] getTemplateArgs(J.VariableDeclarations variable) { + Object[] args = new Object[variable.getVariables().size() * 2]; + int i = 0; + for (J.VariableDeclarations.NamedVariable var : variable.getVariables()) { + args[i++] = var.getSimpleName(); + if (var.getInitializer() != null) { + args[i++] = var.getInitializer(); + } + } + Object[] args2 = new Object[i]; + System.arraycopy(args, 0, args2, 0, i); + return args2; + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java new file mode 100644 index 000000000..27418ff1f --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java @@ -0,0 +1,206 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.joda; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class JodaTimeRecipeTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec + .recipe(new JodaTimeRecipe()) + .parser(JavaParser.fromJavaVersion().classpath("joda-time")); + } + + @DocumentExample + @Test + void migrateSafeVariable() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + + class A { + public void foo() { + DateTime dt = new DateTime(); + System.out.println(dt.toDateTime()); + } + } + """, + """ + import java.time.ZonedDateTime; + + class A { + public void foo() { + ZonedDateTime dt = ZonedDateTime.now(); + System.out.println(dt); + } + } + """ + ) + ); + } + + @Test + void dontChangeClassVariable() { + // not supported yet + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + + class A { + public void foo() { + DateTime dt = new DateTime(); + System.out.println(dt.toDateTime()); + System.out.println(new B().dateTime.toDateTime()); + } + public static class B { + DateTime dateTime = new DateTime(); + } + } + """, + """ + import org.joda.time.DateTime; + + import java.time.ZonedDateTime; + + class A { + public void foo() { + ZonedDateTime dt = ZonedDateTime.now(); + System.out.println(dt); + System.out.println(new B().dateTime.toDateTime()); + } + public static class B { + DateTime dateTime = new DateTime(); + } + } + """ + ) + ); + } + + @Test + void dontChangeIncompatibleType() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + + class A { + public void foo() { + new B().print(new DateTime()); // print is public method accepting DateTime, not handled yet + System.out.println(new B().dateTime); + } + } + + class B { + DateTime dateTime = new DateTime(); + public void print(DateTime dateTime) { + System.out.println(dateTime); + } + } + """ + ) + ); + } + + @Test + void noUnsafeVar() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + import java.util.Date; + + class A { + public void foo(String city) { + DateTimeZone dtz; + if ("london".equals(city)) { + dtz = DateTimeZone.forID("Europe/London"); + } else { + dtz = DateTimeZone.forID("America/New_York"); + } + DateTime dt = new DateTime(dtz); + print(dt.toDate()); + } + private void print(Date date) { + System.out.println(date); + } + } + """, + """ + import java.time.ZoneId; + import java.time.ZonedDateTime; + import java.util.Date; + + class A { + public void foo(String city) { + ZoneId dtz; + if ("london".equals(city)) { + dtz = ZoneId.of("Europe/London"); + } else { + dtz = ZoneId.of("America/New_York"); + } + ZonedDateTime dt = ZonedDateTime.now(dtz); + print(Date.from(dt.toInstant())); + } + private void print(Date date) { + System.out.println(date); + } + } + """ + ) + ); + } + + @Test + void localVarUsedReferencedInReturnStatement() { // not supported yet + // language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + + class A { + public DateTime foo(String city) { + DateTimeZone dtz; + if ("london".equals(city)) { + dtz = DateTimeZone.forID("Europe/London"); + } else { + dtz = DateTimeZone.forID("America/New_York"); + } + DateTime dt = new DateTime(dtz); + return dt.plus(2); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java index 520076236..093469353 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java @@ -21,6 +21,8 @@ import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.HashSet; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openrewrite.java.Assertions.java; @@ -29,14 +31,12 @@ class JodaTimeScannerTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec - .recipe(toRecipe(JodaTimeScanner::new)) - .parser(JavaParser.fromJavaVersion().classpath("joda-time")); + spec.parser(JavaParser.fromJavaVersion().classpath("joda-time")); } @Test void noUnsafeVar() { - JodaTimeScanner scanner = new JodaTimeScanner(); + JodaTimeScanner scanner = new JodaTimeScanner(new HashSet<>()); // language=java rewriteRun( spec -> spec.recipe(toRecipe(() -> scanner)), @@ -45,7 +45,7 @@ void noUnsafeVar() { import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import java.util.Date; - + class A { public void foo(String city) { DateTimeZone dtz; @@ -69,7 +69,7 @@ private void print(Date date) { @Test void hasUnsafeVars() { - JodaTimeScanner scanner = new JodaTimeScanner(); + JodaTimeScanner scanner = new JodaTimeScanner(new HashSet<>()); // language=java rewriteRun( spec -> spec.recipe(toRecipe(() -> scanner)), @@ -77,7 +77,7 @@ void hasUnsafeVars() { """ import org.joda.time.DateTime; import org.joda.time.DateTimeZone; - + class A { DateTime dateTime; public void foo(String city) { @@ -110,7 +110,7 @@ private void print(DateTime dateTime) { // method parameter not handled yet @Test void localVarReferencingClassVar() { // not supported yet - JodaTimeScanner scanner = new JodaTimeScanner(); + JodaTimeScanner scanner = new JodaTimeScanner(new HashSet<>()); // language=java rewriteRun( spec -> spec.recipe(toRecipe(() -> scanner)), @@ -118,7 +118,7 @@ void localVarReferencingClassVar() { // not supported yet """ import org.joda.time.DateTime; import org.joda.time.DateTimeZone; - + class A { DateTime dateTime; public void foo(String city) { @@ -144,7 +144,7 @@ public void foo(String city) { @Test void localVarUsedReferencedInReturnStatement() { // not supported yet - JodaTimeScanner scanner = new JodaTimeScanner(); + JodaTimeScanner scanner = new JodaTimeScanner(new HashSet<>()); // language=java rewriteRun( spec -> spec.recipe(toRecipe(() -> scanner)), @@ -152,7 +152,7 @@ void localVarUsedReferencedInReturnStatement() { // not supported yet """ import org.joda.time.DateTime; import org.joda.time.DateTimeZone; - + class A { public DateTime foo(String city) { DateTimeZone dtz; 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 a56da3890..5fa813c35 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java @@ -497,7 +497,7 @@ public void foo() { // Test will be removed once safe variable migration is implemented @Test - void dontChangeMethodAccessOnVariable() { + void migrateSafeVariable() { //language=java rewriteRun( java( @@ -508,36 +508,16 @@ class A { public void foo() { DateTime dt = new DateTime(); System.out.println(dt.toDateTime()); - System.out.println(new B().dateTime.toDateTime()); - } - public static class B { - DateTime dateTime = new DateTime(); } } - """ - ) - ); - } - - @Test - void dontChangeIncompatibleType() { - //language=java - rewriteRun( - java( + """, """ - import org.joda.time.DateTime; - + import java.time.ZonedDateTime; + class A { public void foo() { - new B().print(new DateTime()); // print is public method accepting DateTime, not handled yet - System.out.println(new B().dateTime); - } - } - - class B { - DateTime dateTime = new DateTime(); - public void print(DateTime dateTime) { - System.out.println(dateTime); + ZonedDateTime dt = ZonedDateTime.now(); + System.out.println(dt); } } """