From d11ac09bda027e2d6e81e21eaaba5991d1bf80e3 Mon Sep 17 00:00:00 2001
From: Phil Werli <phil.werli@teampicnic.com>
Date: Tue, 31 Dec 2024 16:30:17 +0100
Subject: [PATCH] Extend `{Is,Non}NullFunction` Refaster rules (#1494)

While there, simplify the associated tests.
---
 .../errorprone/refasterrules/NullRules.java       | 15 +++++++++++----
 .../refasterrules/NullRulesTestInput.java         | 14 ++++++++------
 .../refasterrules/NullRulesTestOutput.java        | 13 +++++++------
 3 files changed, 26 insertions(+), 16 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
index 2d7a6b263b..e9b3a42fda 100644
--- 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
@@ -3,6 +3,7 @@
 import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
 import static java.util.Objects.requireNonNullElse;
 import static java.util.Objects.requireNonNullElseGet;
+import static java.util.function.Predicate.not;
 
 import com.google.common.base.MoreObjects;
 import com.google.errorprone.refaster.Refaster;
@@ -94,11 +95,14 @@ T after(T object, Supplier<S> supplier) {
     }
   }
 
-  /** Prefer {@link Objects#isNull(Object)} over the equivalent lambda function. */
+  /**
+   * Prefer {@link Objects#isNull(Object)} over the equivalent lambda function or more contrived
+   * alternatives.
+   */
   static final class IsNullFunction<T> {
     @BeforeTemplate
     Predicate<T> before() {
-      return o -> o == null;
+      return Refaster.anyOf(o -> o == null, not(Objects::nonNull));
     }
 
     @AfterTemplate
@@ -107,11 +111,14 @@ Predicate<T> after() {
     }
   }
 
-  /** Prefer {@link Objects#nonNull(Object)} over the equivalent lambda function. */
+  /**
+   * Prefer {@link Objects#nonNull(Object)} over the equivalent lambda function or more contrived
+   * alternatives.
+   */
   static final class NonNullFunction<T> {
     @BeforeTemplate
     Predicate<T> before() {
-      return o -> o != null;
+      return Refaster.anyOf(o -> o != null, not(Objects::isNull));
     }
 
     @AfterTemplate
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java
index 5625438429..c3cf2da925 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java
@@ -1,16 +1,18 @@
 package tech.picnic.errorprone.refasterrules;
 
+import static java.util.function.Predicate.not;
+
 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 java.util.function.Predicate;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class NullRulesTest implements RefasterRuleCollectionTestCase {
   @Override
   public ImmutableSet<Object> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(MoreObjects.class, Optional.class);
+    return ImmutableSet.of(MoreObjects.class, Optional.class, not(null));
   }
 
   ImmutableSet<Boolean> testIsNull() {
@@ -30,11 +32,11 @@ String testRequireNonNullElseGet() {
     return Optional.ofNullable("foo").orElseGet(() -> "bar");
   }
 
-  long testIsNullFunction() {
-    return Stream.of("foo").filter(s -> s == null).count();
+  ImmutableSet<Predicate<String>> testIsNullFunction() {
+    return ImmutableSet.of(s -> s == null, not(Objects::nonNull));
   }
 
-  long testNonNullFunction() {
-    return Stream.of("foo").filter(s -> s != null).count();
+  ImmutableSet<Predicate<String>> testNonNullFunction() {
+    return ImmutableSet.of(s -> s != null, not(Objects::isNull));
   }
 }
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java
index 33384f5b42..78f132203e 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java
@@ -2,18 +2,19 @@
 
 import static java.util.Objects.requireNonNullElse;
 import static java.util.Objects.requireNonNullElseGet;
+import static java.util.function.Predicate.not;
 
 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 java.util.function.Predicate;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class NullRulesTest implements RefasterRuleCollectionTestCase {
   @Override
   public ImmutableSet<Object> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(MoreObjects.class, Optional.class);
+    return ImmutableSet.of(MoreObjects.class, Optional.class, not(null));
   }
 
   ImmutableSet<Boolean> testIsNull() {
@@ -32,11 +33,11 @@ String testRequireNonNullElseGet() {
     return requireNonNullElseGet("foo", () -> "bar");
   }
 
-  long testIsNullFunction() {
-    return Stream.of("foo").filter(Objects::isNull).count();
+  ImmutableSet<Predicate<String>> testIsNullFunction() {
+    return ImmutableSet.of(Objects::isNull, Objects::isNull);
   }
 
-  long testNonNullFunction() {
-    return Stream.of("foo").filter(Objects::nonNull).count();
+  ImmutableSet<Predicate<String>> testNonNullFunction() {
+    return ImmutableSet.of(Objects::nonNull, Objects::nonNull);
   }
 }