From c67d20b44f86c0c3090e1c6bcca8bc3d237d661e Mon Sep 17 00:00:00 2001 From: Anshuman Mishra Date: Thu, 17 Oct 2024 01:53:00 -0700 Subject: [PATCH 01/15] [2/x] Implement joda to java time migration recipe --- .../java/migrate/joda/JodaTimeFlowSpec.java | 68 +++++ .../java/migrate/joda/JodaTimeScanner.java | 279 ++++++++++++++++++ .../java/migrate/joda/JodaTimeVisitor.java | 63 +++- .../java/migrate/joda/SafeCheckMarker.java | 47 +++ .../templates/AbstractInstantTemplates.java | 42 +++ .../migrate/joda/templates/TimeClassMap.java | 47 +++ .../joda/templates/TimeClassNames.java | 5 + .../migrate/joda/JodaTimeFlowSpecTest.java | 129 ++++++++ .../migrate/joda/JodaTimeScannerTest.java | 177 +++++++++++ .../migrate/joda/JodaTimeVisitorTest.java | 52 +++- 10 files changed, 904 insertions(+), 5 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java create mode 100644 src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java create mode 100644 src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java create mode 100644 src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java new file mode 100644 index 0000000000..3eb33a013e --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java @@ -0,0 +1,68 @@ +/* + * 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.NonNull; +import org.openrewrite.analysis.dataflow.DataFlowNode; +import org.openrewrite.analysis.dataflow.DataFlowSpec; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; + +import java.util.HashSet; +import java.util.Set; + +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN; + +public class JodaTimeFlowSpec extends DataFlowSpec { + + private static final Set JODA_CLASSES = new HashSet(){{add("org.joda.time.DateTime");}}; + + @Override + public boolean isSource(@NonNull DataFlowNode srcNode) { + Object value = srcNode.getCursor().getParentTreeCursor().getValue(); + + if (value instanceof J.Assignment && ((J.Assignment)value).getVariable() instanceof J.Identifier) { + return isJodaType(((J.Assignment)value).getVariable().getType()); + } + + if (value instanceof J.VariableDeclarations.NamedVariable) { + return isJodaType(((J.VariableDeclarations.NamedVariable)value).getType()); + } + return false; + } + + @Override + public boolean isSink(@NonNull DataFlowNode sinkNode) { + Object value = sinkNode.getCursor().getValue(); + Object parent = sinkNode.getCursor().getParentTreeCursor().getValue(); + if (parent instanceof J.MethodInvocation) { + J.MethodInvocation method = (J.MethodInvocation) parent; + return (method.getSelect() != null && method.getSelect().equals(value)) + || method.getArguments().stream().anyMatch(a -> a.equals(value)); + } + return parent instanceof J.VariableDeclarations.NamedVariable + || parent instanceof J.NewClass + || parent instanceof J.Assignment + || parent instanceof J.Return; + } + + static boolean isJodaType(JavaType type) { + if (!(type instanceof JavaType.Class)) { + return false; + } + return type.isAssignableFrom(JODA_CLASS_PATTERN); + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java new file mode 100644 index 0000000000..15649bda4c --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -0,0 +1,279 @@ +/* + * 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.Getter; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Value; +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +import fj.data.Option; +import org.openrewrite.analysis.dataflow.Dataflow; +import org.openrewrite.analysis.dataflow.analysis.SinkFlowSummary; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; +import org.openrewrite.java.tree.JavaType; + +import java.util.*; + +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN; + +public class JodaTimeScanner extends JavaIsoVisitor { + + @Getter + private final Set unsafeVars = new HashSet<>(); + + private final LinkedList scopes = new LinkedList<>(); + + private final Map> varDependencies = new HashMap<>(); + + @Override + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + cu = super.visitCompilationUnit(cu, ctx); + Set allReachable = new HashSet<>(); + for (NamedVariable var : unsafeVars) { + dfs(var, allReachable); + } + unsafeVars.addAll(allReachable); + 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; + } + // TODO: handle class variables && method parameters + if (!isLocalVar(variable)) { + unsafeVars.add(variable); + return variable; + } + variable = 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(); + J.Block block = currentScope.getValue(); + new AddSafeCheckMarker(sinks).visit(block, ctx, currentScope.getParent()); + processMarkersOnExpression(sinks, variable); + return variable; + } + + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + Expression var = assignment.getVariable(); + // not joda expr or not local variable + if (!isJodaExpr(var) || !(var instanceof J.Identifier)) { + return assignment; + } + J.Identifier ident = (J.Identifier) var; + Optional mayBeVar = findVarInScope(ident.getSimpleName()); + if (!mayBeVar.isPresent()) { + return assignment; + } + NamedVariable variable = mayBeVar.get(); + Cursor varScope = findScope(variable); + List sinks = findSinks(assignment.getAssignment()); + new AddSafeCheckMarker(sinks).visit(varScope.getValue(), ctx, varScope.getParent()); + processMarkersOnExpression(sinks, variable); + return assignment; + } + + private void processMarkersOnExpression(List expressions, NamedVariable var) { + for (Expression expr : expressions) { + Optional mayBeMarker = expr.getMarkers().findFirst(SafeCheckMarker.class); + if (!mayBeMarker.isPresent()) { + continue; + } + SafeCheckMarker marker = mayBeMarker.get(); + if (!marker.isSafe()) { + unsafeVars.add(var); + } + if (!marker.getReferences().isEmpty()) { + varDependencies.compute(var, (k, v) -> v == null ? new HashSet<>() : v).addAll(marker.getReferences()); + for (NamedVariable ref : marker.getReferences()) { + varDependencies.compute(ref, (k, v) -> v == null ? new HashSet<>() : v).add(var); + } + } + } + } + + private boolean isJodaExpr(Expression expression) { + return expression.getType() != null && expression.getType().isAssignableFrom(JODA_CLASS_PATTERN); + } + + private List findSinks(Expression expr) { + Cursor cursor = new Cursor(getCursor(), expr); + Option mayBeSinks = Dataflow.startingAt(cursor).findSinks(new JodaTimeFlowSpec()); + if (mayBeSinks.isNone()) { + return Collections.emptyList(); + } + return mayBeSinks.some().getExpressionSinks(); + } + + private boolean isLocalVar(NamedVariable variable) { + if (!(variable.getVariableType().getOwner() instanceof JavaType.Method)) { + return false; + } + J j = getCursor().dropParentUntil(t -> t instanceof J.Block || t instanceof J.MethodDeclaration).getValue(); + 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; + } + visited.add(root); + for (NamedVariable dep : varDependencies.getOrDefault(root, Collections.emptySet())) { + dfs(dep, 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 { + + @NonNull + private List expressions; + + public Expression visitExpression(Expression expression, ExecutionContext ctx) { + int index = expressions.indexOf(expression); + if (index == -1) { + return super.visitExpression(expression, ctx); + } + Expression withMarker = expression.withMarkers(expression.getMarkers().addIfAbsent(getMarker(expression, ctx))); + expressions.set(index, withMarker); + return withMarker; + } + + private SafeCheckMarker getMarker(Expression expr, ExecutionContext ctx) { + Optional mayBeMarker = expr.getMarkers().findFirst(SafeCheckMarker.class); + if (mayBeMarker.isPresent()) { + return mayBeMarker.get(); + } + + Cursor boundary = findBoundaryCursorForJodaExpr(); + boolean isSafe = true; + // TODO: handle return statement + if (boundary.getParentTreeCursor().getValue() instanceof J.Return) { + isSafe = false; + } + Expression boundaryExpr = boundary.getValue(); + J j = new JodaTimeVisitor(true).visit(boundaryExpr, ctx, boundary.getParentTreeCursor()); + Set referencedVars = new HashSet<>(); + new FindVarReferences().visit(expr, referencedVars, getCursor().getParentTreeCursor()); + Boolean[] hasJodaType = {false}; + new HasJodaType().visit(j, hasJodaType); + isSafe = isSafe && !hasJodaType[0] && !referencedVars.contains(null); + referencedVars.remove(null); + return new SafeCheckMarker(UUID.randomUUID(), isSafe, referencedVars); + } + + /** + * Traverses the cursor to find the first non-Joda expression in the path. + * If no non-Joda expression is found, it returns the cursor pointing + * to the last Joda expression whose parent is not an Expression. + */ + private Cursor findBoundaryCursorForJodaExpr() { + Cursor cursor = getCursor(); + while (cursor.getValue() instanceof Expression && isJodaExpr(cursor.getValue())) { + Cursor parent = cursor.getParentTreeCursor(); + if (parent.getValue() instanceof J && !(parent.getValue() instanceof Expression)) { + return cursor; + } + cursor = parent; + } + return cursor; + } + } + + private class FindVarReferences extends JavaIsoVisitor> { + + @Override + public J.Identifier visitIdentifier(J.Identifier ident, Set vars) { + if (!isJodaExpr(ident) || ident.getFieldType() == null) { + return ident; + } + if (ident.getFieldType().getOwner() instanceof JavaType.Class) { + vars.add(null); // class variable not supported yet. + } + + // find variable in the closest scope + findVarInScope(ident.getSimpleName()).ifPresent(vars::add); + return ident; + } + } + + private static class HasJodaType extends JavaIsoVisitor { + @Override + public Expression visitExpression(Expression expression, Boolean[] hasJodaType) { + if (hasJodaType[0]) { + return expression; + } + if (expression.getType() != null && expression.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + hasJodaType[0] = true; + } + return super.visitExpression(expression, hasJodaType); + } + } +} 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 120476654f..eaff5960c4 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Optional; -import java.util.regex.Pattern; import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; @@ -39,6 +38,18 @@ public class JodaTimeVisitor extends JavaVisitor { private final MethodMatcher anyTimeFormatter = new MethodMatcher(JODA_TIME_FORMAT + " *(..)"); private final MethodMatcher anyNewDuration = new MethodMatcher(JODA_DURATION + "(..)"); private final MethodMatcher anyDuration = new MethodMatcher(JODA_DURATION + " *(..)"); + private final MethodMatcher anyAbstractInstant = new MethodMatcher(JODA_ABSTRACT_INSTANT + " *(..)"); + + private boolean scanMode; + + public JodaTimeVisitor(boolean scanMode) { + this.scanMode = scanMode; + } + + public JodaTimeVisitor() { + this(false); + } + @Override public @NonNull J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) { @@ -46,6 +57,7 @@ public class JodaTimeVisitor extends JavaVisitor { maybeRemoveImport(JODA_DATE_TIME_ZONE); maybeRemoveImport(JODA_TIME_FORMAT); maybeRemoveImport(JODA_DURATION); + maybeRemoveImport(JODA_ABSTRACT_INSTANT); maybeRemoveImport("java.util.Locale"); maybeAddImport(JAVA_DATE_TIME); @@ -59,6 +71,7 @@ public class JodaTimeVisitor extends JavaVisitor { maybeAddImport(JAVA_LOCAL_TIME); maybeAddImport(JAVA_TEMPORAL_ISO_FIELDS); maybeAddImport(JAVA_CHRONO_FIELD); + maybeAddImport(JAVA_UTIL_DATE); return super.visitCompilationUnit(cu, ctx); } @@ -71,6 +84,12 @@ public class JodaTimeVisitor extends JavaVisitor { return super.visitVariable(variable, 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()); + } + @Override public @NonNull J visitNewClass(@NonNull J.NewClass newClass, @NonNull ExecutionContext ctx) { MethodCall updated = (MethodCall) super.visitNewClass(newClass, ctx); @@ -102,12 +121,20 @@ public class JodaTimeVisitor extends JavaVisitor { if (anyDateTime.matches(method) || anyBaseDateTime.matches(method)) { return applyTemplate(method, m, DateTimeTemplates.getTemplates()).orElse(method); } + if (anyAbstractInstant.matches(method)) { + return applyTemplate(method, m, AbstractInstantTemplates.getTemplates()).orElse(method); + } if (anyTimeFormatter.matches(method)) { return applyTemplate(method, m, DateTimeFormatTemplates.getTemplates()).orElse(method); } if (anyDuration.matches(method)) { return applyTemplate(method, m, DurationTemplates.getTemplates()).orElse(method); } + if (method.getSelect() != null + && method.getSelect().getType() != null + && method.getSelect().getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return method; // unhandled case + } if (areArgumentsAssignable(m)) { return m; } @@ -126,17 +153,35 @@ public class JodaTimeVisitor extends JavaVisitor { return f; } + @Override + public @NonNull J visitIdentifier(@NonNull J.Identifier ident, @NonNull ExecutionContext ctx) { + if (!(isJodaVarRef(ident) && scanMode)) { + return super.visitIdentifier(ident, ctx); + } + + // TODO: support migration for class variables + if (!(ident.getType() instanceof JavaType.Class)) { + return ident; + } + + JavaType.FullyQualified jodaType = ((JavaType.Class) ident.getType()); + JavaType.FullyQualified fqType = TimeClassMap.getJavaTimeType(jodaType.getFullyQualifiedName()); + + return ident.withType(fqType) + .withFieldType(ident.getFieldType().withType(fqType)); + } + private boolean hasJodaType(List exprs) { for (Expression expr : exprs) { JavaType exprType = expr.getType(); - if (exprType != null && exprType.isAssignableFrom(Pattern.compile("org.joda.time.*"))) { + if (exprType != null && exprType.isAssignableFrom(JODA_CLASS_PATTERN)) { return true; } } return false; } - private Optional applyTemplate(MethodCall original, MethodCall updated, List templates) { + private Optional applyTemplate(MethodCall original, MethodCall updated, List templates) { for (MethodTemplate template : templates) { if (template.getMatcher().matches(original)) { Expression[] args = template.getTemplateArgsFunc().apply(updated); @@ -150,9 +195,12 @@ private Optional applyTemplate(MethodCall original, MethodCall updat } private boolean areArgumentsAssignable(MethodCall m) { - if (m.getMethodType() == null || m.getArguments().size() != m.getMethodType().getParameterTypes().size()) { + if (m.getMethodType() == null || getArgumentsCount(m) != m.getMethodType().getParameterTypes().size()) { return false; } + if (getArgumentsCount(m) == 0) { + return true; + } for (int i = 0; i < m.getArguments().size(); i++) { if (!TypeUtils.isAssignableTo(m.getMethodType().getParameterTypes().get(i), m.getArguments().get(i).getType())) { return false; @@ -161,6 +209,13 @@ private boolean areArgumentsAssignable(MethodCall m) { return true; } + private int getArgumentsCount(MethodCall m) { + if (m.getArguments().size() == 1 && m.getArguments().get(0) instanceof J.Empty) { + return 0; + } + return m.getArguments().size(); + } + private boolean isJodaVarRef(@Nullable Expression expr) { if (expr == null || expr.getType() == null || !expr.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { return false; diff --git a/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java b/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java new file mode 100644 index 0000000000..e29a89d244 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java @@ -0,0 +1,47 @@ +/* + * 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.EqualsAndHashCode; +import lombok.Value; +import lombok.With; +import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; +import org.openrewrite.marker.Marker; + +import java.util.*; + +/** + * A marker to indicate whether an expression is safe to migrate + * and variables that are referenced in the expression. + */ +@With +@Value +public class SafeCheckMarker implements Marker { + @EqualsAndHashCode.Include + UUID id; + boolean isSafe; + Set references; + + public SafeCheckMarker(UUID id, boolean isSafe, Set references) { + this.id = id; + this.isSafe = isSafe; + this.references = references; + } + + public SafeCheckMarker(UUID id, boolean isSafe) { + this(id, isSafe, new HashSet<>()); + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java b/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java new file mode 100644 index 0000000000..bf190472e1 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java @@ -0,0 +1,42 @@ +/* + * 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.MethodMatcher; + +import java.util.ArrayList; +import java.util.List; + +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; + +public class AbstractInstantTemplates { + private final MethodMatcher toDate = new MethodMatcher(JODA_ABSTRACT_INSTANT + " toDate()"); + + private final JavaTemplate toDateTemplate = JavaTemplate.builder("Date.from(#{any(java.time.ZonedDateTime)}.toInstant())") + .imports(JAVA_UTIL_DATE) + .build(); + + private final List templates = new ArrayList() { + { + add(new MethodTemplate(toDate, toDateTemplate)); + } + }; + + public static List getTemplates() { + return new AbstractInstantTemplates().templates; + } +} 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 new file mode 100644 index 0000000000..ab0fedfeaf --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java @@ -0,0 +1,47 @@ +/* + * 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 java.util.HashMap; +import java.util.Map; +import org.openrewrite.java.tree.JavaType; + +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; + +public class TimeClassMap { + + private static final JavaType.Class object = JavaType.ShallowClass.build("java.lang.Object"); + + private final Map jodaToJavaTimeMap = new HashMap() { + { + put(JODA_DATE_TIME, javaTypeClass(JAVA_DATE_TIME, object)); + put(JODA_BASE_DATE_TIME, javaTypeClass(JAVA_DATE_TIME, object)); + put(JODA_DATE_TIME_ZONE, javaTypeClass(JAVA_ZONE_ID, object)); + put(JODA_TIME_FORMATTER, javaTypeClass(JAVA_TIME_FORMATTER, object)); + put(JODA_DURATION, javaTypeClass(JAVA_DURATION, object)); + put(JODA_READABLE_DURATION, javaTypeClass(JAVA_DURATION, object)); + } + }; + + 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); + } + + public static JavaType.Class getJavaTimeType(String typeFqn) { + return new TimeClassMap().jodaToJavaTimeMap.get(typeFqn); + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassNames.java b/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassNames.java index a9d56b0e40..ad018b0069 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassNames.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassNames.java @@ -19,6 +19,10 @@ public class TimeClassNames { public static final Pattern JODA_CLASS_PATTERN = Pattern.compile("org\\.joda\\.time\\..*"); + + // java util + public static final String JAVA_UTIL_DATE = "java.util.Date"; + // Joda-Time classes public static final String JODA_TIME_PKG = "org.joda.time"; public static final String JODA_BASE_DATE_TIME = JODA_TIME_PKG + ".base.BaseDateTime"; @@ -32,6 +36,7 @@ public class TimeClassNames { public static final String JODA_DURATION_FIELD_TYPE = JODA_TIME_PKG + ".DurationFieldType"; public static final String JODA_DURATION = JODA_TIME_PKG + ".Duration"; public static final String JODA_READABLE_DURATION = JODA_TIME_PKG + ".ReadableDuration"; + public static final String JODA_ABSTRACT_INSTANT = JODA_TIME_PKG + ".base.AbstractInstant"; // Java Time classes public static final String JAVA_TIME_PKG = "java.time"; diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java new file mode 100644 index 0000000000..f726d79fd3 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java @@ -0,0 +1,129 @@ +/* + * 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.Cursor; +import org.openrewrite.ExecutionContext; +import org.openrewrite.analysis.dataflow.Dataflow; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaParser; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.marker.SearchResult; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.test.RewriteTest.toRecipe; + +public class JodaTimeFlowSpecTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec + .recipe(toRecipe(() -> new JavaIsoVisitor<>() { + Map> exprVarBindings = new HashMap<>(); + + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + if (variable.getInitializer() == null) { + return super.visitVariable(variable, ctx); + } + updateSinks(variable.getInitializer(), variable.getName()); + return super.visitVariable(variable, ctx); + } + + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + if (!(assignment.getVariable() instanceof J.Identifier)) { + return super.visitAssignment(assignment, ctx); + } + updateSinks(assignment.getAssignment(), (J.Identifier) assignment.getVariable()); + return super.visitAssignment(assignment, ctx); + } + + @Override + public Expression visitExpression(Expression expression, ExecutionContext ctx) { + List identifiers = exprVarBindings.get(expression); + if (identifiers == null || identifiers.isEmpty()) { + return expression; + } + String desc = identifiers.stream().map(J.Identifier::getSimpleName).collect(Collectors.joining(", ")); + return SearchResult.found(expression, desc); + } + + private void updateSinks(Expression expr, J.Identifier identifier) { + Cursor cursor = new Cursor(getCursor(), expr); + Dataflow.startingAt(cursor).findSinks(new JodaTimeFlowSpec()) + .foreachDoEffect(sinkFlow -> { + for (Expression sink : sinkFlow.getExpressionSinks()) { + exprVarBindings.computeIfAbsent(sink, e -> new ArrayList<>()).add(identifier); + } + }); + } + })) + .parser(JavaParser.fromJavaVersion().classpath("joda-time")); + } + + @Test + void jodaTimeUsageWithVarBindings() { + rewriteRun( + // language=java + java( + """ + import org.joda.time.DateTime; + import org.joda.time.Interval; + + class A { + public void foo() { + DateTime dateTime = new DateTime(), _dateTime = DateTime.now(); + System.out.println(dateTime); + DateTime dateTimePlus2 = dateTime.plusDays(2); + System.out.println(dateTimePlus2); + dateTime = dateTime.minusDays(1); + _dateTime = dateTime; + Interval interval = new Interval(_dateTime, dateTimePlus2); + System.out.println(interval); + } + } + """, + """ + import org.joda.time.DateTime; + import org.joda.time.Interval; + + class A { + public void foo() { + DateTime dateTime = /*~~(dateTime)~~>*/new DateTime(), _dateTime = /*~~(_dateTime)~~>*/DateTime.now(); + System.out.println(/*~~(dateTime)~~>*/dateTime); + DateTime dateTimePlus2 = /*~~(dateTimePlus2)~~>*//*~~(dateTime)~~>*/dateTime.plusDays(2); + System.out.println(/*~~(dateTimePlus2)~~>*/dateTimePlus2); + dateTime = /*~~(dateTime)~~>*//*~~(dateTime)~~>*/dateTime.minusDays(1); + _dateTime = /*~~(dateTime, _dateTime)~~>*/dateTime; + Interval interval = /*~~(interval)~~>*/new Interval(/*~~(dateTime, _dateTime)~~>*/_dateTime, /*~~(dateTimePlus2)~~>*/dateTimePlus2); + System.out.println(/*~~(interval)~~>*/interval); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java new file mode 100644 index 0000000000..9b11595ea5 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java @@ -0,0 +1,177 @@ +/* + * 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.java.JavaParser; +import org.openrewrite.java.tree.J; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.test.RewriteTest.toRecipe; + +public class JodaTimeScannerTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec + .recipe(toRecipe(JodaTimeScanner::new)) + .parser(JavaParser.fromJavaVersion().classpath("joda-time")); + } + + @Test + void noUnsafeVar() { + JodaTimeScanner scanner = new JodaTimeScanner(); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + 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); + } + } + """ + ) + ); + assertTrue(scanner.getUnsafeVars().isEmpty()); + } + + @Test + void hasUnsafeVars() { + JodaTimeScanner scanner = new JodaTimeScanner(); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + + class A { + DateTime dateTime; + 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.toDateTime()); + } + private void print(DateTime dateTime) { // method parameter not handled yet + System.out.println(dateTime); + } + } + """ + ) + ); + // The variable dtz is unsafe due to dt. The dt variable is unsafe because its associated expression + // is passed as argument to method, and migration of method parameters has not been implemented yet. + assertEquals(4, scanner.getUnsafeVars().size()); + for (J.VariableDeclarations.NamedVariable var : scanner.getUnsafeVars()) { + assertTrue(var.getSimpleName().equals("dtz") + || var.getSimpleName().equals("dt") + || var.getSimpleName().equals("dateTime") + ); + } + } + + @Test + void localVarReferencingClassVar() { // not supported yet + JodaTimeScanner scanner = new JodaTimeScanner(); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + + class A { + DateTime dateTime; + 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 = dateTime.minus(2); + System.out.println(dt); + } + } + """ + ) + ); + // The local variable dt is unsafe due to class var datetime. + assertEquals(2, scanner.getUnsafeVars().size()); + for (J.VariableDeclarations.NamedVariable var : scanner.getUnsafeVars()) { + assertTrue(var.getSimpleName().equals("dateTime") || var.getSimpleName().equals("dt")); + } + } + + @Test + void localVarUsedReferencedInReturnStatement() { // not supported yet + JodaTimeScanner scanner = new JodaTimeScanner(); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + 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); + } + } + """ + ) + ); + // The local variable dt used in return statement. + assertEquals(2, scanner.getUnsafeVars().size()); + for (J.VariableDeclarations.NamedVariable var : scanner.getUnsafeVars()) { + assertTrue(var.getSimpleName().equals("dtz") || var.getSimpleName().equals("dt")); + } + } +} 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 37b8734d99..eee33b8def 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java @@ -28,7 +28,7 @@ class JodaTimeVisitorTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { spec - .recipe(toRecipe(JodaTimeVisitor::new)) + .recipe(toRecipe(() -> new JodaTimeVisitor())) .parser(JavaParser.fromJavaVersion().classpath("joda-time")); } @@ -413,6 +413,34 @@ public void foo() { ); } + @Test + void migrateAbstractInstant() { + // language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + + class A { + public void foo() { + new DateTime().toDate(); + } + } + """, + """ + import java.time.ZonedDateTime; + import java.util.Date; + + class A { + public void foo() { + Date.from(ZonedDateTime.now().toInstant()); + } + } + """ + ) + ); + } + @Test void migrateClassesWithFqn() { // language=java @@ -502,10 +530,12 @@ void dontChangeIncompatibleType() { 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); } @@ -534,4 +564,24 @@ public void foo() { ) ); } + + @Test + void unhandledCases() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import org.joda.time.Instant; + + class A { + public void foo() { + new Instant(); + new DateTime().getZone(); + } + } + """ + ) + ); + } } From ad74a10715b941689d1e4a8acef57194fd7808d8 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:15:32 -0700 Subject: [PATCH 02/15] Update src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../openrewrite/java/migrate/joda/JodaTimeScannerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 9b11595ea5..3e30524001 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java @@ -101,9 +101,9 @@ private void print(DateTime dateTime) { // method parameter not handled yet // is passed as argument to method, and migration of method parameters has not been implemented yet. assertEquals(4, scanner.getUnsafeVars().size()); for (J.VariableDeclarations.NamedVariable var : scanner.getUnsafeVars()) { - assertTrue(var.getSimpleName().equals("dtz") - || var.getSimpleName().equals("dt") - || var.getSimpleName().equals("dateTime") + assertTrue(var.getSimpleName().equals("dtz") || + var.getSimpleName().equals("dt") || + var.getSimpleName().equals("dateTime") ); } } From 87d5abcfd02bec0c16321b959cb8763c59f9dc4d Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:15:42 -0700 Subject: [PATCH 03/15] Update src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3e30524001..5bd79cce68 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java @@ -26,7 +26,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.RewriteTest.toRecipe; -public class JodaTimeScannerTest implements RewriteTest { +class JodaTimeScannerTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { spec From b2829a867c65091f82c0fcb5fbc038292a8a5b1f Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:16:01 -0700 Subject: [PATCH 04/15] Update src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/migrate/joda/JodaTimeFlowSpec.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java index 3eb33a013e..a0d56e6b02 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java @@ -50,12 +50,12 @@ public boolean isSink(@NonNull DataFlowNode sinkNode) { Object parent = sinkNode.getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.MethodInvocation) { J.MethodInvocation method = (J.MethodInvocation) parent; - return (method.getSelect() != null && method.getSelect().equals(value)) - || method.getArguments().stream().anyMatch(a -> a.equals(value)); - } - return parent instanceof J.VariableDeclarations.NamedVariable - || parent instanceof J.NewClass - || parent instanceof J.Assignment + return (method.getSelect() != null && method.getSelect().equals(value)) || + method.getArguments().stream().anyMatch(a -> a.equals(value)); + return parent instanceof J.VariableDeclarations.NamedVariable || + parent instanceof J.NewClass || + parent instanceof J.Assignment || + parent instanceof J.Return; || parent instanceof J.Return; } From fb6c710d89c6235490819b90fd25db5dc384e2ef Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:16:23 -0700 Subject: [PATCH 05/15] Update src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java | 1 + 1 file changed, 1 insertion(+) 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 15649bda4c..1410010e5d 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -196,6 +196,7 @@ private class AddSafeCheckMarker extends JavaIsoVisitor { @NonNull private List expressions; + @Override public Expression visitExpression(Expression expression, ExecutionContext ctx) { int index = expressions.indexOf(expression); if (index == -1) { From 73a659a08a38f4cf043b9d5925b645e74df2d890 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:16:33 -0700 Subject: [PATCH 06/15] Update src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/migrate/joda/SafeCheckMarker.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java b/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java index e29a89d244..bc5ec02b8d 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/SafeCheckMarker.java @@ -21,7 +21,9 @@ import org.openrewrite.java.tree.J.VariableDeclarations.NamedVariable; import org.openrewrite.marker.Marker; -import java.util.*; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; /** * A marker to indicate whether an expression is safe to migrate From dcc3934cc78814fb3ba9d06216f2928f1048f006 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:16:50 -0700 Subject: [PATCH 07/15] Update src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java index f726d79fd3..4b7e17f081 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java @@ -84,6 +84,7 @@ private void updateSinks(Expression expr, J.Identifier identifier) { .parser(JavaParser.fromJavaVersion().classpath("joda-time")); } + @DocumentExample @Test void jodaTimeUsageWithVarBindings() { rewriteRun( From b525129c643febb60c727133956f894516aac1d5 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:17:03 -0700 Subject: [PATCH 08/15] Update src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java index 4b7e17f081..0401bca56e 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java @@ -36,7 +36,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.RewriteTest.toRecipe; -public class JodaTimeFlowSpecTest implements RewriteTest { +class JodaTimeFlowSpecTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { spec From e33cb4ab6a7ac2ff7c95f2013f4c5cbfaf7c653a Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:17:23 -0700 Subject: [PATCH 09/15] Update src/main/java/org/openrewrite/java/migrate/joda/templates/TimeClassMap.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../openrewrite/java/migrate/joda/templates/TimeClassMap.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ab0fedfeaf..dbe751fc93 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 @@ -15,7 +15,8 @@ */ package org.openrewrite.java.migrate.joda.templates; -import java.util.HashMap; +import org.openrewrite.java.tree.JavaType; + import java.util.Map; import org.openrewrite.java.tree.JavaType; From 51693888768dd6fe26136089dc7f9cc25ba600d4 Mon Sep 17 00:00:00 2001 From: amishra-u <119983081+amishra-u@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:17:31 -0700 Subject: [PATCH 10/15] Update src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/migrate/joda/templates/AbstractInstantTemplates.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java b/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java index bf190472e1..ded2197aa8 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java @@ -21,7 +21,8 @@ import java.util.ArrayList; import java.util.List; -import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JAVA_UTIL_DATE; +import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_ABSTRACT_INSTANT; public class AbstractInstantTemplates { private final MethodMatcher toDate = new MethodMatcher(JODA_ABSTRACT_INSTANT + " toDate()"); From 6accb3572b13e5050d56f19d5b8107128bd9983c Mon Sep 17 00:00:00 2001 From: Anshuman Mishra Date: Thu, 17 Oct 2024 12:23:27 -0700 Subject: [PATCH 11/15] github suggestion --- .../java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java | 2 +- .../openrewrite/java/migrate/joda/templates/TimeClassMap.java | 2 +- .../org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) 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 1410010e5d..31e7b6bf31 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -15,13 +15,13 @@ */ package org.openrewrite.java.migrate.joda; +import fj.data.Option; import lombok.Getter; import lombok.NonNull; import lombok.RequiredArgsConstructor; import lombok.Value; import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; -import fj.data.Option; import org.openrewrite.analysis.dataflow.Dataflow; import org.openrewrite.analysis.dataflow.analysis.SinkFlowSummary; import org.openrewrite.java.JavaIsoVisitor; 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 dbe751fc93..08c2f66787 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 @@ -17,8 +17,8 @@ import org.openrewrite.java.tree.JavaType; +import java.util.HashMap; import java.util.Map; -import org.openrewrite.java.tree.JavaType; import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*; diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java index 0401bca56e..a54e65d460 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpecTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.Cursor; +import org.openrewrite.DocumentExample; import org.openrewrite.ExecutionContext; import org.openrewrite.analysis.dataflow.Dataflow; import org.openrewrite.java.JavaIsoVisitor; From 3bdcc77208857224f6a8234f4bef6ac69843a992 Mon Sep 17 00:00:00 2001 From: Anshuman Mishra Date: Thu, 17 Oct 2024 12:28:54 -0700 Subject: [PATCH 12/15] github auto commit mess --- .../org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java | 6 ++---- .../org/openrewrite/java/migrate/joda/JodaTimeVisitor.java | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java index a0d56e6b02..2cfe4fa51c 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java @@ -28,8 +28,6 @@ public class JodaTimeFlowSpec extends DataFlowSpec { - private static final Set JODA_CLASSES = new HashSet(){{add("org.joda.time.DateTime");}}; - @Override public boolean isSource(@NonNull DataFlowNode srcNode) { Object value = srcNode.getCursor().getParentTreeCursor().getValue(); @@ -51,12 +49,12 @@ public boolean isSink(@NonNull DataFlowNode sinkNode) { if (parent instanceof J.MethodInvocation) { J.MethodInvocation method = (J.MethodInvocation) parent; return (method.getSelect() != null && method.getSelect().equals(value)) || - method.getArguments().stream().anyMatch(a -> a.equals(value)); + method.getArguments().stream().anyMatch(a -> a.equals(value)); + } return parent instanceof J.VariableDeclarations.NamedVariable || parent instanceof J.NewClass || parent instanceof J.Assignment || parent instanceof J.Return; - || parent instanceof J.Return; } static boolean isJodaType(JavaType type) { 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 eaff5960c4..38cf2dc408 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -130,9 +130,9 @@ public JodaTimeVisitor() { if (anyDuration.matches(method)) { return applyTemplate(method, m, DurationTemplates.getTemplates()).orElse(method); } - if (method.getSelect() != null - && method.getSelect().getType() != null - && method.getSelect().getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + if (method.getSelect() != null && + method.getSelect().getType() != null && + method.getSelect().getType().isAssignableFrom(JODA_CLASS_PATTERN)) { return method; // unhandled case } if (areArgumentsAssignable(m)) { From 2d1d0fb9c8fa595a9e6646a944ccbfadeb8fb390 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 21 Oct 2024 14:55:01 +0200 Subject: [PATCH 13/15] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java index 2cfe4fa51c..5e3f324d5b 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java @@ -21,9 +21,6 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; -import java.util.HashSet; -import java.util.Set; - import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN; public class JodaTimeFlowSpec extends DataFlowSpec { From f655d9dad4993edb77385358edc171ad01a56b83 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Oct 2024 23:51:13 +0200 Subject: [PATCH 14/15] Adopt AtomicBoolean --- .../java/migrate/joda/JodaTimeScanner.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 31e7b6bf31..9df8f6141b 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -31,6 +31,7 @@ import org.openrewrite.java.tree.JavaType; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN; @@ -223,9 +224,9 @@ private SafeCheckMarker getMarker(Expression expr, ExecutionContext ctx) { J j = new JodaTimeVisitor(true).visit(boundaryExpr, ctx, boundary.getParentTreeCursor()); Set referencedVars = new HashSet<>(); new FindVarReferences().visit(expr, referencedVars, getCursor().getParentTreeCursor()); - Boolean[] hasJodaType = {false}; + AtomicBoolean hasJodaType = new AtomicBoolean(); new HasJodaType().visit(j, hasJodaType); - isSafe = isSafe && !hasJodaType[0] && !referencedVars.contains(null); + isSafe = isSafe && !hasJodaType.get() && !referencedVars.contains(null); referencedVars.remove(null); return new SafeCheckMarker(UUID.randomUUID(), isSafe, referencedVars); } @@ -265,14 +266,14 @@ public J.Identifier visitIdentifier(J.Identifier ident, Set vars) } } - private static class HasJodaType extends JavaIsoVisitor { + private static class HasJodaType extends JavaIsoVisitor { @Override - public Expression visitExpression(Expression expression, Boolean[] hasJodaType) { - if (hasJodaType[0]) { + public Expression visitExpression(Expression expression, AtomicBoolean hasJodaType) { + if (hasJodaType.get()) { return expression; } if (expression.getType() != null && expression.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { - hasJodaType[0] = true; + hasJodaType.set(true); } return super.visitExpression(expression, hasJodaType); } From 17b0ec46998a47e080183231256dd7c993c7d8d7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Oct 2024 23:51:40 +0200 Subject: [PATCH 15/15] Apply formatter --- .../java/migrate/joda/JodaTimeFlowSpec.java | 62 ++++---- .../java/migrate/joda/JodaTimeVisitor.java | 4 +- .../migrate/joda/JodaTimeScannerTest.java | 150 +++++++++--------- .../migrate/joda/JodaTimeVisitorTest.java | 2 +- 4 files changed, 109 insertions(+), 109 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java index 5e3f324d5b..3639b708a3 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeFlowSpec.java @@ -25,39 +25,39 @@ public class JodaTimeFlowSpec extends DataFlowSpec { - @Override - public boolean isSource(@NonNull DataFlowNode srcNode) { - Object value = srcNode.getCursor().getParentTreeCursor().getValue(); - - if (value instanceof J.Assignment && ((J.Assignment)value).getVariable() instanceof J.Identifier) { - return isJodaType(((J.Assignment)value).getVariable().getType()); + @Override + public boolean isSource(@NonNull DataFlowNode srcNode) { + Object value = srcNode.getCursor().getParentTreeCursor().getValue(); + + if (value instanceof J.Assignment && ((J.Assignment) value).getVariable() instanceof J.Identifier) { + return isJodaType(((J.Assignment) value).getVariable().getType()); + } + + if (value instanceof J.VariableDeclarations.NamedVariable) { + return isJodaType(((J.VariableDeclarations.NamedVariable) value).getType()); + } + return false; } - if (value instanceof J.VariableDeclarations.NamedVariable) { - return isJodaType(((J.VariableDeclarations.NamedVariable)value).getType()); - } - return false; - } - - @Override - public boolean isSink(@NonNull DataFlowNode sinkNode) { - Object value = sinkNode.getCursor().getValue(); - Object parent = sinkNode.getCursor().getParentTreeCursor().getValue(); - if (parent instanceof J.MethodInvocation) { - J.MethodInvocation method = (J.MethodInvocation) parent; - return (method.getSelect() != null && method.getSelect().equals(value)) || - method.getArguments().stream().anyMatch(a -> a.equals(value)); + @Override + public boolean isSink(@NonNull DataFlowNode sinkNode) { + Object value = sinkNode.getCursor().getValue(); + Object parent = sinkNode.getCursor().getParentTreeCursor().getValue(); + if (parent instanceof J.MethodInvocation) { + J.MethodInvocation method = (J.MethodInvocation) parent; + return (method.getSelect() != null && method.getSelect().equals(value)) || + method.getArguments().stream().anyMatch(a -> a.equals(value)); + } + return parent instanceof J.VariableDeclarations.NamedVariable || + parent instanceof J.NewClass || + parent instanceof J.Assignment || + parent instanceof J.Return; } - return parent instanceof J.VariableDeclarations.NamedVariable || - parent instanceof J.NewClass || - parent instanceof J.Assignment || - parent instanceof J.Return; - } - - static boolean isJodaType(JavaType type) { - if (!(type instanceof JavaType.Class)) { - return false; + + static boolean isJodaType(JavaType type) { + if (!(type instanceof JavaType.Class)) { + return false; + } + return type.isAssignableFrom(JODA_CLASS_PATTERN); } - return type.isAssignableFrom(JODA_CLASS_PATTERN); - } } 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 38cf2dc408..e54b953449 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -131,8 +131,8 @@ public JodaTimeVisitor() { return applyTemplate(method, m, DurationTemplates.getTemplates()).orElse(method); } if (method.getSelect() != null && - method.getSelect().getType() != null && - method.getSelect().getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + method.getSelect().getType() != null && + method.getSelect().getType().isAssignableFrom(JODA_CLASS_PATTERN)) { return method; // unhandled case } if (areArgumentsAssignable(m)) { 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 5bd79cce68..5200762363 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeScannerTest.java @@ -42,26 +42,26 @@ void noUnsafeVar() { spec -> spec.recipe(toRecipe(() -> scanner)), 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 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); + } + } + """ ) ); assertTrue(scanner.getUnsafeVars().isEmpty()); @@ -75,26 +75,26 @@ void hasUnsafeVars() { spec -> spec.recipe(toRecipe(() -> scanner)), java( """ - import org.joda.time.DateTime; - import org.joda.time.DateTimeZone; - - class A { - DateTime dateTime; - 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.toDateTime()); - } - private void print(DateTime dateTime) { // method parameter not handled yet - System.out.println(dateTime); - } - } - """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + + class A { + DateTime dateTime; + 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.toDateTime()); + } + private void print(DateTime dateTime) { // method parameter not handled yet + System.out.println(dateTime); + } + } + """ ) ); // The variable dtz is unsafe due to dt. The dt variable is unsafe because its associated expression @@ -102,8 +102,8 @@ private void print(DateTime dateTime) { // method parameter not handled yet assertEquals(4, scanner.getUnsafeVars().size()); for (J.VariableDeclarations.NamedVariable var : scanner.getUnsafeVars()) { assertTrue(var.getSimpleName().equals("dtz") || - var.getSimpleName().equals("dt") || - var.getSimpleName().equals("dateTime") + var.getSimpleName().equals("dt") || + var.getSimpleName().equals("dateTime") ); } } @@ -116,23 +116,23 @@ void localVarReferencingClassVar() { // not supported yet spec -> spec.recipe(toRecipe(() -> scanner)), java( """ - import org.joda.time.DateTime; - import org.joda.time.DateTimeZone; - - class A { - DateTime dateTime; - 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 = dateTime.minus(2); - System.out.println(dt); - } - } - """ + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; + + class A { + DateTime dateTime; + 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 = dateTime.minus(2); + System.out.println(dt); + } + } + """ ) ); // The local variable dt is unsafe due to class var datetime. @@ -150,22 +150,22 @@ void localVarUsedReferencedInReturnStatement() { // not supported yet spec -> spec.recipe(toRecipe(() -> scanner)), 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); - } - } - """ + 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); + } + } + """ ) ); // The local variable dt used in return statement. 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 eee33b8def..a56da3890d 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java @@ -420,7 +420,7 @@ void migrateAbstractInstant() { java( """ import org.joda.time.DateTime; - + class A { public void foo() { new DateTime().toDate();