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

Apache Commons File Utils Migrations #274

Merged
merged 6 commits into from
Aug 22, 2023

Conversation

AlekSimpson
Copy link
Contributor

What's changed?

This PR adds two refaster templates to migrate FileUtils methods.

What's your motivation?

Moving onto FileUtils after StringUtils

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

So currently it won't build because the Write template is a little off. I think the reason its failing with this one is that FileUtils.write() returns void while Java's write method Files.write() returns a Path. I don't think there are any other one line method Java provides to write to a file. Will we just not be able to do migrations for FileUtils.write() because of this?

Anyone you would like to review specifically?

@timtebeek

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ auto-formatter on affected files
  • I've updated the documentation (if applicable)

@AlekSimpson AlekSimpson requested a review from timtebeek August 18, 2023 20:20
@AlekSimpson AlekSimpson marked this pull request as draft August 18, 2023 20:20
@AlekSimpson AlekSimpson self-assigned this Aug 18, 2023
@AlekSimpson AlekSimpson added the recipe Recipe requested label Aug 18, 2023

@AfterTemplate
void after(File file, CharSequence data, Charset cs) throws Exception {
Files.write(file.toPath(), Arrays.asList(data), cs);
Copy link
Contributor

Choose a reason for hiding this comment

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

@knutwannheden (no rush! just interesting)
This fails compile in the generated recipe due to

java: reference to compile is ambiguous
both method compile(org.openrewrite.java.JavaVisitor<?>,java.lang.String,org.openrewrite.java.JavaTemplate.P3<?,?,?>) in org.openrewrite.java.JavaTemplate
 and method compile(org.openrewrite.java.JavaVisitor<?>,java.lang.String,org.openrewrite.java.JavaTemplate.F3<?,?,?,?>) in org.openrewrite.java.JavaTemplate match

Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea what might be going on here? The generated code is:

final JavaTemplate before = JavaTemplate.compile(this, "before", (java.io.File file, CharSequence data, java.nio.charset.Charset cs) -> FileUtils.write(file, data, cs)).build();
final JavaTemplate after = JavaTemplate.compile(this, "after", (java.io.File file, CharSequence data, java.nio.charset.Charset cs) -> Files.write(file.toPath(), Arrays.asList(data), cs)).build();

Copy link
Contributor

Choose a reason for hiding this comment

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

We can work around this with a simple cast of course. ;/

Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't looked at it yet. Possibly we need to generate an explicit cast, because the return type of the called method is not void like that of the template method.

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Let's only expand on the ubertest before we merge this first structure, and from there discuss what methods to pick up.

@AlekSimpson AlekSimpson changed the title Apach Commons File Utils Migrations Apache Commons File Utils Migrations Aug 21, 2023
@timtebeek timtebeek marked this pull request as ready for review August 22, 2023 10:11
@timtebeek timtebeek merged commit 29a24e1 into main Aug 22, 2023
@timtebeek timtebeek deleted the apache_commons_file_utils_refaster_templates branch August 22, 2023 10:18
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.

3 participants