Introduce/extend RequireNonNullElse{,Get} Refaster rules (#425)

Fixes #364.
PicnicSupermarket · Jan 2, 2023 · d456821 · d456821
1 parent 4237732
commit d456821
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 10 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java
@@ -2,13 +2,17 @@
 
 import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
 import static java.util.Objects.requireNonNullElse;
+import static java.util.Objects.requireNonNullElseGet;
 
 import com.google.common.base.MoreObjects;
+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.UseImportPolicy;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import org.jspecify.annotations.Nullable;
 import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
@@ -43,13 +47,18 @@ boolean after(@Nullable Object object) {
     }
   }
 
-  /** Prefer {@link Objects#requireNonNullElse(Object, Object)} over the Guava alternative. */
-  // XXX: This rule is not valid in case `second` is `@Nullable`: in that case the Guava variant
-  // will return `null`, while the JDK variant will throw an NPE.
+  /**
+   * Prefer {@link Objects#requireNonNullElse(Object, Object)} over non-JDK or more contrived
+   * alternatives.
+   */
+  // XXX: This rule is not valid in case `second` is `@Nullable`: in that case the Guava and
+  // `Optional` variants will return `null`, where the `requireNonNullElse` alternative will throw
+  // an NPE.
   static final class RequireNonNullElse<T> {
     @BeforeTemplate
     T before(T first, T second) {
-      return MoreObjects.firstNonNull(first, second);
+      return Refaster.anyOf(
+          MoreObjects.firstNonNull(first, second), Optional.ofNullable(first).orElse(second));
     }
 
     @AfterTemplate
@@ -59,6 +68,26 @@ T after(T first, T second) {
     }
   }
 
+  /**
+   * Prefer {@link Objects#requireNonNullElseGet(Object, Supplier)} over more contrived
+   * alternatives.
+   */
+  // XXX: This rule is not valid in case `supplier` yields `@Nullable` values: in that case the
+  // `Optional` variant will return `null`, where the `requireNonNullElseGet` alternative will throw
+  // an NPE.
+  static final class RequireNonNullElseGet<T, S extends T> {
+    @BeforeTemplate
+    T before(T object, Supplier<S> supplier) {
+      return Optional.ofNullable(object).orElseGet(supplier);
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    T after(T object, Supplier<S> supplier) {
+      return requireNonNullElseGet(object, supplier);
+    }
+  }
+
   /** Prefer {@link Objects#isNull(Object)} over the equivalent lambda function. */
   static final class IsNullFunction<T> {
     @BeforeTemplate

diff --git a/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java b/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java
@@ -3,13 +3,14 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.stream.Stream;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class NullRulesTest implements RefasterRuleCollectionTestCase {
   @Override
   public ImmutableSet<?> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(MoreObjects.class);
+    return ImmutableSet.of(MoreObjects.class, Optional.class);
   }
 
   boolean testIsNull() {
@@ -20,8 +21,13 @@ boolean testIsNotNull() {
     return Objects.nonNull("foo");
   }
 
-  String testRequireNonNullElse() {
-    return MoreObjects.firstNonNull("foo", "bar");
+  ImmutableSet<String> testRequireNonNullElse() {
+    return ImmutableSet.of(
+        MoreObjects.firstNonNull("foo", "bar"), Optional.ofNullable("baz").orElse("qux"));
+  }
+
+  String testRequireNonNullElseGet() {
+    return Optional.ofNullable("foo").orElseGet(() -> "bar");
   }
 
   long testIsNullFunction() {

diff --git a/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java b/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java
@@ -1,17 +1,19 @@
 package tech.picnic.errorprone.refasterrules;
 
 import static java.util.Objects.requireNonNullElse;
+import static java.util.Objects.requireNonNullElseGet;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.stream.Stream;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class NullRulesTest implements RefasterRuleCollectionTestCase {
   @Override
   public ImmutableSet<?> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(MoreObjects.class);
+    return ImmutableSet.of(MoreObjects.class, Optional.class);
   }
 
   boolean testIsNull() {
@@ -22,8 +24,12 @@ boolean testIsNotNull() {
     return "foo" != null;
   }
 
-  String testRequireNonNullElse() {
-    return requireNonNullElse("foo", "bar");
+  ImmutableSet<String> testRequireNonNullElse() {
+    return ImmutableSet.of(requireNonNullElse("foo", "bar"), requireNonNullElse("baz", "qux"));
+  }
+
+  String testRequireNonNullElseGet() {
+    return requireNonNullElseGet("foo", () -> "bar");
   }
 
   long testIsNullFunction() {