modernizer-maven-plugin
modernizer-maven-plugin copied to clipboard
Should recommend Path.of() instead of Paths.get()
I appreciate the recent phase out of old java.io.File usage via #301. However, in my eyes the recommended replacement is not good. Paths.get() works fine, but it has a note in its Javadoc which recommends the usage of Path.of() instead, and that's what modernizer should suggest, too: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/nio/file/Paths.java#L36-L38
However, that method is only available from Java 11, therefore there is the tiny window of people updating from 7 to 8 only (instead of from 7 to 11+), which would need Paths.get(). Not sure if that tiny fraction can be ignored, or whether the error message should point to both replacements (one for Java 8, one for Java 11+).
@mkarg , please share your thoughts on this
Maybe just add another violation for Paths.get recommending Path.of? This will annoy users who fix one violation and are greeted by a second but at least gives them a path to the recommended state. I could see more involved solutions like adding an untilVersion element to modernizer.xml which could trigger on a version range instead of all newer ones.
Actually I was under the impression to have implemented the violations in a way that with target 8, 9 and 10 Paths.get() is recommended, and with target 11+ Path.of() is recommended. If that is not the case, then I made a mistake (please check an tell me). Thanks.
what is problem with usage of new File(path)
https://github.com/checkstyle/checkstyle/pull/16095#issuecomment-2565949133 ? what should be used instead ?
what is problem with usage of
new File(path)checkstyle/checkstyle#16095 (comment) ? what should be used instead ?
Since Java 7 there is no need to use java.io.File. In particular, it behaves wrong by design in some cases, and it is less efficient than java.nio.file.Files / java.nio.file.Path.
Replace it by using NIO2 API, which is more complex thant simply replacing a single class.
For target Java 7-10 go with Paths.get, for Java 11+ go with Path.of.
- allFiles.add(new File(fileName));
+ allFiles.add(java.nio.file.Path.of(fileName).toFile());
- allFiles.add(new File(fileName)); + allFiles.add(java.nio.file.Path.of(fileName).toFile());
I am sorry but I need to say I do not understand what you like to tell us. Can you please elaborate what your question, problem, and / or proposal is? Thanks. 😳
I don't think the intent should be to create a Path to call toFile. Rather, codebases should adopt Path throughout, e.g., https://github.com/gaul/s3proxy/commit/26dd4a46e9d017c037bc53692d2c9a64b7d387fa. Perhaps this seems pedantic to some applications but using Path has advantages, e.g., using jimfs for in-memory testing. I was also able to fix some Linux/Windows issues with its emulation capabilities.
I don't think the intent should be to create a Path to call toFile. Rather, codebases should adopt Path throughout, e.g., gaul/s3proxy@26dd4a4. Perhaps this seems pedantic to some applications but using Path has advantages, e.g., using jimfs for in-memory testing. I was also able to fix some Linux/Windows issues with its emulation capabilities.
In modern Java (7+) there is no justified use for File other than staying compatible with non-modernized binaries. If the binary can be modernized, modernize it. If the binary cannot be modernized, don't create a Path just to create a File in turn, but stick with new File in that code location and annotate it to be ignored by the modernizer plugin.
Having said that, I would say, this thread is OT.
I generally like the idea.
We encountered some cases with Desktop.getDesktop().open(new File(filePath)); from java.awt
It accepts only File. There are a couple of other awt methods
And also Process builder directory still requires File()
Could you report an issue against the JDK? This sounds like an oversight on their part. I previously encountered similar situations where some APIs only accepted StringBuffer instead of modern StringBuilder.
I generally like the idea. We encountered some cases with
Desktop.getDesktop().open(new File(filePath));from java.awt It accepts only File. There are a couple of other awt methods And also Process builder directory still requires File()
You are right, the JRE contains (few) APIs that still make use of File, and it is a good idea to use Path.of().toFile() in that situations, as it will provide an optimized, fast, and more correct implementation that way.
Could you report an issue against the JDK? This sounds like an oversight on their part. I previously encountered similar situations where some APIs only accepted
StringBufferinstead of modernStringBuilder.
Just proposed the needed changed. Waiting for approval by OpenJDK team.