Introduce FilesCreateTempFileToFile Refaster rule
#1162
Conversation
Stephan202
force-pushed
the
sschroevers/create-temp-file-rules
branch
from
879ba3a to
25204a9
Compare
Stephan202
changed the title
FilesCreateTempFileToFile Refaster rule
|
Looks good. No mutations were possible for these changes. |
2 similar comments
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
|
|
||
| @AfterTemplate | ||
| File after(String prefix, String suffix) throws IOException { | ||
| return Files.createTempFile(prefix, suffix).toFile(); |
There was a problem hiding this comment.
SonarCloud now compains about this line, while the documentation literally says:
File f = Files.createTempFile("prefix", "suffix").toFile(); // CompliantI suppose this is something to be reported upstream (either it's incorrect, or unclear).
There was a problem hiding this comment.
Alright, I had a look, and for POSIX file systems at least this is safe: (a), (b). I tested this on my Linux laptop: the suggested code produces a file with -rw------- permissions, while the other two variants create a file with -rw-rw-r-- permissions.
The question is then what happens on Windows, but regardless this change is an improvement. I've added a comment to this effect, and suggest that this PR is good to go.
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/FileRules.java
Outdated
Show resolved
Hide resolved
Stephan202
force-pushed
the
sschroevers/create-temp-file-rules
branch
from
03ec072 to
8f08ea5
Compare
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
rickie
force-pushed
the
sschroevers/create-temp-file-rules
branch
from
ac67277 to
4c34d51
Compare
|
|
Looks good. No mutations were possible for these changes. |
Fun to see that link here; Leitschuh has been using Moderne to roll that out broadly indeed. I did notice that we support both a 2 and 3 arg variant, whereas this only support the two arg option without additional directory argument. Could be worth adding that 3 arg variant here too, such that we can drop our recipe and just use this one. ;) |
|
Good one! I think I can open a PR for that shortly. |
|
Filed #1282 :) |
Suggested commit message:
Inspired by apache/maven-install-plugin#47, found through #1159.