Introduce additional File and Path Refaster rules (#1282)

PicnicSupermarket · Aug 22, 2024 · 4cbfdba · 4cbfdba
1 parent d94f65a
commit 4cbfdba
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 3 deletions.
diff --git a/...rc/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java b/...rc/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java
@@ -14,7 +14,6 @@
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ServiceLoader;
 import javax.tools.JavaFileObject;
 
@@ -87,6 +86,6 @@ private <T> void writeToFile(String identifier, String className, T data) {
   }
 
   private static String getSimpleClassName(URI path) {
-    return Paths.get(path).getFileName().toString().replace(".java", "");
+    return Path.of(path).getFileName().toString().replace(".java", "");
   }
 }
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
@@ -51,6 +51,7 @@
 // is effectively the identity operation.
 // XXX: Also flag nullary instance method invocations that represent an identity conversion, such as
 // `Boolean#booleanValue()`, `Byte#byteValue()` and friends.
+// XXX: Also flag redundant round-trip conversions such as `path.toFile().toPath()`.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Avoid or clarify identity conversions",

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/FileRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/FileRules.java
@@ -5,11 +5,14 @@
 import com.google.errorprone.refaster.Refaster;
 import com.google.errorprone.refaster.annotation.AfterTemplate;
 import com.google.errorprone.refaster.annotation.BeforeTemplate;
+import com.google.errorprone.refaster.annotation.Repeated;
 import java.io.File;
 import java.io.IOException;
+import java.net.URI;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.attribute.FileAttribute;
 import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
@@ -18,6 +21,49 @@
 final class FileRules {
   private FileRules() {}
 
+  /** Prefer the more idiomatic {@link Path#of(URI)} over {@link Paths#get(URI)}. */
+  static final class PathOfUri {
+    @BeforeTemplate
+    Path before(URI uri) {
+      return Paths.get(uri);
+    }
+
+    @AfterTemplate
+    Path after(URI uri) {
+      return Path.of(uri);
+    }
+  }
+
+  /**
+   * Prefer the more idiomatic {@link Path#of(String, String...)} over {@link Paths#get(String,
+   * String...)}.
+   */
+  static final class PathOfString {
+    @BeforeTemplate
+    Path before(String first, @Repeated String more) {
+      return Paths.get(first, more);
+    }
+
+    @AfterTemplate
+    Path after(String first, @Repeated String more) {
+      return Path.of(first, more);
+    }
+  }
+
+  /** Avoid redundant conversions from {@link Path} to {@link File}. */
+  // XXX: Review whether a rule such as this one is better handled by the `IdentityConversion` rule.
+  static final class PathInstance {
+    @BeforeTemplate
+    Path before(Path path) {
+      return path.toFile().toPath();
+    }
+
+    @AfterTemplate
+    Path after(Path path) {
+      return path;
+    }
+  }
+
   /** Prefer {@link Files#readString(Path, Charset)} over more contrived alternatives. */
   static final class FilesReadStringWithCharset {
     @BeforeTemplate
@@ -50,7 +96,11 @@ String after(Path path) throws IOException {
    */
   static final class FilesCreateTempFileToFile {
     @BeforeTemplate
-    @SuppressWarnings("java:S5443" /* This violation will be rewritten. */)
+    @SuppressWarnings({
+      "FilesCreateTempFileInCustomDirectoryToFile" /* This is a more specific template. */,
+      "java:S5443" /* This violation will be rewritten. */,
+      "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
+    })
     File before(String prefix, String suffix) throws IOException {
       return Refaster.anyOf(
           File.createTempFile(prefix, suffix), File.createTempFile(prefix, suffix, null));
@@ -63,4 +113,20 @@ File after(String prefix, String suffix) throws IOException {
       return Files.createTempFile(prefix, suffix).toFile();
     }
   }
+
+  /**
+   * Prefer {@link Files#createTempFile(Path, String, String, FileAttribute[])} over alternatives
+   * that create files with more liberal permissions.
+   */
+  static final class FilesCreateTempFileInCustomDirectoryToFile {
+    @BeforeTemplate
+    File before(File directory, String prefix, String suffix) throws IOException {
+      return File.createTempFile(prefix, suffix, directory);
+    }
+
+    @AfterTemplate
+    File after(File directory, String prefix, String suffix) throws IOException {
+      return Files.createTempFile(directory.toPath(), prefix, suffix).toFile();
+    }
+  }
 }
diff --git a/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestInput.java b/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestInput.java
@@ -3,12 +3,26 @@
 import com.google.common.collect.ImmutableSet;
 import java.io.File;
 import java.io.IOException;
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class FileRulesTest implements RefasterRuleCollectionTestCase {
+  Path testPathOfUri() {
+    return Paths.get(URI.create("foo"));
+  }
+
+  ImmutableSet<Path> testPathOfString() {
+    return ImmutableSet.of(Paths.get("foo"), Paths.get("bar", "baz", "qux"));
+  }
+
+  Path testPathInstance() {
+    return Path.of("foo").toFile().toPath();
+  }
+
   String testFilesReadStringWithCharset() throws IOException {
     return new String(Files.readAllBytes(Paths.get("foo")), StandardCharsets.ISO_8859_1);
   }
@@ -21,4 +35,8 @@ ImmutableSet<File> testFilesCreateTempFileToFile() throws IOException {
     return ImmutableSet.of(
         File.createTempFile("foo", "bar"), File.createTempFile("baz", "qux", null));
   }
+
+  File testFilesCreateTempFileInCustomDirectoryToFile() throws IOException {
+    return File.createTempFile("foo", "bar", new File("baz"));
+  }
 }
diff --git a/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestOutput.java b/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/FileRulesTestOutput.java
@@ -3,12 +3,26 @@
 import com.google.common.collect.ImmutableSet;
 import java.io.File;
 import java.io.IOException;
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class FileRulesTest implements RefasterRuleCollectionTestCase {
+  Path testPathOfUri() {
+    return Path.of(URI.create("foo"));
+  }
+
+  ImmutableSet<Path> testPathOfString() {
+    return ImmutableSet.of(Path.of("foo"), Path.of("bar", "baz", "qux"));
+  }
+
+  Path testPathInstance() {
+    return Path.of("foo");
+  }
+
   String testFilesReadStringWithCharset() throws IOException {
     return Files.readString(Paths.get("foo"), StandardCharsets.ISO_8859_1);
   }
@@ -21,4 +35,8 @@ ImmutableSet<File> testFilesCreateTempFileToFile() throws IOException {
     return ImmutableSet.of(
         Files.createTempFile("foo", "bar").toFile(), Files.createTempFile("baz", "qux").toFile());
   }
+
+  File testFilesCreateTempFileInCustomDirectoryToFile() throws IOException {
+    return Files.createTempFile(new File("baz").toPath(), "foo", "bar").toFile();
+  }
 }