Extend null check Refaster rules (#523)

Summary of changes: - Replace `CheckNotNull` with `RequireNonNull{,WithMessage}{,Statement}`. - Extend `Is{,Not}Null`. Fixes #437.
PicnicSupermarket · Mar 27, 2023 · 334c374 · 334c374
1 parent 57cd084
commit 334c374
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 24 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
@@ -21,11 +21,14 @@
 final class NullRules {
   private NullRules() {}
 
-  /** Prefer the {@code ==} operator over {@link Objects#isNull(Object)}. */
+  /**
+   * Prefer the {@code ==} operator (with {@code null} as the second operand) over {@link
+   * Objects#isNull(Object)}.
+   */
   static final class IsNull {
     @BeforeTemplate
     boolean before(@Nullable Object object) {
-      return Objects.isNull(object);
+      return Refaster.anyOf(null == object, Objects.isNull(object));
     }
 
     @AfterTemplate
@@ -34,11 +37,14 @@ boolean after(@Nullable Object object) {
     }
   }
 
-  /** Prefer the {@code !=} operator over {@link Objects#nonNull(Object)}. */
+  /**
+   * Prefer the {@code !=} operator (with {@code null} as the second operand) over {@link
+   * Objects#nonNull(Object)}.
+   */
   static final class IsNotNull {
     @BeforeTemplate
     boolean before(@Nullable Object object) {
-      return Objects.nonNull(object);
+      return Refaster.anyOf(null != object, Objects.nonNull(object));
     }
 
     @AfterTemplate

diff --git a/...-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java b/...-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java
@@ -6,11 +6,13 @@
 import static com.google.common.base.Preconditions.checkPositionIndex;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
+import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.Preconditions;
 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 tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
 /** Refaster templates related to statements dealing with {@link Preconditions}. */
@@ -72,8 +74,22 @@ void after(int index, int size, String message) {
     }
   }
 
-  /** Prefer {@link Preconditions#checkNotNull(Object)} over more verbose alternatives. */
-  static final class CheckNotNull<T> {
+  /** Prefer {@link Objects#requireNonNull(Object)} over non-JDK alternatives. */
+  static final class RequireNonNull<T> {
+    @BeforeTemplate
+    T before(T object) {
+      return checkNotNull(object);
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    T after(T object) {
+      return requireNonNull(object);
+    }
+  }
+
+  /** Prefer {@link Objects#requireNonNull(Object)} over more verbose alternatives. */
+  static final class RequireNonNullStatement<T> {
     @BeforeTemplate
     void before(T object) {
       if (object == null) {
@@ -84,12 +100,26 @@ void before(T object) {
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
     void after(T object) {
-      checkNotNull(object);
+      requireNonNull(object);
+    }
+  }
+
+  /** Prefer {@link Objects#requireNonNull(Object, String)} over non-JDK alternatives. */
+  static final class RequireNonNullWithMessage<T> {
+    @BeforeTemplate
+    T before(T object, String message) {
+      return checkNotNull(object, message);
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    T after(T object, String message) {
+      return requireNonNull(object, message);
     }
   }
 
-  /** Prefer {@link Preconditions#checkNotNull(Object, Object)} over more verbose alternatives. */
-  static final class CheckNotNullWithMessage<T> {
+  /** Prefer {@link Objects#requireNonNull(Object, String)} over more verbose alternatives. */
+  static final class RequireNonNullWithMessageStatement<T> {
     @BeforeTemplate
     void before(T object, String message) {
       if (object == null) {
@@ -100,7 +130,7 @@ void before(T object, String message) {
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
     void after(T object, String message) {
-      checkNotNull(object, message);
+      requireNonNull(object, message);
     }
   }
 

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
@@ -13,12 +13,12 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
     return ImmutableSet.of(MoreObjects.class, Optional.class);
   }
 
-  boolean testIsNull() {
-    return Objects.isNull("foo");
+  ImmutableSet<Boolean> testIsNull() {
+    return ImmutableSet.of(null == "foo", Objects.isNull("bar"));
   }
 
-  boolean testIsNotNull() {
-    return Objects.nonNull("foo");
+  ImmutableSet<Boolean> testIsNotNull() {
+    return ImmutableSet.of(null != "foo", Objects.nonNull("bar"));
   }
 
   ImmutableSet<String> testRequireNonNullElse() {

diff --git a/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java b/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java
@@ -16,12 +16,12 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
     return ImmutableSet.of(MoreObjects.class, Optional.class);
   }
 
-  boolean testIsNull() {
-    return "foo" == null;
+  ImmutableSet<Boolean> testIsNull() {
+    return ImmutableSet.of("foo" == null, "bar" == null);
   }
 
-  boolean testIsNotNull() {
-    return "foo" != null;
+  ImmutableSet<Boolean> testIsNotNull() {
+    return ImmutableSet.of("foo" != null, "bar" != null);
   }
 
   ImmutableSet<String> testRequireNonNullElse() {

diff --git a/.../src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java b/.../src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java
@@ -1,8 +1,17 @@
 package tech.picnic.errorprone.refasterrules;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.collect.ImmutableSet;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class PreconditionsRulesTest implements RefasterRuleCollectionTestCase {
+  @Override
+  @SuppressWarnings("RequireNonNull")
+  public ImmutableSet<?> elidedTypesAndStaticImports() {
+    return ImmutableSet.of(checkNotNull(null));
+  }
+
   void testCheckArgument() {
     if ("foo".isEmpty()) {
       throw new IllegalArgumentException();
@@ -21,13 +30,21 @@ void testCheckElementIndexWithMessage() {
     }
   }
 
-  void testCheckNotNull() {
+  String testRequireNonNull() {
+    return checkNotNull("foo");
+  }
+
+  void testRequireNonNullStatement() {
     if ("foo" == null) {
       throw new NullPointerException();
     }
   }
 
-  void testCheckNotNullWithMessage() {
+  String testRequireNonNullWithMessage() {
+    return checkNotNull("foo", "The string is null");
+  }
+
+  void testRequireNonNullWithMessageStatement() {
     if ("foo" == null) {
       throw new NullPointerException("The string is null");
     }

diff --git a/...src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java b/...src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java
@@ -5,10 +5,18 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkPositionIndex;
 import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
 
+import com.google.common.collect.ImmutableSet;
 import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
 
 final class PreconditionsRulesTest implements RefasterRuleCollectionTestCase {
+  @Override
+  @SuppressWarnings("RequireNonNull")
+  public ImmutableSet<?> elidedTypesAndStaticImports() {
+    return ImmutableSet.of(checkNotNull(null));
+  }
+
   void testCheckArgument() {
     checkArgument(!"foo".isEmpty());
   }
@@ -21,12 +29,20 @@ void testCheckElementIndexWithMessage() {
     checkElementIndex(1, 2, "My index");
   }
 
-  void testCheckNotNull() {
-    checkNotNull("foo");
+  String testRequireNonNull() {
+    return requireNonNull("foo");
+  }
+
+  void testRequireNonNullStatement() {
+    requireNonNull("foo");
+  }
+
+  String testRequireNonNullWithMessage() {
+    return requireNonNull("foo", "The string is null");
   }
 
-  void testCheckNotNullWithMessage() {
-    checkNotNull("foo", "The string is null");
+  void testRequireNonNullWithMessageStatement() {
+    requireNonNull("foo", "The string is null");
   }
 
   void testCheckPositionIndex() {