Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plexus FileUtils refaster template recipes #272

Merged
merged 10 commits into from
Aug 15, 2023

Conversation

timtebeek
Copy link
Contributor

What's changed?

Added recipes to convert from Plexus FileUtils to either JDK or Apache Commons IO FileUtils.

What's your motivation?

Standardizing on a single implementation aids later migration to JDK internals, or at least reduces the number of dependencies.

Anything in particular you'd like reviewers to focus on?

Matchers fail to match 🤔

Anyone you would like to review specifically?

@knutwannheden

Any additional context

https://issues.apache.org/jira/browse/MNG-6825
Assumes openrewrite/rewrite-templating#17 gets merged

Copy link
Contributor

@knutwannheden knutwannheden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now have multiple FileUtils classes with the same name, can you add a test and see what happens when the source also references a method which isn't migrated? I suspect we will get conflicting imports.

@timtebeek timtebeek self-assigned this Aug 14, 2023
@timtebeek timtebeek added the recipe Recipe requested label Aug 14, 2023
@timtebeek
Copy link
Contributor Author

@knutwannheden This PR is now at the stage where we insert FQN everywhere, when comparing expected vs actual.
Should we try again with a variant of openrewrite/rewrite-templating#18 merged?

That would mean we have to update the expected values here, but overall I think should improve both the recipe development experience (able to use FQN) and resulting output (non-FQN). What are your thoughts at this stage?

Of course we can iterate again later, but it'd be nice to have a mostly functional set of recipes with some broad assumptions.

Unexpected result in "Test.java"
diff --git a/Test.java b/Test.java
index 1880345..310d3d3 100644
--- a/Test.java
+++ b/Test.java
@@ -1,13 +1,13 @@ 
 import java.io.File;
 import java.io.IOException;
-import org.apache.commons.io.FileUtils;
+import org.codehaus.plexus.util.FileUtils;
 class Test {
     void test() throws IOException {
-        FileUtils.deleteDirectory(new File("test"));
+        org.apache.commons.io.FileUtils.deleteDirectory(new File("test"));
         org.apache.commons.io.FileUtils.deleteDirectory(new File("test"));
 
         File file = new File("test");
-        FileUtils.deleteDirectory(file);
+        org.apache.commons.io.FileUtils.deleteDirectory(file);
         org.apache.commons.io.FileUtils.deleteDirectory(file);
 
         FileUtils.dirname("/foo/bar"); // Unused

@knutwannheden
Copy link
Contributor

Yes, let's do that. We won't be able to have full control anyway.

@timtebeek timtebeek marked this pull request as ready for review August 15, 2023 06:46
@timtebeek timtebeek merged commit 2a8527b into main Aug 15, 2023
@timtebeek timtebeek deleted the plexus_fileutils_refaster_template_recipes branch August 15, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe requested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants