Introduce IsCharacter matcher for use by Refaster templates (#237)

This new matcher is used to improve the `AssertThatIsOdd` and
`AssertThatIsEven` Refaster templates.

While there, apply assorted semi-related test improvements.
This commit is contained in:
Rick Ossendrijver
2022-09-18 14:39:25 +02:00
committed by GitHub
parent b30562bbd8
commit bfc951b61f
11 changed files with 174 additions and 56 deletions

View File

@@ -6,6 +6,7 @@ import static org.assertj.core.api.Assertions.assertThat;
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.AbstractIntegerAssert;
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,16 @@ final class AssertJNumberTemplates {
}
}
/**
* Prefer {@link AbstractLongAssert#isOdd()} (and similar methods for other {@link NumberAssert}
* subtypes) over alternatives with less informative error messages.
*
* <p>Note that {@link org.assertj.core.api.AbstractCharacterAssert} does not implement {@link
* NumberAssert} and does not provide an {@code isOdd} test.
*/
static final class AssertThatIsOdd {
@BeforeTemplate
AbstractIntegerAssert<?> before(int number) {
AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
return assertThat(number % 2).isEqualTo(1);
}
@@ -244,9 +253,16 @@ final class AssertJNumberTemplates {
}
}
/**
* Prefer {@link AbstractLongAssert#isEven()} (and similar methods for other {@link NumberAssert}
* subtypes) over alternatives with less informative error messages.
*
* <p>Note that {@link org.assertj.core.api.AbstractCharacterAssert} does not implement {@link
* NumberAssert} and does not provide an {@code isEven} test.
*/
static final class AssertThatIsEven {
@BeforeTemplate
AbstractIntegerAssert<?> before(int number) {
AbstractIntegerAssert<?> before(@NotMatches(IsCharacter.class) int number) {
return assertThat(number % 2).isEqualTo(0);
}

View File

@@ -68,17 +68,14 @@ final class EqualityTemplates {
* 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 @@ final class EqualityTemplates {
* 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);

View File

@@ -10,11 +10,6 @@ final class 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 @@ final class PrimitiveTemplates {
/** 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 @@ final class PrimitiveTemplates {
/** 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 @@ final class PrimitiveTemplates {
/** 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);

View File

@@ -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));
}
}

View File

@@ -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());
}
}

View File

@@ -31,11 +31,13 @@ final class EqualityTemplatesTest implements RefasterTemplateTestCase {
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 @@ final class EqualityTemplatesTest implements RefasterTemplateTestCase {
!(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),

View File

@@ -31,11 +31,13 @@ final class EqualityTemplatesTest implements RefasterTemplateTestCase {
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 @@ final class EqualityTemplatesTest implements RefasterTemplateTestCase {
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,

View File

@@ -13,6 +13,7 @@ final class PrimitiveTemplatesTest implements RefasterTemplateTestCase {
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 @@ final class PrimitiveTemplatesTest implements RefasterTemplateTestCase {
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 @@ final class PrimitiveTemplatesTest implements RefasterTemplateTestCase {
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 @@ final class PrimitiveTemplatesTest implements RefasterTemplateTestCase {
ImmutableSet<Boolean> testGreaterThanOrEqualTo() {
return ImmutableSet.of(
!((byte) 3 < (byte) 4),
!((char) 3 < (char) 4),
!((short) 3 < (short) 4),
!(3 < 4),
!(3L < 4L),

View File

@@ -12,22 +12,46 @@ final class PrimitiveTemplatesTest implements RefasterTemplateTestCase {
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() {

View File

@@ -0,0 +1,21 @@
package tech.picnic.errorprone.refaster.util;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.suppliers.Suppliers.CHAR_TYPE;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
/** A matcher of {@code char}- and {@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(CHAR_TYPE), isSameType(Character.class));
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
return DELEGATE.matches(tree, state);
}
}

View File

@@ -0,0 +1,63 @@
package tech.picnic.errorprone.refaster.util;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.BugChecker;
import org.junit.jupiter.api.Test;
final class IsCharacterTest {
@Test
void matches() {
CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
.addSourceLines(
"A.java",
"class A {",
" String negative1() {",
" return \"a\";",
" }",
"",
" char[] negative2() {",
" return \"a\".toCharArray();",
" }",
"",
" byte negative3() {",
" return (byte) 0;",
" }",
"",
" int negative4() {",
" return 0;",
" }",
"",
" char positive1() {",
" // BUG: Diagnostic contains:",
" return 'a';",
" }",
"",
" Character positive2() {",
" // BUG: Diagnostic contains:",
" return (Character) null;",
" }",
"",
" char positive3() {",
" // BUG: Diagnostic contains:",
" return (char) 1;",
" }",
"}")
.doTest();
}
/** A {@link BugChecker} which simply delegates to {@link IsCharacter}. */
@BugPattern(summary = "Flags expressions matched by `IsCharacter`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;
// XXX: This is a false positive reported by Checkstyle. See
// https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
@SuppressWarnings("RedundantModifier")
public MatcherTestChecker() {
super(new IsCharacter());
}
}
}