Prefer simple null reference check over calling `Objects#{isNull,no…

…nNull}` (#228)
PicnicSupermarket · Sep 10, 2022 · e34c2ba · e34c2ba
1 parent 184ba8a
commit e34c2ba
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 7 deletions.
diff --git a/...r-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/NullTemplates.java b/...r-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/NullTemplates.java
@@ -9,11 +9,38 @@
 import com.google.errorprone.refaster.annotation.UseImportPolicy;
 import java.util.Objects;
 import java.util.function.Predicate;
+import javax.annotation.Nullable;
 
 /** Refaster templates related to expressions dealing with (possibly) null values. */
 final class NullTemplates {
   private NullTemplates() {}
 
+  /** Prefer the {@code ==} operator over {@link Objects#isNull(Object)}. */
+  static final class IsNull {
+    @BeforeTemplate
+    boolean before(@Nullable Object object) {
+      return Objects.isNull(object);
+    }
+
+    @AfterTemplate
+    boolean after(@Nullable Object object) {
+      return object == null;
+    }
+  }
+
+  /** Prefer the {@code !=} operator over {@link Objects#nonNull(Object)}. */
+  static final class IsNotNull {
+    @BeforeTemplate
+    boolean before(@Nullable Object object) {
+      return Objects.nonNull(object);
+    }
+
+    @AfterTemplate
+    boolean after(@Nullable Object object) {
+      return object != null;
+    }
+  }
+
   /** 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.

diff --git a/...b/src/test/resources/tech/picnic/errorprone/refastertemplates/NullTemplatesTestInput.java b/...b/src/test/resources/tech/picnic/errorprone/refastertemplates/NullTemplatesTestInput.java
@@ -2,6 +2,7 @@
 
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
+import java.util.Objects;
 import java.util.stream.Stream;
 import tech.picnic.errorprone.refaster.test.RefasterTemplateTestCase;
 
@@ -11,6 +12,14 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
     return ImmutableSet.of(MoreObjects.class);
   }
 
+  boolean testIsNull() {
+    return Objects.isNull("foo");
+  }
+
+  boolean testIsNotNull() {
+    return Objects.nonNull("foo");
+  }
+
   String testRequireNonNullElse() {
     return MoreObjects.firstNonNull("foo", "bar");
   }

diff --git a/.../src/test/resources/tech/picnic/errorprone/refastertemplates/NullTemplatesTestOutput.java b/.../src/test/resources/tech/picnic/errorprone/refastertemplates/NullTemplatesTestOutput.java
@@ -14,6 +14,14 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
     return ImmutableSet.of(MoreObjects.class);
   }
 
+  boolean testIsNull() {
+    return "foo" == null;
+  }
+
+  boolean testIsNotNull() {
+    return "foo" != null;
+  }
+
   String testRequireNonNullElse() {
     return requireNonNullElse("foo", "bar");
   }

diff --git a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/ValidTemplates.java b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/ValidTemplates.java
@@ -7,7 +7,6 @@
 import com.google.errorprone.refaster.annotation.BeforeTemplate;
 import com.google.errorprone.refaster.annotation.Placeholder;
 import com.google.errorprone.refaster.annotation.UseImportPolicy;
-import java.util.Objects;
 import java.util.Set;
 import javax.annotation.Nullable;
 
@@ -30,7 +29,7 @@ boolean after(String string) {
   static final class StaticImportStringLength {
     @BeforeTemplate
     boolean before(@Nullable String string) {
-      return Objects.isNull(string) || string.isEmpty();
+      return string == null || string.toCharArray().length == 0;
     }
 
     @AfterTemplate

diff --git a/...port/src/test/resources/tech/picnic/errorprone/refaster/test/ValidTemplatesTestInput.java b/...port/src/test/resources/tech/picnic/errorprone/refaster/test/ValidTemplatesTestInput.java
@@ -3,22 +3,21 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import java.util.HashSet;
-import java.util.Objects;
 import java.util.Set;
 
 /** Code to test the Refaster templates from {@link ValidTemplates}. */
 final class ValidTemplatesTest implements RefasterTemplateTestCase {
   @Override
   public ImmutableSet<?> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(Objects.class, Strings.class);
+    return ImmutableSet.of(Strings.class);
   }
 
   boolean testStringIsEmpty2() {
     return "foo".toCharArray().length == 0;
   }
 
   boolean testStaticImportStringLength() {
-    return Objects.isNull("foo") || "foo".isEmpty();
+    return "foo" == null || "foo".toCharArray().length == 0;
   }
 
   void testBlockTemplateSetAddElement() {

diff --git a/...ort/src/test/resources/tech/picnic/errorprone/refaster/test/ValidTemplatesTestOutput.java b/...ort/src/test/resources/tech/picnic/errorprone/refaster/test/ValidTemplatesTestOutput.java
@@ -5,14 +5,13 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import java.util.HashSet;
-import java.util.Objects;
 import java.util.Set;
 
 /** Code to test the Refaster templates from {@link ValidTemplates}. */
 final class ValidTemplatesTest implements RefasterTemplateTestCase {
   @Override
   public ImmutableSet<?> elidedTypesAndStaticImports() {
-    return ImmutableSet.of(Objects.class, Strings.class);
+    return ImmutableSet.of(Strings.class);
   }
 
   boolean testStringIsEmpty2() {