diff --git a/src/main/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedException.java b/src/main/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedException.java new file mode 100644 index 000000000..a430e3723 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedException.java @@ -0,0 +1,66 @@ +/* + * 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; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.ChangeType; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.search.FindMethods; +import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.TypeUtils; + +public class IllegalArgumentExceptionToAlreadyConnectedException extends Recipe { + + private static final String ILLEGAL_ARGUMENT_EXCEPTION = "java.lang.IllegalArgumentException"; + private static final String ALREADY_CONNECTED_EXCEPTION = "java.nio.channels.AlreadyConnectedException"; + + @Override + public String getDisplayName() { + return "Replace `IllegalArgumentException` with `AlreadyConnectedException` in `DatagramChannel.send()` method"; + } + + @Override + public String getDescription() { + return "Replace `IllegalArgumentException` with `AlreadyConnectedException` for DatagramChannel.send() to ensure compatibility with Java 11+."; + } + + @Override + public TreeVisitor getVisitor() { + String datagramChannelSendMethodPattern = "java.nio.channels.DatagramChannel send(java.nio.ByteBuffer, java.net.SocketAddress)"; + return Preconditions.check(new UsesMethod<>(datagramChannelSendMethodPattern), new JavaIsoVisitor() { + @Override + public J.Try visitTry(J.Try tryStatement, ExecutionContext ctx) { + J.Try try_ = super.visitTry(tryStatement, ctx); + if (FindMethods.find(try_, datagramChannelSendMethodPattern).isEmpty()) { + return try_; + } + return try_.withCatches(ListUtils.map(try_.getCatches(), catch_ -> { + if (TypeUtils.isOfClassType(catch_.getParameter().getType(), ILLEGAL_ARGUMENT_EXCEPTION)) { + maybeAddImport(ALREADY_CONNECTED_EXCEPTION); + return (J.Try.Catch) new ChangeType(ILLEGAL_ARGUMENT_EXCEPTION, ALREADY_CONNECTED_EXCEPTION, true) + .getVisitor().visit(catch_, ctx); + } + return catch_; + })); + } + }); + } +} diff --git a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java index c9634697f..8a2936725 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeRecipe.java @@ -54,6 +54,7 @@ public JodaTimeVisitor getVisitor(Accumulator acc) { @Getter public static class Accumulator { private final Set unsafeVars = new HashSet<>(); + private final Map safeMethodMap = new HashMap<>(); private final VarTable varTable = new VarTable(); } 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 643620351..0f0132c00 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeScanner.java @@ -19,6 +19,7 @@ import lombok.Getter; import lombok.NonNull; import lombok.RequiredArgsConstructor; +import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; @@ -43,6 +44,8 @@ class JodaTimeScanner extends ScopeAwareVisitor { private final Map> varDependencies = new HashMap<>(); private final Map> unsafeVarsByType = new HashMap<>(); + private final Map> methodReferencedVars = new HashMap<>(); + private final Map> methodUnresolvedReferencedVars = new HashMap<>(); public JodaTimeScanner(JodaTimeRecipe.Accumulator acc) { super(new LinkedList<>()); @@ -57,13 +60,30 @@ public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { dfs(var, allReachable); } acc.getUnsafeVars().addAll(allReachable); + + Set unsafeMethods = new HashSet<>(); + acc.getSafeMethodMap().forEach((method, isSafe) -> { + if (!isSafe) { + unsafeMethods.add(method); + return; + } + Set intersection = new HashSet<>(methodReferencedVars.getOrDefault(method, Collections.emptySet())); + intersection.retainAll(acc.getUnsafeVars()); + if (!intersection.isEmpty()) { + unsafeMethods.add(method); + } + }); + for (JavaType.Method method : unsafeMethods) { + acc.getSafeMethodMap().put(method, false); + acc.getUnsafeVars().addAll(methodReferencedVars.getOrDefault(method, Collections.emptySet())); + } return cu; } @Override - public NamedVariable visitVariable(NamedVariable variable, ExecutionContext ctx) { + public J visitVariable(NamedVariable variable, ExecutionContext ctx) { if (!variable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { - return (NamedVariable) super.visitVariable(variable, ctx); + return super.visitVariable(variable, ctx); } // TODO: handle class variables if (isClassVar(variable)) { @@ -96,27 +116,27 @@ public NamedVariable visitVariable(NamedVariable variable, ExecutionContext ctx) } @Override - public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + public J 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; + return super.visitAssignment(assignment, ctx); } J.Identifier ident = (J.Identifier) var; Optional mayBeVar = findVarInScope(ident.getSimpleName()); if (!mayBeVar.isPresent()) { - return assignment; + return super.visitAssignment(assignment, ctx); } NamedVariable variable = mayBeVar.get(); Cursor varScope = findScope(variable); List sinks = findSinks(new Cursor(getCursor(), assignment.getAssignment())); new AddSafeCheckMarker(sinks).visit(varScope.getValue(), ctx, varScope.getParentOrThrow()); processMarkersOnExpression(sinks, variable); - return assignment; + return super.visitAssignment(assignment, ctx); } @Override - public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + public J visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { acc.getVarTable().addVars(method); unsafeVarsByType.getOrDefault(method.getMethodType(), Collections.emptySet()).forEach(varName -> { NamedVariable var = acc.getVarTable().getVarByName(method.getMethodType(), varName); @@ -124,7 +144,84 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex acc.getUnsafeVars().add(var); } }); - return (J.MethodDeclaration) super.visitMethodDeclaration(method, ctx); + Set unresolvedVars = methodUnresolvedReferencedVars.remove(method.getMethodType()); + if (unresolvedVars != null) { + unresolvedVars.forEach(var -> { + NamedVariable namedVar = acc.getVarTable().getVarByName(var.getDeclaringType(), var.getVarName()); + if (namedVar != null) { + methodReferencedVars.computeIfAbsent(method.getMethodType(), k -> new HashSet<>()).add(namedVar); + } + }); + } + return super.visitMethodDeclaration(method, ctx); + } + + @Override + public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + if (!isJodaExpr(method) || method.getMethodType().getDeclaringType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return super.visitMethodInvocation(method, ctx); + } + Cursor boundary = findBoundaryCursorForJodaExpr(getCursor()); + J j = new JodaTimeVisitor(new JodaTimeRecipe.Accumulator(), false, scopes) + .visit(boundary.getValue(), ctx, boundary.getParentTreeCursor()); + + boolean isSafe = j != boundary.getValue(); + acc.getSafeMethodMap().compute(method.getMethodType(), (k, v) -> v == null ? isSafe : v && isSafe); + J parent = boundary.getParentTreeCursor().getValue(); + if (parent instanceof NamedVariable) { + methodReferencedVars.computeIfAbsent(method.getMethodType(), k -> new HashSet<>()) + .add((NamedVariable) parent); + } + if (parent instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) parent; + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier ident = (J.Identifier) assignment.getVariable(); + findVarInScope(ident.getSimpleName()) + .map(var -> methodReferencedVars.computeIfAbsent(method.getMethodType(), k -> new HashSet<>()).add(var)); + } + } + if (parent instanceof MethodCall) { + MethodCall parentMethod = (MethodCall) parent; + int argPos = parentMethod.getArguments().indexOf(boundary.getValue()); + if (argPos == -1) { + return method; + } + String paramName = parentMethod.getMethodType().getParameterNames().get(argPos); + NamedVariable var = acc.getVarTable().getVarByName(parentMethod.getMethodType(), paramName); + if (var != null) { + methodReferencedVars.computeIfAbsent(method.getMethodType(), k -> new HashSet<>()).add(var); + } else { + methodUnresolvedReferencedVars.computeIfAbsent(method.getMethodType(), k -> new HashSet<>()) + .add(new UnresolvedVar(parentMethod.getMethodType(), paramName)); + } + } + return method; + } + + @Override + public J.Return visitReturn(J.Return _return, ExecutionContext ctx) { + if (_return.getExpression() == null) { + return _return; + } + Expression expr = _return.getExpression(); + if (!isJodaExpr(expr)) { + return _return; + } + J methodOrLambda = getCursor().dropParentUntil(j -> j instanceof J.MethodDeclaration || j instanceof J.Lambda).getValue(); + if (methodOrLambda instanceof J.Lambda) { + return _return; + } + J.MethodDeclaration method = (J.MethodDeclaration) methodOrLambda; + Expression updatedExpr = (Expression) new JodaTimeVisitor(acc, true, scopes) + .visit(expr, ctx, getCursor().getParentTreeCursor()); + boolean isSafe = !isJodaExpr(updatedExpr); + + addReferencedVars(expr, method.getMethodType()); + acc.getSafeMethodMap().compute(method.getMethodType(), (k, v) -> v == null ? isSafe : v && isSafe); + if (!isSafe) { + acc.getUnsafeVars().addAll(methodReferencedVars.get(method.getMethodType())); + } + return _return; } private void processMarkersOnExpression(List expressions, NamedVariable var) { @@ -146,7 +243,23 @@ private void processMarkersOnExpression(List expressions, NamedVaria } } - private boolean isJodaExpr(Expression expression) { + /** + * 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 static Cursor findBoundaryCursorForJodaExpr(Cursor cursor) { + 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 static boolean isJodaExpr(Expression expression) { return expression.getType() != null && expression.getType().isAssignableFrom(JODA_CLASS_PATTERN); } @@ -172,6 +285,13 @@ private void dfs(NamedVariable root, Set visited) { } } + private void addReferencedVars(Expression expr, JavaType.Method method) { + Set<@Nullable NamedVariable> referencedVars = new HashSet<>(); + new FindVarReferences().visit(expr, referencedVars, getCursor().getParentTreeCursor()); + referencedVars.remove(null); + methodReferencedVars.computeIfAbsent(method, k -> new HashSet<>()).addAll(referencedVars); + } + @RequiredArgsConstructor private class AddSafeCheckMarker extends JavaIsoVisitor { @@ -205,11 +325,12 @@ private SafeCheckMarker getMarker(Expression expr, ExecutionContext ctx) { return mayBeMarker.get(); } - Cursor boundary = findBoundaryCursorForJodaExpr(); + Cursor boundary = findBoundaryCursorForJodaExpr(getCursor()); boolean isSafe = true; - // TODO: handle return statement if (boundary.getParentTreeCursor().getValue() instanceof J.Return) { - isSafe = false; + // TODO: handle return statement in lambda + isSafe = boundary.dropParentUntil(j -> j instanceof J.MethodDeclaration || j instanceof J.Lambda) + .getValue() instanceof J.MethodDeclaration; } Expression boundaryExpr = boundary.getValue(); J j = new JodaTimeVisitor(new JodaTimeRecipe.Accumulator(), false, scopes) @@ -223,23 +344,6 @@ private SafeCheckMarker getMarker(Expression expr, ExecutionContext ctx) { 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 Optional findArgumentExprCursor() { Cursor cursor = getCursor(); while (cursor.getValue() instanceof Expression && isJodaExpr(cursor.getValue())) { @@ -283,4 +387,10 @@ public Expression visitExpression(Expression expression, AtomicBoolean hasJodaTy return super.visitExpression(expression, hasJodaType); } } + + @Value + private static class UnresolvedVar { + JavaType declaringType; + String varName; + } } 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 d396f3452..b84b09d49 100644 --- a/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java +++ b/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java @@ -84,6 +84,22 @@ public Javadoc visitReference(Javadoc.Reference reference, ExecutionContext ctx) return super.visitCompilationUnit(cu, ctx); } + @Override + public @NonNull J visitMethodDeclaration(@NonNull J.MethodDeclaration method, @NonNull ExecutionContext ctx) { + J.MethodDeclaration m = (J.MethodDeclaration) super.visitMethodDeclaration(method, ctx); + if (m.getReturnTypeExpression() == null || !m.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return m; + } + if (safeMigration && !acc.getSafeMethodMap().getOrDefault(m.getMethodType(), false)) { + return m; + } + + JavaType.Class returnType = TimeClassMap.getJavaTimeType(((JavaType.Class) m.getType()).getFullyQualifiedName()); + J.Identifier returnExpr = TypeTree.build(returnType.getClassName()).withType(returnType).withPrefix(Space.format(" ")); + return m.withReturnTypeExpression(returnExpr) + .withMethodType(m.getMethodType().withReturnType(returnType)); + } + @Override public @NonNull J visitVariableDeclarations(@NonNull J.VariableDeclarations multiVariable, @NonNull ExecutionContext ctx) { if (multiVariable.getTypeExpression() == null || !multiVariable.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { @@ -147,6 +163,13 @@ public Javadoc visitReference(Javadoc.Reference reference, ExecutionContext ctx) @Override public @NonNull J visitMethodInvocation(@NonNull J.MethodInvocation method, @NonNull ExecutionContext ctx) { J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + + // internal method with Joda class as return type + if (!method.getMethodType().getDeclaringType().isAssignableFrom(JODA_CLASS_PATTERN) && + method.getType().isAssignableFrom(JODA_CLASS_PATTERN)) { + return migrateNonJodaMethod(method, m); + } + if (hasJodaType(m.getArguments()) || isJodaVarRef(m.getSelect())) { return method; } @@ -179,7 +202,9 @@ public Javadoc visitReference(Javadoc.Reference reference, ExecutionContext ctx) JavaType.FullyQualified jodaType = ((JavaType.Class) ident.getType()); JavaType.FullyQualified fqType = TimeClassMap.getJavaTimeType(jodaType.getFullyQualifiedName()); - + if (fqType == null) { + return ident; + } return ident.withType(fqType) .withFieldType(ident.getFieldType().withType(fqType)); } @@ -218,6 +243,19 @@ private J migrateMethodCall(MethodCall original, MethodCall updated) { return original; } + private J.MethodInvocation migrateNonJodaMethod(J.MethodInvocation original, J.MethodInvocation updated) { + if (safeMigration && !acc.getSafeMethodMap().getOrDefault(updated.getMethodType(), false)) { + return original; + } + JavaType.Class returnType = (JavaType.Class) updated.getMethodType().getReturnType(); + JavaType.Class updatedReturnType = TimeClassMap.getJavaTimeType(returnType.getFullyQualifiedName()); + if (updatedReturnType == null) { + return original; // unhandled case + } + return updated.withMethodType(updated.getMethodType().withReturnType(updatedReturnType)) + .withName(updated.getName().withType(updatedReturnType)); + } + private boolean hasJodaType(List exprs) { for (Expression expr : exprs) { JavaType exprType = expr.getType(); diff --git a/src/main/resources/META-INF/rewrite/java-version-11.yml b/src/main/resources/META-INF/rewrite/java-version-11.yml index 2ce31ac4a..999637af8 100644 --- a/src/main/resources/META-INF/rewrite/java-version-11.yml +++ b/src/main/resources/META-INF/rewrite/java-version-11.yml @@ -74,6 +74,7 @@ recipeList: - org.openrewrite.java.migrate.ReplaceComSunAWTUtilitiesMethods - org.openrewrite.java.migrate.ReplaceLocalizedStreamMethods - org.openrewrite.java.migrate.ArrayStoreExceptionToTypeNotPresentException + - org.openrewrite.java.migrate.IllegalArgumentExceptionToAlreadyConnectedException --- type: specs.openrewrite.org/v1beta/recipe @@ -290,4 +291,3 @@ recipeList: - org.openrewrite.java.ChangeMethodName: methodPattern: java.nio.file.Path get(..) newMethodName: of - diff --git a/src/main/resources/META-INF/rewrite/java-version-17.yml b/src/main/resources/META-INF/rewrite/java-version-17.yml index 3d8b68144..0299c8f1e 100644 --- a/src/main/resources/META-INF/rewrite/java-version-17.yml +++ b/src/main/resources/META-INF/rewrite/java-version-17.yml @@ -52,6 +52,10 @@ recipeList: groupId: com.google.inject artifactId: guice newVersion: 5.x + - org.openrewrite.java.dependencies.UpgradeDependencyVersion: + groupId: commons-codec + artifactId: commons-codec + newVersion: 1.17.x - org.openrewrite.java.migrate.AddLombokMapstructBinding --- diff --git a/src/test/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedExceptionTest.java b/src/test/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedExceptionTest.java new file mode 100644 index 000000000..1fd0d0214 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/IllegalArgumentExceptionToAlreadyConnectedExceptionTest.java @@ -0,0 +1,166 @@ +/* + * 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; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class IllegalArgumentExceptionToAlreadyConnectedExceptionTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new IllegalArgumentExceptionToAlreadyConnectedException()); + } + + @DocumentExample + @Test + void catchException() { + rewriteRun( + //language=java + java( + """ + import java.nio.ByteBuffer; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + class Test { + void sendDataCatch() { + try { + DatagramChannel channel = DatagramChannel.open(); + channel.send(ByteBuffer.allocate(1024), new java.net.InetSocketAddress("localhost", 8080)); + } catch (IllegalArgumentException e) { + System.out.println("Caught Exception"); + } + } + } + """, + """ + import java.nio.ByteBuffer; + import java.nio.channels.AlreadyConnectedException; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + class Test { + void sendDataCatch() { + try { + DatagramChannel channel = DatagramChannel.open(); + channel.send(ByteBuffer.allocate(1024), new java.net.InetSocketAddress("localhost", 8080)); + } catch (AlreadyConnectedException e) { + System.out.println("Caught Exception"); + } + } + } + """ + ) + ); + } + + @Test + void rethrowException() { + rewriteRun( + //language=java + java( + """ + import java.nio.ByteBuffer; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + class Test { + void sendDataRethrow() { + try { + DatagramChannel channel = DatagramChannel.open(); + channel.send(ByteBuffer.allocate(1024), new java.net.InetSocketAddress("localhost", 8080)); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("DatagramChannel already connected to a different address"); + } + } + } + """, + """ + import java.nio.ByteBuffer; + import java.nio.channels.AlreadyConnectedException; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + class Test { + void sendDataRethrow() { + try { + DatagramChannel channel = DatagramChannel.open(); + channel.send(ByteBuffer.allocate(1024), new java.net.InetSocketAddress("localhost", 8080)); + } catch (AlreadyConnectedException e) { + throw new AlreadyConnectedException("DatagramChannel already connected to a different address"); + } + } + } + """ + ) + ); + } + + @Test + void retainOtherCaughtExceptions() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.nio.ByteBuffer; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + class Test { + void sendData() { + try { + DatagramChannel channel = DatagramChannel.open(); + channel.send(ByteBuffer.allocate(1024), new java.net.InetSocketAddress("localhost", 8080)); + } catch (IOException e) { + System.out.println("Caught Exception"); + } + } + } + """ + ) + ); + } + + @Test + void retainIllegalArgumentExceptionWithoutChannelSend() { + rewriteRun( + //language=java + java( + """ + import java.nio.ByteBuffer; + import java.net.SocketAddress; + import java.nio.channels.DatagramChannel; + + public class Test { + public void sendData() { + try { + DatagramChannel channel = DatagramChannel.open(); + } catch (IllegalArgumentException e) { + System.out.println("Caught Exception"); + } + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java index 96873be82..4fa27d21e 100644 --- a/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java +++ b/src/test/java/org/openrewrite/java/migrate/joda/JodaTimeRecipeTest.java @@ -198,13 +198,13 @@ private void print(Date date) { } @Test - void localVarUsedReferencedInReturnStatement() { // not supported yet + void localVarUsedReferencedInReturnStatement() { // language=java rewriteRun( java( """ - import org.joda.time.DateTime; - import org.joda.time.DateTimeZone; + import org.joda.time.DateTime; + import org.joda.time.DateTimeZone; class A { public DateTime foo(String city) { @@ -218,6 +218,24 @@ public DateTime foo(String city) { return dt.plus(2); } } + """, + """ + import java.time.Duration; + import java.time.ZoneId; + import java.time.ZonedDateTime; + + class A { + public ZonedDateTime 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); + return dt.plus(Duration.ofMillis(2)); + } + } """ ) ); @@ -262,6 +280,105 @@ public void bar(ZonedDateTime dt) { ); } + @Test + void migrateMethodWithSafeReturnExpression() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import org.joda.time.Interval; + + class A { + public DateTime foo(DateTime dt) { + Interval interval = new Interval(dt, dt.plusDays(1)); + return interval.getEnd(); + } + + private static class Bar { + public void bar(DateTime dt) { + DateTime d = foo(dt); + System.out.println(d.getMillis()); + } + } + } + """, + """ + import org.threeten.extra.Interval; + + import java.time.ZoneId; + import java.time.ZonedDateTime; + + class A { + public ZonedDateTime foo(ZonedDateTime dt) { + Interval interval = Interval.of(dt.toInstant(), dt.plusDays(1).toInstant()); + return interval.getEnd().atZone(ZoneId.systemDefault()); + } + + private static class Bar { + public void bar(ZonedDateTime dt) { + ZonedDateTime d = foo(dt); + System.out.println(d.toInstant().toEpochMilli()); + } + } + } + """ + ) + ); + } + + @Test + void migrateMethodWithSafeReturnExpressionAndUnsafeParam() { + //language=java + rewriteRun( + java( + """ + import org.joda.time.DateTime; + import org.joda.time.Interval; + + class A { + public DateTime foo(DateTime dt) { + DateTime d = dt.toDateMidnight(); + DateTime d2 = DateTime.now(); + Interval interval = new Interval(d2, d2.plusDays(1)); + return interval.getEnd(); + } + + private static class Bar { + public void bar() { + DateTime d = foo(new DateTime()); + System.out.println(d.getMillis()); + } + } + } + """, + """ + import org.joda.time.DateTime; + import org.threeten.extra.Interval; + + import java.time.ZoneId; + import java.time.ZonedDateTime; + + class A { + public ZonedDateTime foo(DateTime dt) { + DateTime d = dt.toDateMidnight(); + ZonedDateTime d2 = ZonedDateTime.now(); + Interval interval = Interval.of(d2.toInstant(), d2.plusDays(1).toInstant()); + return interval.getEnd().atZone(ZoneId.systemDefault()); + } + + private static class Bar { + public void bar() { + ZonedDateTime d = foo(new DateTime()); + System.out.println(d.toInstant().toEpochMilli()); + } + } + } + """ + ) + ); + } + @Test void doNotMigrateUnsafeMethodParam() { //language=java 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 b5acde58a..4dd326c17 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.Map; + import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -142,7 +144,7 @@ public void foo(String city) { } @Test - void localVarUsedReferencedInReturnStatement() { // not supported yet + void localVarUsedReferencedInReturnStatement() { JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); // language=java rewriteRun( @@ -167,11 +169,7 @@ public DateTime foo(String city) { """ ) ); - // The local variable dt used in return statement. - assertEquals(2, scanner.getAcc().getUnsafeVars().size()); - for (J.VariableDeclarations.NamedVariable var : scanner.getAcc().getUnsafeVars()) { - assertTrue(var.getSimpleName().equals("dtz") || var.getSimpleName().equals("dt")); - } + assertThat(scanner.getAcc().getUnsafeVars()).isEmpty(); } @Test @@ -213,22 +211,22 @@ void detectUnsafeVarsInInitializer() { spec -> spec.recipe(toRecipe(() -> scanner)), java( """ - import org.joda.time.Interval; + import org.joda.time.Period; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.List; class A { - public Interval interval() { - return new Interval(10, 20); + public Period period() { + return new Period(); } public void foo() { List list = Stream.of(1, 2, 3).peek(i -> { - Interval i1 = interval(); - Interval i2 = new Interval(i, 100); - if (i1 != null && !i1.contains(i2)) { - System.out.println("i1 does not contain i2"); + Period p1 = period(); + Period p2 = new Period(i, 100); + if (p1 != null && p1.plus(p2).getDays() > 10) { + System.out.println("Hello world!"); } }).toList(); } @@ -238,7 +236,7 @@ public void foo() { ); assertThat(scanner.getAcc().getUnsafeVars().stream().map(J.VariableDeclarations.NamedVariable::getSimpleName)) .hasSize(2) - .containsExactlyInAnyOrder("i1", "i2"); + .containsExactlyInAnyOrder("p1", "p2"); } @Test @@ -249,28 +247,28 @@ void detectUnsafeVarsInChainedLambdaExpressions() { spec -> spec.recipe(toRecipe(() -> scanner)), java( """ - import org.joda.time.Interval; + import org.joda.time.Period; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.List; class A { - public Interval interval() { - return new Interval(10, 20); + public Period period() { + return new Period(); } public void foo() { List list = Stream.of(1, 2, 3).peek(i -> { - Interval i1 = interval(); - Interval i2 = new Interval(i, 100); - if (i1 != null && !i1.contains(i2)) { - System.out.println("i1 does not contain i2"); + Period p1 = period(); + Period p2 = new Period(i, 100); + if (p1 != null && p1.plus(p2).getDays() > 10) { + System.out.println("Hello world!"); } }).peek(i -> { - Interval i3 = interval(); - Interval i4 = new Interval(i, 100); - if (i3 != null && !i3.contains(i4)) { - System.out.println("i3 does not contain i4"); + Period p3 = period(); + Period p4 = new Period(i, 100); + if (p3 != null && p3.plus(p4).getDays() > 10) { + System.out.println("Hello world!"); } }).toList(); } @@ -280,6 +278,150 @@ public void foo() { ); assertThat(scanner.getAcc().getUnsafeVars().stream().map(J.VariableDeclarations.NamedVariable::getSimpleName)) .hasSize(4) - .containsExactlyInAnyOrder("i1", "i2", "i3", "i4"); + .containsExactlyInAnyOrder("p1", "p2", "p3", "p4"); + } + + @Test + void hasSafeMethods() { + JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + + class A { + private DateTime dateTime() { + DateTime dt = new DateTime(); + return dt; + } + public void print() { + System.out.println(dateTime()); + } + } + """ + ) + ); + assertThat(scanner.getAcc().getSafeMethodMap()).hasSize(1); + assertThat(scanner.getAcc().getSafeMethodMap().entrySet().stream().filter(Map.Entry::getValue).map(e -> e.getKey().toString())) + .containsExactlyInAnyOrder("A{name=dateTime,return=org.joda.time.DateTime,parameters=[]}"); + assertThat(scanner.getAcc().getUnsafeVars()).isEmpty(); + } + + @Test + void methodInvocationBeforeDeclaration() { + JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + + class A { + public void print() { + System.out.println(dateTime()); + } + private DateTime dateTime() { + DateTime dt = new DateTime(); + return dt; + } + } + """ + ) + ); + assertThat(scanner.getAcc().getSafeMethodMap()).hasSize(1); + assertThat(scanner.getAcc().getSafeMethodMap().entrySet().stream().filter(Map.Entry::getValue).map(e -> e.getKey().toString())) + .containsExactlyInAnyOrder("A{name=dateTime,return=org.joda.time.DateTime,parameters=[]}"); + assertThat(scanner.getAcc().getUnsafeVars()).isEmpty(); + } + + @Test + void safeMethodWithUnsafeParam() { + JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + import org.joda.time.Period; + + class A { + private DateTime dateTime(Period period) { + DateTime dt = new DateTime(); + System.out.println(period); + return dt; + } + public void print() { + System.out.println(dateTime(new Period())); + } + } + """ + ) + ); + assertThat(scanner.getAcc().getSafeMethodMap()).hasSize(1); + assertThat(scanner.getAcc().getSafeMethodMap().entrySet().stream().filter(Map.Entry::getValue).map(e -> e.getKey().toString())) + .containsExactlyInAnyOrder("A{name=dateTime,return=org.joda.time.DateTime,parameters=[org.joda.time.Period]}"); + assertThat(scanner.getAcc().getUnsafeVars().stream().map(J.VariableDeclarations.NamedVariable::getSimpleName)) + .containsExactlyInAnyOrder("period"); + } + + @Test + void unsafeMethodDueToUnhandledUsage() { + JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + + class A { + private DateTime dateTime() { + DateTime dt = new DateTime(); + return dt; + } + public void print() { + dateTime().toDateMidnight(); + } + } + """ + ) + ); + assertThat(scanner.getAcc().getSafeMethodMap()).hasSize(1); + assertThat(scanner.getAcc().getSafeMethodMap().entrySet().stream().filter(Map.Entry::getValue)).isEmpty(); + assertThat(scanner.getAcc().getUnsafeVars().stream().map(J.VariableDeclarations.NamedVariable::getSimpleName)) + .containsExactlyInAnyOrder("dt"); + } + + @Test + void unsafeMethodDueToIndirectUnhandledUsage() { + JodaTimeScanner scanner = new JodaTimeScanner(new JodaTimeRecipe.Accumulator()); + // language=java + rewriteRun( + spec -> spec.recipe(toRecipe(() -> scanner)), + java( + """ + import org.joda.time.DateTime; + + class A { + private DateTime dateTime() { + DateTime dt = new DateTime(); + return dt; + } + public void print() { + DateTime dt = dateTime(); + dt.toDateMidnight(); + } + } + """ + ) + ); + assertThat(scanner.getAcc().getSafeMethodMap()).hasSize(1); + assertThat(scanner.getAcc().getSafeMethodMap().entrySet().stream().filter(Map.Entry::getValue)).isEmpty(); + assertThat(scanner.getAcc().getUnsafeVars().stream().map(J.VariableDeclarations.NamedVariable::getSimpleName)) + .containsExactlyInAnyOrder("dt", "dt"); } }