Add mapping for migration for joda LocalDate #677

YarochkinMichael · 2025-02-18T06:44:57Z

What's changed?

Added mapping for joda LocalDate to java LocalDate

Anyone you would like to review specifically?

@amishra-u
@timtebeek
Recipe is still not updating the field as it is used further in the code.
In the snippet below, for example;

il1 will not be updated, if2 will.
when I use id2 on line 3, then it will also be skipped, but ld1 updated
if i remove line 3 both line 1 and 2 will be updated

LocalDate ld1 = new LocalDate();
LocalDate ld2 = new LocalDate(0L);
LocalDate ld7 = ld1.plusDays(5);

Checklist

No unit tests

YarochkinMichael · 2025-02-18T07:02:15Z

The update issue is fixed by the second commit

YarochkinMichael · 2025-02-18T17:03:44Z

Add LocalDateTime mapping
Add tests

amishra-u · 2025-02-18T17:07:52Z

src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java

@@ -219,10 +223,14 @@ private J migrateMethodCall(MethodCall original, MethodCall updated) {
        }
        MethodTemplate template = AllTemplates.getTemplate(original);
        if (template == null) {
+            //is it a better way to print logs?


Can you generate datatable for unhandled case?
https://docs.openrewrite.org/authoring-recipes/data-tables
or atleast use ExecutionContext messaging for unhandled case.
https://javadoc.io/doc/org.openrewrite/rewrite-core/8.19.0/org/openrewrite/InMemoryExecutionContext.html

Not sure how big your repo is but the datatable will be very useful for large repos.

Can you give me example usage of how to print log line please?

Hi @YarochkinMichael 👋 you can find out documentation on how to generate data tables from recipes here https://docs.openrewrite.org/authoring-recipes/data-tables#generating-the-data-table

amishra-u · 2025-02-18T17:10:08Z

src/main/java/org/openrewrite/java/migrate/joda/templates/LocatDateTemplates.java

+        }
+    };
+
+    private JavaTemplate build(JavaTemplate.Builder builder) {


amishra-u · 2025-02-18T17:23:20Z

src/test/java/org/openrewrite/java/migrate/joda/NoJodaTimeTest.java

@@ -43,12 +43,16 @@ void migrateJodaTime() {
              java(


Could you add a test case for each template? This will at least ensure basic verification for each one.

FYI: If you use GitHub Copilot or ChatGPT, you’ll be surprised at how well they generate these test cases.

I added a no-arg constructor. locally, for testing, I have a project with examples. After I finish migration, I will have more time to move it to tests.

Most of my code is generated by ChatGPT. ;)

Hi @YarochkinMichael tests are crucial for us and we require a decent test coverage.
Please see our guides here on creating RewriteTests easily.

YarochkinMichael · 2025-02-19T07:47:27Z

Add LocalTime mapping

Add Hours mapping

YarochkinMichael · 2025-02-21T13:03:15Z

I was able to fix all my use cases. Not all Joda types are mapped, but now it is printing message about missing mapping.

@amishra-u please have a look into my last commit. on my side class variables are migrated without any issues.

TODO: add tests. i have a local project I was using for testing. Need to turn it into unit tests.

github-actions · 2025-02-23T07:39:51Z

src/main/java/org/openrewrite/java/migrate/joda/templates/DaysTemplates.java

+import java.util.ArrayList;
+import java.util.List;
+
+import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*;


Suggested change

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

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

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

github-actions · 2025-02-23T07:39:51Z

src/main/java/org/openrewrite/java/migrate/joda/templates/HoursTemplates.java

+import java.util.ArrayList;
+import java.util.List;
+
+import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*;


Suggested change

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

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

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

github-actions · 2025-02-23T07:39:51Z

src/main/java/org/openrewrite/java/migrate/joda/templates/SecondsTemplates.java

+import org.openrewrite.java.tree.Expression;
+


Suggested change

import org.openrewrite.java.tree.Expression;

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

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

YarochkinMichael · 2025-02-23T21:50:47Z

@amishra-u can you pls check my "fix" for class wars?

YarochkinMichael · 2025-02-23T21:58:14Z

Not only did the tests with my changes fail.

amishra-u · 2025-02-24T20:16:27Z

Not only did the tests with my changes fail.

Yes, it failed as expected

Refer to this field to disable safeMigration a better way.

rewrite-migrate-java/src/main/java/org/openrewrite/java/migrate/joda/JodaTimeVisitor.java

Line 44 in 548f4db

this.safeMigration = safeMigration;

amishra-u · 2025-02-24T20:27:02Z

Before this PR can be merged, please address the following:

Ensure unit tests are included—they are essential.
Avoid merging multiple features in one PR. For example, adding new templates and disabling optional safe migration should be separate.

Alternatively, you can create a local copy of the JAR with these changes and use it in your project, skipping the PR process. If someone else needs the same feature later, they can take over and upstream it.

PS: I’m not on the maintainer list—this is just my general advice.

YarochkinMichael · 2025-02-25T09:27:59Z

@amishra-u
thank you for the reply

My question about "safe" is more about whether it is possible just to disable it. If so, I will remove it completely. The code you see now is mostly for the test. And allow me to continue my work, of course. :)

Removing the "safe" feature is a part of migration. I can extract it in a separate PR. But it makes no sense to me.

Add mapping for migration for joda LocalDate

bce32bb

Mikhail.Yarochkin and others added 2 commits February 18, 2025 08:52

Fix safe migration issue.

0ed4bf2

Merge branch 'main' into add-localdate-mapping-to-joda-recipe

fa80bf7

timtebeek self-requested a review February 18, 2025 12:53

timtebeek assigned YarochkinMichael Feb 18, 2025

Add LocalDateTime mapping

4d2d32f

amishra-u reviewed Feb 18, 2025

View reviewed changes

Add LocalTime mapping

d058821

Mikhail.Yarochkin added 3 commits February 19, 2025 17:18

Add Seconds mapping

d5f61bc

Add Days mapping

e16c12d

Add Hours mapping

Add tests

ccbc050

YarochkinMichael mentioned this pull request Feb 20, 2025

org.openrewrite.java.migrate.joda.JodaTimeRecipe not changing anything #673

Open

Disable safe check in Visitor

37eb88c

github-actions bot reviewed Feb 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mapping for migration for joda LocalDate #677

Add mapping for migration for joda LocalDate #677

YarochkinMichael commented Feb 18, 2025

YarochkinMichael commented Feb 18, 2025 •

edited

Loading

YarochkinMichael commented Feb 18, 2025

amishra-u Feb 18, 2025

YarochkinMichael Feb 18, 2025 •

edited by jevanlingen

Loading

MBoegers Feb 20, 2025

amishra-u Feb 18, 2025

amishra-u Feb 18, 2025

YarochkinMichael Feb 19, 2025

YarochkinMichael Feb 19, 2025

MBoegers Feb 20, 2025

YarochkinMichael commented Feb 19, 2025

YarochkinMichael commented Feb 21, 2025

github-actions bot Feb 23, 2025

github-actions bot Feb 23, 2025

github-actions bot Feb 23, 2025

YarochkinMichael commented Feb 23, 2025

YarochkinMichael commented Feb 23, 2025

amishra-u commented Feb 24, 2025

amishra-u commented Feb 24, 2025 •

edited

Loading

YarochkinMichael commented Feb 25, 2025

	import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.*;
	import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JAVA_PERIOD;
	import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_DAYS;

	import org.openrewrite.java.tree.Expression;
	import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JAVA_DURATION;
	import static org.openrewrite.java.migrate.joda.templates.TimeClassNames.JODA_SECONDS;

Add mapping for migration for joda LocalDate #677

Are you sure you want to change the base?

Add mapping for migration for joda LocalDate #677

Conversation

YarochkinMichael commented Feb 18, 2025

What's changed?

Anyone you would like to review specifically?

Checklist

YarochkinMichael commented Feb 18, 2025 • edited Loading

YarochkinMichael commented Feb 18, 2025

amishra-u Feb 18, 2025

Choose a reason for hiding this comment

YarochkinMichael Feb 18, 2025 • edited by jevanlingen Loading

Choose a reason for hiding this comment

MBoegers Feb 20, 2025

Choose a reason for hiding this comment

amishra-u Feb 18, 2025

Choose a reason for hiding this comment

amishra-u Feb 18, 2025

Choose a reason for hiding this comment

YarochkinMichael Feb 19, 2025

Choose a reason for hiding this comment

YarochkinMichael Feb 19, 2025

Choose a reason for hiding this comment

MBoegers Feb 20, 2025

Choose a reason for hiding this comment

YarochkinMichael commented Feb 19, 2025

YarochkinMichael commented Feb 21, 2025

github-actions bot Feb 23, 2025

Choose a reason for hiding this comment

github-actions bot Feb 23, 2025

Choose a reason for hiding this comment

github-actions bot Feb 23, 2025

Choose a reason for hiding this comment

YarochkinMichael commented Feb 23, 2025

YarochkinMichael commented Feb 23, 2025

amishra-u commented Feb 24, 2025

amishra-u commented Feb 24, 2025 • edited Loading

YarochkinMichael commented Feb 25, 2025

YarochkinMichael commented Feb 18, 2025 •

edited

Loading

YarochkinMichael Feb 18, 2025 •

edited by jevanlingen

Loading

amishra-u commented Feb 24, 2025 •

edited

Loading