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.

Sign up for GitHub

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

Introduce FilesCreateTempFileToFile Refaster rule #1162

Merged
merged 4 commits into from
Aug 7, 2024
Merged

Introduce FilesCreateTempFileToFile Refaster rule #1162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7cfff6
Introduce `FilesCreateTempFileToFile` Refaster rule
Stephan202 Apr 30, 2024
ccc70a6
Nice :)
Stephan202 Apr 30, 2024
bf9eab3
Fix typo
Stephan202 Aug 4, 2024
4c34d51
Suppress warning
Stephan202 Aug 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/FileRules.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileAttribute;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with files. */
Expand Down Expand Up @@ -40,4 +43,24 @@ String after(Path path) throws IOException {
return Files.readString(path);
}
}

/**
* Prefer {@link Files#createTempFile(String, String, FileAttribute[])} over alternatives that
* create files with more liberal permissions.
*/
static final class FilesCreateTempFileToFile {
@BeforeTemplate
@SuppressWarnings("java:S5443" /* This violation will be rewritten. */)
File before(String prefix, String suffix) throws IOException {
return Refaster.anyOf(
File.createTempFile(prefix, suffix), File.createTempFile(prefix, suffix, null));
}

@AfterTemplate
@SuppressWarnings(
"java:S5443" /* On POSIX systems the file will only have user read-write permissions. */)
File after(String prefix, String suffix) throws IOException {
return Files.createTempFile(prefix, suffix).toFile();
Copy link
Member Author

@Stephan202 Stephan202 Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

SonarCloud now compains about this line, while the documentation literally says:

File f = Files.createTempFile("prefix", "suffix").toFile();  // Compliant

I suppose this is something to be reported upstream (either it's incorrect, or unclear).

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

}
}
}
7 changes: 7 additions & 0 deletions ...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
Expand All @@ -14,4 +16,9 @@ String testFilesReadStringWithCharset() throws IOException {
String testFilesReadString() throws IOException {
return Files.readString(Paths.get("foo"), StandardCharsets.UTF_8);
}

ImmutableSet<File> testFilesCreateTempFileToFile() throws IOException {
return ImmutableSet.of(
File.createTempFile("foo", "bar"), File.createTempFile("baz", "qux", null));
}
}
7 changes: 7 additions & 0 deletions ...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
Expand All @@ -14,4 +16,9 @@ String testFilesReadStringWithCharset() throws IOException {
String testFilesReadString() throws IOException {
return Files.readString(Paths.get("foo"));
}

ImmutableSet<File> testFilesCreateTempFileToFile() throws IOException {
return ImmutableSet.of(
Files.createTempFile("foo", "bar").toFile(), Files.createTempFile("baz", "qux").toFile());
}
}