rewrite-migrate-java icon indicating copy to clipboard operation
rewrite-migrate-java copied to clipboard

Add explicit `Charset` to Apache `FileUtils` methods

Open yeikel opened this issue 2 years ago • 4 comments

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 : https://github.com/openrewrite/rewrite-migrate-java/issues/99

See also : https://github.com/openrewrite/rewrite/issues/1724

yeikel avatar Apr 28 '22 18:04 yeikel

@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 avatar Apr 28 '22 18:04 yeikel

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 avatar Apr 28 '22 19:04 tkvangorder

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

pway99 avatar Apr 28 '22 19:04 pway99

@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

yeikel avatar Apr 28 '22 22:04 yeikel