Introduce ArraysCompareUnsigned{Byte,Int,Long}s Refaster rules (#1542)

While there, document possible `StreamFlatMapOptional` improvement.
PicnicSupermarket · Feb 18, 2025 · 3beadee · 3beadee
1 parent c2a26ed
commit 3beadee
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 8 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java
@@ -282,6 +282,8 @@ T after(Optional<T> optional, T value) {
   // XXX: The rewritten `filter`/`map` expression may be more performant than its replacement. See
   // https://github.com/palantir/gradle-baseline/pull/2946. (There are plans to pair Refaster rules
   // with JMH benchmarks; this would be a great use case.)
+  // XXX: Perhaps `stream.mapMulti(Optional::ifPresent)` is what we should use. See
+  // https://github.com/palantir/gradle-baseline/pull/2996.
   static final class StreamFlatMapOptional<T> {
     @BeforeTemplate
     Stream<T> before(Stream<Optional<T>> stream) {

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PrimitiveRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PrimitiveRules.java
@@ -8,12 +8,15 @@
 import com.google.common.primitives.Ints;
 import com.google.common.primitives.Longs;
 import com.google.common.primitives.Shorts;
+import com.google.common.primitives.UnsignedBytes;
 import com.google.common.primitives.UnsignedInts;
 import com.google.common.primitives.UnsignedLongs;
 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 java.util.Arrays;
+import java.util.Comparator;
 import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
 /** Refaster rules related to expressions dealing with primitives. */
@@ -525,10 +528,7 @@ String after(long i) {
     }
   }
 
-  /**
-   * Prefer JDK's {@link Integer#toUnsignedString(int,int)} over third-party or more verbose
-   * alternatives.
-   */
+  /** Prefer JDK's {@link Integer#toUnsignedString(int,int)} over third-party alternatives. */
   static final class IntegerToUnsignedStringWithRadix {
     @BeforeTemplate
     String before(int i, int radix) {
@@ -541,10 +541,7 @@ String after(int i, int radix) {
     }
   }
 
-  /**
-   * Prefer JDK's {@link Long#toUnsignedString(long,int)} over third-party or more verbose
-   * alternatives.
-   */
+  /** Prefer JDK's {@link Long#toUnsignedString(long,int)} over third-party alternatives. */
   static final class LongToUnsignedStringWithRadix {
     @BeforeTemplate
     String before(long i, int radix) {
@@ -556,4 +553,49 @@ String after(long i, int radix) {
       return Long.toUnsignedString(i, radix);
     }
   }
+
+  /** Prefer JDK's {@link Arrays#compareUnsigned(byte[], byte[])} over third-party alternatives. */
+  // XXX: This rule will yield non-compilable code if the result of the replaced expression is
+  // dereferenced. Investigate how to make this safe.
+  static final class ArraysCompareUnsignedBytes {
+    @BeforeTemplate
+    Comparator<byte[]> before() {
+      return UnsignedBytes.lexicographicalComparator();
+    }
+
+    @AfterTemplate
+    Comparator<byte[]> after() {
+      return Arrays::compareUnsigned;
+    }
+  }
+
+  /** Prefer JDK's {@link Arrays#compareUnsigned(int[], int[])} over third-party alternatives. */
+  // XXX: This rule will yield non-compilable code if the result of the replaced expression is
+  // dereferenced. Investigate how to make this safe.
+  static final class ArraysCompareUnsignedInts {
+    @BeforeTemplate
+    Comparator<int[]> before() {
+      return UnsignedInts.lexicographicalComparator();
+    }
+
+    @AfterTemplate
+    Comparator<int[]> after() {
+      return Arrays::compareUnsigned;
+    }
+  }
+
+  /** Prefer JDK's {@link Arrays#compareUnsigned(long[], long[])} over third-party alternatives. */
+  // XXX: This rule will yield non-compilable code if the result of the replaced expression is
+  // dereferenced. Investigate how to make this safe.
+  static final class ArraysCompareUnsignedLongs {
+    @BeforeTemplate
+    Comparator<long[]> before() {
+      return UnsignedLongs.lexicographicalComparator();
+    }
+
+    @AfterTemplate
+    Comparator<long[]> after() {
+      return Arrays::compareUnsigned;
+    }
+  }
 }
diff --git a/...trib/src/test/resources/tech/picnic/errorprone/refasterrules/PrimitiveRulesTestInput.java b/...trib/src/test/resources/tech/picnic/errorprone/refasterrules/PrimitiveRulesTestInput.java
@@ -9,8 +9,10 @@
 import com.google.common.primitives.Ints;
 import com.google.common.primitives.Longs;
 import com.google.common.primitives.Shorts;
+import com.google.common.primitives.UnsignedBytes;
 import com.google.common.primitives.UnsignedInts;
 import com.google.common.primitives.UnsignedLongs;
+import java.util.Comparator;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class PrimitiveRulesTest implements RefasterRuleCollectionTestCase {
@@ -25,6 +27,7 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
         Ints.class,
         Longs.class,
         Shorts.class,
+        UnsignedBytes.class,
         UnsignedInts.class,
         UnsignedLongs.class);
   }
@@ -222,4 +225,16 @@ String testIntegerToUnsignedStringWithRadix() {
   String testLongToUnsignedStringWithRadix() {
     return UnsignedLongs.toString(1, 2);
   }
+
+  Comparator<byte[]> testArraysCompareUnsignedBytes() {
+    return UnsignedBytes.lexicographicalComparator();
+  }
+
+  Comparator<int[]> testArraysCompareUnsignedInts() {
+    return UnsignedInts.lexicographicalComparator();
+  }
+
+  Comparator<long[]> testArraysCompareUnsignedLongs() {
+    return UnsignedLongs.lexicographicalComparator();
+  }
 }
diff --git a/...rib/src/test/resources/tech/picnic/errorprone/refasterrules/PrimitiveRulesTestOutput.java b/...rib/src/test/resources/tech/picnic/errorprone/refasterrules/PrimitiveRulesTestOutput.java
@@ -9,8 +9,11 @@
 import com.google.common.primitives.Ints;
 import com.google.common.primitives.Longs;
 import com.google.common.primitives.Shorts;
+import com.google.common.primitives.UnsignedBytes;
 import com.google.common.primitives.UnsignedInts;
 import com.google.common.primitives.UnsignedLongs;
+import java.util.Arrays;
+import java.util.Comparator;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class PrimitiveRulesTest implements RefasterRuleCollectionTestCase {
@@ -25,6 +28,7 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
         Ints.class,
         Longs.class,
         Shorts.class,
+        UnsignedBytes.class,
         UnsignedInts.class,
         UnsignedLongs.class);
   }
@@ -222,4 +226,16 @@ String testIntegerToUnsignedStringWithRadix() {
   String testLongToUnsignedStringWithRadix() {
     return Long.toUnsignedString(1, 2);
   }
+
+  Comparator<byte[]> testArraysCompareUnsignedBytes() {
+    return Arrays::compareUnsigned;
+  }
+
+  Comparator<int[]> testArraysCompareUnsignedInts() {
+    return Arrays::compareUnsigned;
+  }
+
+  Comparator<long[]> testArraysCompareUnsignedLongs() {
+    return Arrays::compareUnsigned;
+  }
 }