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

Add explicit Charset to Apache FileUtils methods #36

Open
yeikel opened this issue Apr 28, 2022 · 4 comments
Open

Add explicit Charset to Apache FileUtils methods #36

yeikel opened this issue Apr 28, 2022 · 4 comments
Labels
good first issue Good for newcomers recipe

Comments

@yeikel
Copy link

yeikel commented Apr 28, 2022

Examples :

    /**
     * Writes a String to a file creating the file if it does not exist using the default encoding for the VM.
     *
     * @deprecated 2.5 use {@link #writeStringToFile(File, String, Charset, boolean)} instead (and specify the appropriate encoding)
     */
    @Deprecated
    public static void writeStringToFile(final File file, final String data, final boolean append) throws IOException {
        writeStringToFile(file, data, Charset.defaultCharset(), append);
    }

 @Deprecated
    public static void writeStringToFile(final File file, final String data) throws IOException {
        writeStringToFile(file, data, Charset.defaultCharset(), false);
    }

 @Deprecated
    public static String readFileToString(final File file) throws IOException {
        return readFileToString(file, Charset.defaultCharset());
    }

    @Deprecated
    public static List<String> readLines(final File file) 

And many others : https://github.com/apache/commons-io/blob/master/src/main/java/org/apache/commons/io/FileUtils.java

Charset should be configurable. Ideally, it should accept :

  • Use StandardCharsets whenever possible
  • Charsets by name such as Windows-1252

Similar issue : openrewrite/rewrite-migrate-java#99

See also : openrewrite/rewrite#1724

@yeikel
Copy link
Author

yeikel commented Apr 28, 2022

@tkvangorder Should this be separate issues per method?

I imagine it'll be different recipes per methods but I did not want to create a bunch of similar issues

Maybe, we can just create tasks instead?

Like

  • readFileToString
  • writeStringToFile

etc

@yeikel yeikel changed the title add explicit Charset to Apache FileUtils methods Add explicit Charset to Apache FileUtils methods Apr 28, 2022
@tkvangorder
Copy link
Contributor

Yeah, I think we can just define tasks in this issue to organize the work.

We used the "one recipe per method" in the AssertJ migrations: https://github.com/openrewrite/rewrite-testing-frameworks/tree/main/src/main/java/org/openrewrite/java/testing/assertj

I think you could make a reasonable argument to do something similar or to combine similar methods under a single recipe. The "copy/paste/slightly edit recipe/test" can be tedious to maintain when you want to make changes across the suite of recipes.

@tkvangorder tkvangorder added the good first issue Good for newcomers label Apr 28, 2022
@pway99
Copy link
Contributor

pway99 commented Apr 28, 2022

@tkvangorder @yeikel for the Apache commons IOUtils task I handled all charset specific deprecations in a single recipe f758efb62aa4dea938841766a81fd1d14d44c321

@yeikel
Copy link
Author

yeikel commented Apr 28, 2022

@tkvangorder @yeikel for the Apache commons IOUtils task I handled all charset specific deprecations in a single recipe f758efb62aa4dea938841766a81fd1d14d44c321

Thank you for sharing. As @tkvangorder mentioned, doing the same for this class will be better

@timtebeek timtebeek transferred this issue from openrewrite/rewrite-migrate-java Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers recipe
Projects
Status: Recipes Wanted
Development

No branches or pull requests

3 participants