Introduce IsCharacter matcher and improve test coverage

PicnicSupermarket · Sep 13, 2022 · 79c9139 · 79c9139
1 parent 6f199a3
commit 79c9139
Show file tree

Hide file tree

Showing 11 changed files with 171 additions and 56 deletions.
diff --git a/...ontrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplates.java b/...ontrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplates.java
@@ -6,6 +6,7 @@
 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.NotMatches;
 import com.google.errorprone.refaster.annotation.UseImportPolicy;
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -18,6 +19,7 @@
 import org.assertj.core.api.AbstractLongAssert;
 import org.assertj.core.api.AbstractShortAssert;
 import org.assertj.core.api.NumberAssert;
+import tech.picnic.errorprone.refaster.util.IsCharacter;
 
 final class AssertJNumberTemplates {
   private AssertJNumberTemplates() {}
@@ -226,9 +228,14 @@ AbstractBigDecimalAssert<?> before(AbstractBigDecimalAssert<?> numberAssert) {
     }
   }
 
+  /**
+   * Prefer {@link AbstractLongAssert#isOdd()} over more contrived alternatives.
+   *
+   * <p>Note that for {@link Character}s this rewrite would lead to non-compilable code.
+   */
   static final class AssertThatIsOdd {
     @BeforeTemplate
-    AbstractIntegerAssert<?> before(int number) {
+    AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
       return assertThat(number % 2).isEqualTo(1);
     }
 
@@ -244,9 +251,14 @@ AbstractLongAssert<?> before(long number) {
     }
   }
 
