Skip to content

Commit

Permalink
[3/3] Implement joda to java time migration recipe (#591)
Browse files Browse the repository at this point in the history 
* [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>
  • Loading branch information
@amishra-u @github-actions
amishra-u and github-actions[bot] authored Oct 25, 2024
1 parent 460fad7 commit ec8663c
Show file tree
Hide file tree
Showing 9 changed files with 542 additions and 108 deletions.
50 changes: 50 additions & 0 deletions src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* 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
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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<Set<NamedVariable>> {
@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<NamedVariable> getInitialValue(ExecutionContext ctx) {
return new HashSet<>();
}

@Override
public JodaTimeScanner getScanner(Set<NamedVariable> acc) {
return new JodaTimeScanner(acc);
}

@Override
public JodaTimeVisitor getVisitor(Set<NamedVariable> acc) {
return new JodaTimeVisitor(acc);
}
}
70 changes: 17 additions & 53 deletions src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,18 +34,25 @@

import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_CLASS_PATTERN;

public class JodaTimeScanner extends JavaIsoVisitor<ExecutionContext> {
public class JodaTimeScanner extends ScopeAwareVisitor {

@Getter
private final Set<NamedVariable> unsafeVars = new HashSet<>();

private final LinkedList<VariablesInScope> scopes = new LinkedList<>();
private final Set<NamedVariable> unsafeVars;

private final Map<NamedVariable, Set<NamedVariable>> varDependencies = new HashMap<>();

public JodaTimeScanner(Set<NamedVariable> unsafeVars, LinkedList<VariablesInScope> scopes) {
super(scopes);
this.unsafeVars = unsafeVars;
}

public JodaTimeScanner(Set<NamedVariable> 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<NamedVariable> allReachable = new HashSet<>();
for (NamedVariable var : unsafeVars) {
dfs(var, allReachable);
Expand All @@ -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;
}
Expand All @@ -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<Expression> 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);
Expand Down Expand Up @@ -149,27 +145,6 @@ private boolean isLocalVar(NamedVariable variable) {
return j instanceof J.Block;
}

// Returns the variable in the closest scope
private Optional<NamedVariable> 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<NamedVariable> visited) {
if (visited.contains(root)) {
return;
Expand All @@ -180,17 +155,6 @@ private void dfs(NamedVariable root, Set<NamedVariable> visited) {
}
}

@Value
private static class VariablesInScope {
Cursor scope;
Set<NamedVariable> variables;

public VariablesInScope(Cursor scope) {
this.scope = scope;
this.variables = new HashSet<>();
}
}

@RequiredArgsConstructor
private class AddSafeCheckMarker extends JavaIsoVisitor<ExecutionContext> {

Expand Down Expand Up @@ -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<NamedVariable> referencedVars = new HashSet<>();
new FindVarReferences().visit(expr, referencedVars, getCursor().getParentTreeCursor());
AtomicBoolean hasJodaType = new AtomicBoolean();
Expand Down
74 changes: 56 additions & 18 deletions src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExecutionContext> {
public class JodaTimeVisitor extends ScopeAwareVisitor {

private final MethodMatcher anyNewDateTime = new MethodMatcher(JODA_DATE_TIME + "<constructor>(..)");
private final MethodMatcher anyDateTime = new MethodMatcher(JODA_DATE_TIME + " *(..)");
Expand All @@ -40,16 +40,20 @@ public class JodaTimeVisitor extends JavaVisitor<ExecutionContext> {
private final MethodMatcher anyDuration = new MethodMatcher(JODA_DURATION + " *(..)");
private final MethodMatcher anyAbstractInstant = new MethodMatcher(JODA_ABSTRACT_INSTANT + " *(..)");

private boolean scanMode;
private final Set<NamedVariable> unsafeVars;

public JodaTimeVisitor(boolean scanMode) {
this.scanMode = scanMode;
public JodaTimeVisitor(Set<NamedVariable> unsafeVars, LinkedList<VariablesInScope> scopes) {
super(scopes);
this.unsafeVars = unsafeVars;
}

public JodaTimeVisitor() {
this(false);
public JodaTimeVisitor(Set<NamedVariable> unsafeVars) {
this(unsafeVars, new LinkedList<>());
}

public JodaTimeVisitor() {
this(new HashSet<>());
}

@Override
public @NonNull J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) {
Expand All @@ -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<NamedVariable> 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
Expand Down Expand Up @@ -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<NamedVariable> mayBeVar = findVarInScope(ident.getSimpleName());
if (!mayBeVar.isPresent() || unsafeVars.contains(mayBeVar.get())) {
return ident;
}

Expand Down
Loading

0 comments on commit ec8663c

Please sign in to comment.