Skip to content

Commit

Permalink
Fix things I broke blindly accepting the suggestions from the code re…
Browse files Browse the repository at this point in the history 
…view recipe bot. Plus polish.
  • Loading branch information
@sambsnyd
sambsnyd committed Oct 4, 2024
1 parent 2e06b9b commit 0ff9b62
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 38 deletions.
4 changes: 4 additions & 0 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,7 @@ dependencies {
testRuntimeOnly("com.google.code.findbugs:jsr305:3.0.2")
testRuntimeOnly(gradleApi())
}

tasks.withType(Javadoc::class.java) {
exclude("**/PlanJavaMigration.java")
}
21 changes: 12 additions & 9 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 @@ -16,8 +16,11 @@
package org.openrewrite.java.migrate.joda;

import lombok.NonNull;
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.migrate.joda.templates.MethodTemplate;
import org.openrewrite.java.migrate.joda.templates.TimeZoneTemplates;
Expand Down Expand Up @@ -45,7 +48,7 @@ public class JodaTimeVisitor extends JavaVisitor<ExecutionContext> {
private final MethodMatcher anyDuration = new MethodMatcher(JODA_DURATION + " *(..)");

@Override
public J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) {
public @NonNull J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionContext ctx) {
maybeRemoveImport(JODA_DATE_TIME);
maybeRemoveImport(JODA_DATE_TIME_ZONE);
maybeRemoveImport(JODA_TIME_FORMAT);
Expand All @@ -67,7 +70,7 @@ public J visitCompilationUnit(@NonNull J.CompilationUnit cu, @NonNull ExecutionC
}

@Override
public J visitVariable(@NonNull J.VariableDeclarations.NamedVariable variable, @NonNull ExecutionContext ctx) {
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)) {
return variable;
Expand All @@ -76,7 +79,7 @@ public J visitVariable(@NonNull J.VariableDeclarations.NamedVariable variable, @
}

@Override
public J visitNewClass(@NonNull J.NewClass newClass, @NonNull ExecutionContext ctx) {
public @NonNull J visitNewClass(@NonNull J.NewClass newClass, @NonNull ExecutionContext ctx) {
MethodCall updated = (MethodCall) super.visitNewClass(newClass, ctx);
if (hasJodaType(updated.getArguments())) {
return newClass;
Expand All @@ -95,7 +98,7 @@ public J visitNewClass(@NonNull J.NewClass newClass, @NonNull ExecutionContext c


@Override
public J visitMethodInvocation(@NonNull J.MethodInvocation method, @NonNull ExecutionContext ctx) {
public @NonNull J visitMethodInvocation(@NonNull J.MethodInvocation method, @NonNull ExecutionContext ctx) {
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
if (hasJodaType(m.getArguments()) || isJodaVarRef(m.getSelect())) {
return method;
Expand All @@ -119,7 +122,7 @@ public J visitMethodInvocation(@NonNull J.MethodInvocation method, @NonNull Exec
}

@Override
public J visitFieldAccess(@NonNull J.FieldAccess fieldAccess, @NonNull ExecutionContext ctx) {
public @NonNull J visitFieldAccess(@NonNull J.FieldAccess fieldAccess, @NonNull ExecutionContext ctx) {
J.FieldAccess f = (J.FieldAccess) super.visitFieldAccess(fieldAccess, ctx);
if (TypeUtils.isOfClassType(f.getType(), JODA_DATE_TIME_ZONE) && f.getSimpleName().equals("UTC")) {
return JavaTemplate.builder("ZoneOffset.UTC")
Expand Down Expand Up @@ -147,14 +150,14 @@ private Optional<MethodCall> applyTemplate(MethodCall original, MethodCall updat
if (args.length == 0) {
return Optional.of(template.getTemplate().apply(updateCursor(updated), updated.getCoordinates().replace()));
}
return Optional.of(template.getTemplate().apply(updateCursor(updated), updated.getCoordinates().replace(), args));
return Optional.of(template.getTemplate().apply(updateCursor(updated), updated.getCoordinates().replace(), (Object[]) args));
}
}
return Optional.empty(); // unhandled case
}

private boolean areArgumentsAssignable(MethodCall m) {
if (m.getArguments().size() != m.getMethodType().getParameterTypes().size()) {
if (m.getMethodType() == null || m.getArguments().size() != m.getMethodType().getParameterTypes().size()) {
return false;
}
for (int i = 0; i < m.getArguments().size(); i++) {
Expand All @@ -165,8 +168,8 @@ private boolean areArgumentsAssignable(MethodCall m) {
return true;
}

private boolean isJodaVarRef(Expression expr) {
if (expr.getType() == null || !expr.getType().isAssignableFrom(JODA_CLASS_PATTERN)) {
private boolean isJodaVarRef(@Nullable Expression expr) {
if (expr == null || expr.getType() == null || !expr.getType().isAssignableFrom(JODA_CLASS_PATTERN)) {
return false;
}
if (expr instanceof J.FieldAccess) {
Expand Down
58 changes: 29 additions & 29 deletions src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import org.openrewrite.DocumentExample;
package org.openrewrite.java.migrate.joda;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;


import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.test.RewriteTest.toRecipe;

Expand All @@ -62,7 +63,6 @@ public void defaults(RecipeSpec spec) {
.parser(JavaParser.fromJavaVersion().classpath("joda-time"));
}

@DocumentExample
@DocumentExample
@Test
void migrateNewDateTime() {
Expand All @@ -73,7 +73,7 @@ void migrateNewDateTime() {
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(new DateTime());
Expand All @@ -95,7 +95,7 @@ public void foo() {
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(ZonedDateTime.now());
Expand Down Expand Up @@ -125,7 +125,7 @@ void migrateDateTimeStaticCalls() {
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(DateTime.now());
Expand All @@ -140,7 +140,7 @@ public void foo() {
import java.time.format.DateTimeFormatter;
import java.time.format.FormatStyle;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(ZonedDateTime.now());
Expand All @@ -164,7 +164,7 @@ void migrateDateTimeInstanceCalls() {
import org.joda.time.DateTimeZone;
import org.joda.time.Duration;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(new DateTime().toDateTime());
Expand Down Expand Up @@ -223,7 +223,7 @@ public void foo() {
import java.time.temporal.ChronoField;
import java.time.temporal.IsoFields;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(ZonedDateTime.now());
Expand Down Expand Up @@ -286,7 +286,7 @@ void migrateDateTimeZone() {
"""
import org.joda.time.DateTimeZone;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(DateTimeZone.UTC);
Expand All @@ -301,7 +301,7 @@ public void foo() {
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.TimeZone;
class A {
public void foo() {
System.out.println(ZoneOffset.UTC);
Expand All @@ -323,7 +323,7 @@ void migrateDateTimeFormatter() {
java(
"""
import org.joda.time.format.DateTimeFormat;
class A {
public void foo() {
System.out.println(DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"));
Expand All @@ -347,7 +347,7 @@ public void foo() {
"""
import java.time.format.DateTimeFormatter;
import java.time.format.FormatStyle;
class A {
public void foo() {
System.out.println(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"));
Expand Down Expand Up @@ -379,7 +379,7 @@ void migrateJodaDuration() {
java(
"""
import org.joda.time.Duration;
class A {
public void foo() {
System.out.println(Duration.standardDays(1L));
Expand Down Expand Up @@ -411,7 +411,7 @@ public void foo() {
"""
import java.time.Duration;
import java.time.Instant;
class A {
public void foo() {
System.out.println(Duration.ofDays(1L));
Expand Down Expand Up @@ -458,7 +458,7 @@ public void foo() {
""",
"""
import java.time.format.DateTimeFormatter;
class A {
public void foo() {
System.out.println(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"));
Expand All @@ -476,7 +476,7 @@ void migrateJodaTypeExpressionReferencingNonJodaTypeVar() {
java(
"""
import org.joda.time.DateTime;
class A {
public void foo() {
long millis = DateTime.now().getMillis();
Expand All @@ -485,15 +485,15 @@ public void foo() {
}
""",
"""
import java.time.ZonedDateTime;
class A {
public void foo() {
long millis = ZonedDateTime.now().toInstant().toEpochMilli();
System.out.println(millis);
}
}
"""
import java.time.ZonedDateTime;
class A {
public void foo() {
long millis = ZonedDateTime.now().toInstant().toEpochMilli();
System.out.println(millis);
}
}
"""
)
);
}
Expand All @@ -506,7 +506,7 @@ void dontChangeMethodAccessOnVariable() {
java(
"""
import org.joda.time.DateTime;
class A {
public void foo() {
DateTime dt = new DateTime();
Expand All @@ -529,13 +529,13 @@ void dontChangeIncompatibleType() {
java(
"""
import org.joda.time.DateTime;
class A {
public void foo() {
new B().print(new DateTime()); // print is public method accepting DateTime, not handled yet
}
}
class B {
public void print(DateTime dateTime) {
System.out.println(dateTime);
Expand All @@ -554,7 +554,7 @@ void dontChangeMethodsWithUnhandledArguments() {
"""
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
class A {
public void foo() {
// DateTimeFormat.forStyle is unhandled so parent method should not be changed
Expand Down

0 comments on commit 0ff9b62

Please sign in to comment.