+  /**
+   * Prefer {@link AbstractLongAssert#isEven()} over more contrived alternatives.
+   *
+   * <p>Note that for {@link Character}s this rewrite would lead to non-compilable code.
+   */
   static final class AssertThatIsEven {
     @BeforeTemplate
-    AbstractIntegerAssert<?> before(int number) {
+    AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
       return assertThat(number % 2).isEqualTo(0);
     }
 

diff --git a/...one-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/EqualityTemplates.java b/...one-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/EqualityTemplates.java
@@ -68,17 +68,14 @@ boolean after(boolean b) {
    * Don't negate an equality test or use the ternary operator to compare two booleans; directly
    * test for inequality instead.
    */
+  // XXX: Replacing `a ? !b : b` with `a != b` changes semantics if both `a` and `b` are boxed
+  // booleans.
   static final class Negation {
     @BeforeTemplate
     boolean before(boolean a, boolean b) {
       return Refaster.anyOf(!(a == b), a ? !b : b);
     }
 
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a == b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a == b);
@@ -99,17 +96,14 @@ boolean after(boolean a, boolean b) {
    * Don't negate an inequality test or use the ternary operator to compare two booleans; directly
    * test for equality instead.
    */
+  // XXX: Replacing `a ? b : !b` with `a == b` changes semantics if both `a` and `b` are boxed
+  // booleans.
   static final class IndirectDoubleNegation {
     @BeforeTemplate
     boolean before(boolean a, boolean b) {
       return Refaster.anyOf(!(a != b), a ? b : !b);
     }
 
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a != b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a != b);

diff --git a/...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/PrimitiveTemplates.java b/...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/PrimitiveTemplates.java
@@ -10,11 +10,6 @@ private PrimitiveTemplates() {}
 
   /** Avoid contrived ways of expressing the "less than" relationship. */
   static final class LessThan {
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a >= b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a >= b);
@@ -28,11 +23,6 @@ boolean after(long a, long b) {
 
   /** Avoid contrived ways of expressing the "less than or equal to" relationship. */
   static final class LessThanOrEqualTo {
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a > b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a > b);
@@ -46,11 +36,6 @@ boolean after(long a, long b) {
 
   /** Avoid contrived ways of expressing the "greater than" relationship. */
   static final class GreaterThan {
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a <= b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a <= b);
@@ -64,11 +49,6 @@ boolean after(long a, long b) {
 
   /** Avoid contrived ways of expressing the "greater than or equal to" relationship. */
   static final class GreaterThanOrEqualTo {
-    @BeforeTemplate
-    boolean before(long a, long b) {
-      return !(a < b);
-    }
-
     @BeforeTemplate
     boolean before(double a, double b) {
       return !(a < b);

diff --git a/...t/resources/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplatesTestInput.java b/...t/resources/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplatesTestInput.java
@@ -81,23 +81,27 @@ final class AssertJNumberTemplatesTest implements RefasterTemplateTestCase {
     return ImmutableSet.of(
         assertThat((byte) 1 % 2).isEqualTo(1),
         assertThat(Byte.valueOf((byte) 1) % 2).isEqualTo(1),
+        assertThat((char) 1 % 2).isEqualTo(1),
+        assertThat(Character.valueOf((char) 1) % 2).isEqualTo(1),
+        assertThat((short) 1 % 2).isEqualTo(1),
+        assertThat(Short.valueOf((short) 1) % 2).isEqualTo(1),
         assertThat(1 % 2).isEqualTo(1),
         assertThat(Integer.valueOf(1) % 2).isEqualTo(1),
         assertThat(1L % 2).isEqualTo(1),
-        assertThat(Long.valueOf(1) % 2).isEqualTo(1),
-        assertThat((short) 1 % 2).isEqualTo(1),
-        assertThat(Short.valueOf((short) 1) % 2).isEqualTo(1));
+        assertThat(Long.valueOf(1) % 2).isEqualTo(1));
   }
 
   ImmutableSet<NumberAssert<?, ?>> testAssertThatIsEven() {
     return ImmutableSet.of(
         assertThat((byte) 1 % 2).isEqualTo(0),
         assertThat(Byte.valueOf((byte) 1) % 2).isEqualTo(0),
+        assertThat((char) 1 % 2).isEqualTo(0),
+        assertThat(Character.valueOf((char) 1) % 2).isEqualTo(0),
+        assertThat((short) 1 % 2).isEqualTo(0),
+        assertThat(Short.valueOf((short) 1) % 2).isEqualTo(0),
         assertThat(1 % 2).isEqualTo(0),
         assertThat(Integer.valueOf(1) % 2).isEqualTo(0),
         assertThat(1L % 2).isEqualTo(0),
-        assertThat(Long.valueOf(1) % 2).isEqualTo(0),
-        assertThat((short) 1 % 2).isEqualTo(0),
-        assertThat(Short.valueOf((short) 1) % 2).isEqualTo(0));
+        assertThat(Long.valueOf(1) % 2).isEqualTo(0));
   }
 }
diff --git a/.../resources/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplatesTestOutput.java b/.../resources/tech/picnic/errorprone/refastertemplates/AssertJNumberTemplatesTestOutput.java
@@ -81,23 +81,27 @@ final class AssertJNumberTemplatesTest implements RefasterTemplateTestCase {
     return ImmutableSet.of(
         assertThat((byte) 1).isOdd(),
         assertThat(Byte.valueOf((byte) 1)).isOdd(),
+        assertThat((char) 1 % 2).isEqualTo(1),
+        assertThat(Character.valueOf((char) 1) % 2).isEqualTo(1),
+        assertThat((short) 1).isOdd(),
+        assertThat(Short.valueOf((short) 1)).isOdd(),
         assertThat(1).isOdd(),
         assertThat(Integer.valueOf(1)).isOdd(),
         assertThat(1L).isOdd(),
-        assertThat(Long.valueOf(1)).isOdd(),
-        assertThat((short) 1).isOdd(),
-        assertThat(Short.valueOf((short) 1)).isOdd());
+        assertThat(Long.valueOf(1)).isOdd());
   }
 
   ImmutableSet<NumberAssert<?, ?>> testAssertThatIsEven() {
     return ImmutableSet.of(
         assertThat((byte) 1).isEven(),
         assertThat(Byte.valueOf((byte) 1)).isEven(),
+        assertThat((char) 1 % 2).isEqualTo(0),
+        assertThat(Character.valueOf((char) 1) % 2).isEqualTo(0),
+        assertThat((short) 1).isEven(),
+        assertThat(Short.valueOf((short) 1)).isEven(),
         assertThat(1).isEven(),
         assertThat(Integer.valueOf(1)).isEven(),
         assertThat(1L).isEven(),
-        assertThat(Long.valueOf(1)).isEven(),
-        assertThat((short) 1).isEven(),
-        assertThat(Short.valueOf((short) 1)).isEven());
+        assertThat(Long.valueOf(1)).isEven());
   }
 }
diff --git a/...c/test/resources/tech/picnic/errorprone/refastertemplates/EqualityTemplatesTestInput.java b/...c/test/resources/tech/picnic/errorprone/refastertemplates/EqualityTemplatesTestInput.java
@@ -31,11 +31,13 @@ boolean testDoubleNegation() {
     return !!Boolean.TRUE;
   }
 
+  @SuppressWarnings("SimplifyBooleanExpression")
   ImmutableSet<Boolean> testNegation() {
     return ImmutableSet.of(
-        Boolean.TRUE ? !Boolean.FALSE : Boolean.FALSE,
-        !(Boolean.TRUE == Boolean.FALSE),
+        true ? !false : false,
+        !(true == false),
         !((byte) 3 == (byte) 4),
+        !((char) 3 == (char) 4),
         !((short) 3 == (short) 4),
         !(3 == 4),
         !(3L == 4L),
@@ -44,11 +46,13 @@ ImmutableSet<Boolean> testNegation() {
         !(BoundType.OPEN == BoundType.CLOSED));
   }
 
+  @SuppressWarnings("SimplifyBooleanExpression")
   ImmutableSet<Boolean> testIndirectDoubleNegation() {
     return ImmutableSet.of(
-        Boolean.TRUE ? Boolean.FALSE : !Boolean.FALSE,
-        !(Boolean.TRUE != Boolean.FALSE),
+        true ? false : !false,
+        !(true != false),
         !((byte) 3 != (byte) 4),
+        !((char) 3 != (char) 4),
         !((short) 3 != (short) 4),
         !(3 != 4),
         !(3L != 4L),

diff --git a/.../test/resources/tech/picnic/errorprone/refastertemplates/EqualityTemplatesTestOutput.java b/.../test/resources/tech/picnic/errorprone/refastertemplates/EqualityTemplatesTestOutput.java
@@ -31,11 +31,13 @@ boolean testDoubleNegation() {
     return Boolean.TRUE;
   }
 
+  @SuppressWarnings("SimplifyBooleanExpression")
   ImmutableSet<Boolean> testNegation() {
     return ImmutableSet.of(
-        Boolean.TRUE != Boolean.FALSE,
-        Boolean.TRUE != Boolean.FALSE,
+        true != false,
+        true != false,
         (byte) 3 != (byte) 4,
+        (char) 3 != (char) 4,
         (short) 3 != (short) 4,
         3 != 4,
         3L != 4L,
@@ -44,11 +46,13 @@ ImmutableSet<Boolean> testNegation() {
         BoundType.OPEN != BoundType.CLOSED);
   }
 
+  @SuppressWarnings("SimplifyBooleanExpression")
   ImmutableSet<Boolean> testIndirectDoubleNegation() {
     return ImmutableSet.of(
-        Boolean.TRUE == Boolean.FALSE,
-        Boolean.TRUE == Boolean.FALSE,
+        true == false,
+        true == false,
         (byte) 3 == (byte) 4,
+        (char) 3 == (char) 4,
         (short) 3 == (short) 4,
         3 == 4,
         3L == 4L,

diff --git a/.../test/resources/tech/picnic/errorprone/refastertemplates/PrimitiveTemplatesTestInput.java b/.../test/resources/tech/picnic/errorprone/refastertemplates/PrimitiveTemplatesTestInput.java
@@ -13,6 +13,7 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
   ImmutableSet<Boolean> testLessThan() {
     return ImmutableSet.of(
         !((byte) 3 >= (byte) 4),
+        !((char) 3 >= (char) 4),
         !((short) 3 >= (short) 4),
         !(3 >= 4),
         !(3L >= 4L),
@@ -23,6 +24,7 @@ ImmutableSet<Boolean> testLessThan() {
   ImmutableSet<Boolean> testLessThanOrEqualTo() {
     return ImmutableSet.of(
         !((byte) 3 > (byte) 4),
+        !((char) 3 > (char) 4),
         !((short) 3 > (short) 4),
         !(3 > 4),
         !(3L > 4L),
@@ -33,6 +35,7 @@ ImmutableSet<Boolean> testLessThanOrEqualTo() {
   ImmutableSet<Boolean> testGreaterThan() {
     return ImmutableSet.of(
         !((byte) 3 <= (byte) 4),
+        !((char) 3 <= (char) 4),
         !((short) 3 <= (short) 4),
         !(3 <= 4),
         !(3L <= 4L),
@@ -43,6 +46,7 @@ ImmutableSet<Boolean> testGreaterThan() {
   ImmutableSet<Boolean> testGreaterThanOrEqualTo() {
     return ImmutableSet.of(
         !((byte) 3 < (byte) 4),
+        !((char) 3 < (char) 4),
         !((short) 3 < (short) 4),
         !(3 < 4),
         !(3L < 4L),

diff --git a/...test/resources/tech/picnic/errorprone/refastertemplates/PrimitiveTemplatesTestOutput.java b/...test/resources/tech/picnic/errorprone/refastertemplates/PrimitiveTemplatesTestOutput.java
@@ -12,22 +12,46 @@ public ImmutableSet<?> elidedTypesAndStaticImports() {
 
   ImmutableSet<Boolean> testLessThan() {
     return ImmutableSet.of(
-        (byte) 3 < (byte) 4, (short) 3 < (short) 4, 3 < 4, 3L < 4L, 3F < 4F, 3.0 < 4.0);
+        (byte) 3 < (byte) 4,
+        (char) 3 < (char) 4,
+        (short) 3 < (short) 4,
+        3 < 4,
+        3L < 4L,
+        3F < 4F,
+        3.0 < 4.0);
   }
 
   ImmutableSet<Boolean> testLessThanOrEqualTo() {
     return ImmutableSet.of(
-        (byte) 3 <= (byte) 4, (short) 3 <= (short) 4, 3 <= 4, 3L <= 4L, 3F <= 4F, 3.0 <= 4.0);
+        (byte) 3 <= (byte) 4,
+        (char) 3 <= (char) 4,
+        (short) 3 <= (short) 4,
+        3 <= 4,
+        3L <= 4L,
+        3F <= 4F,
+        3.0 <= 4.0);
   }
 
   ImmutableSet<Boolean> testGreaterThan() {
     return ImmutableSet.of(
-        (byte) 3 > (byte) 4, (short) 3 > (short) 4, 3 > 4, 3L > 4L, 3F > 4F, 3.0 > 4.0);
+        (byte) 3 > (byte) 4,
+        (char) 3 > (char) 4,
+        (short) 3 > (short) 4,
+        3 > 4,
+        3L > 4L,
+        3F > 4F,
+        3.0 > 4.0);
   }
 
   ImmutableSet<Boolean> testGreaterThanOrEqualTo() {
     return ImmutableSet.of(
-        (byte) 3 >= (byte) 4, (short) 3 >= (short) 4, 3 >= 4, 3L >= 4L, 3F >= 4F, 3.0 >= 4.0);
+        (byte) 3 >= (byte) 4,
+        (char) 3 >= (char) 4,
+        (short) 3 >= (short) 4,
+        3 >= 4,
+        3L >= 4L,
+        3F >= 4F,
+        3.0 >= 4.0);
   }
 
   int testLongToIntExact() {

diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/util/IsCharacter.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/util/IsCharacter.java
@@ -0,0 +1,20 @@
+package tech.picnic.errorprone.refaster.util;
+
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.isSameType;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ExpressionTree;
+
+/** A matcher of {@link Character}-typed expressions. */
+public final class IsCharacter implements Matcher<ExpressionTree> {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> DELEGATE =
+      anyOf(isSameType(Character.class), isSameType(s -> s.getSymtab().charType));
+
+  @Override
+  public boolean matches(ExpressionTree tree, VisitorState state) {
+    return DELEGATE.matches(tree, state);
+  }
+}