Introduce two FileRules Refaster rules (#1596)

Invoking `File#mkdirs` before testing for existence of the specified
path, rather than the other way around, avoids a subtle concurrency
issue.

See also openrewrite/rewrite#5189.
This commit is contained in:
Tim te Beek
2025-03-23 11:13:24 +01:00
committed by GitHub
parent 40b3c87b72
commit c3351b9ee1
3 changed files with 57 additions and 0 deletions

View File

@@ -4,6 +4,7 @@ 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.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Repeated;
import java.io.File;
@@ -11,6 +12,7 @@ import java.io.IOException;
import java.net.URI;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;
@@ -141,4 +143,35 @@ final class FileRules {
return Files.createTempFile(directory.toPath(), prefix, suffix).toFile();
}
}
/**
* Invoke {@link File#mkdirs()} before {@link Files#exists(Path, LinkOption...)} to avoid
* concurrency issues.
*/
static final class PathToFileMkDirsFilesExists {
@BeforeTemplate
boolean before(Path path) {
return Files.exists(path) || path.toFile().mkdirs();
}
@AfterTemplate
@AlsoNegation
boolean after(Path path) {
return path.toFile().mkdirs() || Files.exists(path);
}
}
/** Invoke {@link File#mkdirs()} before {@link File#exists()} to avoid concurrency issues. */
static final class FileMkDirsFileExists {
@BeforeTemplate
boolean before(File file) {
return file.exists() || file.mkdirs();
}
@AfterTemplate
@AlsoNegation
boolean after(File file) {
return file.mkdirs() || file.exists();
}
}
}

View File

@@ -39,4 +39,16 @@ final class FileRulesTest implements RefasterRuleCollectionTestCase {
File testFilesCreateTempFileInCustomDirectoryToFile() throws IOException {
return File.createTempFile("foo", "bar", new File("baz"));
}
ImmutableSet<Boolean> testPathToFileMkDirsFilesExists() {
return ImmutableSet.of(
Files.exists(Path.of("foo")) || Path.of("foo").toFile().mkdirs(),
!Files.exists(Path.of("bar")) && !Path.of("bar").toFile().mkdirs());
}
ImmutableSet<Boolean> testFileMkDirsFileExists() {
return ImmutableSet.of(
new File("foo").exists() || new File("foo").mkdirs(),
!new File("bar").exists() && !new File("bar").mkdirs());
}
}

View File

@@ -39,4 +39,16 @@ final class FileRulesTest implements RefasterRuleCollectionTestCase {
File testFilesCreateTempFileInCustomDirectoryToFile() throws IOException {
return Files.createTempFile(new File("baz").toPath(), "foo", "bar").toFile();
}
ImmutableSet<Boolean> testPathToFileMkDirsFilesExists() {
return ImmutableSet.of(
Path.of("foo").toFile().mkdirs() || Files.exists(Path.of("foo")),
!Path.of("bar").toFile().mkdirs() && !Files.exists(Path.of("bar")));
}
ImmutableSet<Boolean> testFileMkDirsFileExists() {
return ImmutableSet.of(
new File("foo").mkdirs() || new File("foo").exists(),
!new File("bar").mkdirs() && !new File("bar").exists());
}
}