From cf7b42e74b836acc2b515b8799099dc0738c3c88 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Sat, 25 Dec 2021 12:38:21 +0100 Subject: [PATCH 1/3] Drop rules from `AssertJBigDecimalTemplates` --- .../AssertJBigDecimalTemplates.java | 38 ++++++++++--------- .../AssertJBigDecimalTemplatesTestInput.java | 21 +++------- .../AssertJBigDecimalTemplatesTestOutput.java | 21 +++------- 3 files changed, 33 insertions(+), 47 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java index f623524c52..60a783c851 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java @@ -8,9 +8,22 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate; import java.math.BigDecimal; import org.assertj.core.api.AbstractBigDecimalAssert; +import org.assertj.core.api.BigDecimalAssert; -// XXX: If we add a rule which drops unnecessary `L` suffixes from literal longs, then the `0L`/`1L` -// cases below can go. +/** + * Refaster templates that improve {@link BigDecimal} asserts written in AssertJ. + * + *

A few {@link BigDecimal} assertions are not rewritten. This is because the {@link BigDecimal} + * also uses scale to determine whether two values are equal. As a result, we cannot rewrite the + * following assertions: + * + *

+ */ final class AssertJBigDecimalTemplates { private AssertJBigDecimalTemplates() {} @@ -45,45 +58,36 @@ AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert, static final class AbstractBigDecimalAssertIsZero { @BeforeTemplate AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return Refaster.anyOf( - bigDecimalAssert.isZero(), - bigDecimalAssert.isEqualTo(0L), - bigDecimalAssert.isEqualTo(BigDecimal.ZERO)); + return bigDecimalAssert.isEqualTo(BigDecimal.ZERO); } @AfterTemplate AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isEqualTo(0); + return bigDecimalAssert.isZero(); } } static final class AbstractBigDecimalAssertIsNotZero { @BeforeTemplate AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return Refaster.anyOf( - bigDecimalAssert.isNotZero(), - bigDecimalAssert.isNotEqualTo(0L), - bigDecimalAssert.isNotEqualTo(BigDecimal.ZERO)); + return bigDecimalAssert.isNotEqualTo(BigDecimal.ZERO); } @AfterTemplate AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isNotEqualTo(0); + return bigDecimalAssert.isNotZero(); } } static final class AbstractBigDecimalAssertIsOne { @BeforeTemplate AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return Refaster.anyOf( - bigDecimalAssert.isOne(), - bigDecimalAssert.isEqualTo(1L), - bigDecimalAssert.isEqualTo(BigDecimal.ONE)); + return bigDecimalAssert.isEqualTo(BigDecimal.ONE); } @AfterTemplate AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isEqualTo(1); + return bigDecimalAssert.isOne(); } } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java index 3749240b06..3ca66fbd1b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java @@ -26,24 +26,15 @@ ImmutableSet> testAbstractBigDecimalAssertIsNotEqual assertThat(BigDecimal.ZERO).isNotCloseTo(BigDecimal.ONE, withPercentage(0))); } - ImmutableSet> testAbstractBigDecimalAssertIsZero() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isZero(), - assertThat(BigDecimal.ZERO).isEqualTo(0L), - assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ZERO)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsZero() { + return assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ZERO); } - ImmutableSet> testAbstractBigDecimalAssertIsNotZero() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isNotZero(), - assertThat(BigDecimal.ZERO).isNotEqualTo(0L), - assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ZERO)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsNotZero() { + return assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ZERO); } - ImmutableSet> testAbstractBigDecimalAssertIsOne() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isOne(), - assertThat(BigDecimal.ZERO).isEqualTo(1L), - assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ONE)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsOne() { + return assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ONE); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java index 3172f16f0c..0415c7391b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java @@ -26,24 +26,15 @@ ImmutableSet> testAbstractBigDecimalAssertIsNotEqual assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ONE)); } - ImmutableSet> testAbstractBigDecimalAssertIsZero() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isEqualTo(0), - assertThat(BigDecimal.ZERO).isEqualTo(0), - assertThat(BigDecimal.ZERO).isEqualTo(0)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsZero() { + return assertThat(BigDecimal.ZERO).isZero(); } - ImmutableSet> testAbstractBigDecimalAssertIsNotZero() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isNotEqualTo(0), - assertThat(BigDecimal.ZERO).isNotEqualTo(0), - assertThat(BigDecimal.ZERO).isNotEqualTo(0)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsNotZero() { + return assertThat(BigDecimal.ZERO).isNotZero(); } - ImmutableSet> testAbstractBigDecimalAssertIsOne() { - return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isEqualTo(1), - assertThat(BigDecimal.ZERO).isEqualTo(1), - assertThat(BigDecimal.ZERO).isEqualTo(1)); + AbstractBigDecimalAssert testAbstractBigDecimalAssertIsOne() { + return assertThat(BigDecimal.ZERO).isOne(); } } From 18ef89559e4e4845014eab5f727daaff2abdd4da Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Tue, 8 Mar 2022 08:44:03 +0100 Subject: [PATCH 2/3] Improve explanations --- .../AssertJBigDecimalTemplates.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java index 60a783c851..9de6dfd771 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java @@ -14,14 +14,14 @@ * Refaster templates that improve {@link BigDecimal} asserts written in AssertJ. * *

A few {@link BigDecimal} assertions are not rewritten. This is because the {@link BigDecimal} - * also uses scale to determine whether two values are equal. As a result, we cannot rewrite the - * following assertions: + * also uses scale to determine whether two values are equal. As a result, we cannot perform the + * following rewrites: * *

    - *
  • {@link BigDecimalAssert#isEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and - * BigDecimal.ONE) - *
  • {@link BigDecimalAssert#isNotEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and - * BigDecimal.ONE) + *
  • From {@link BigDecimalAssert#isEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and + * BigDecimal.ONE) to replace the parameter with either 0 or 1. + *
  • From {@link BigDecimalAssert#isNotEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and + * BigDecimal.ONE) to replace the parameter with either 0 or 1. *
*/ final class AssertJBigDecimalTemplates { From 3480d9353caeabf2620046621604644a5155a27f Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 10 Apr 2022 11:30:03 +0200 Subject: [PATCH 3/3] Suggestions --- .../AssertJBigDecimalTemplates.java | 62 ++++--------------- .../AssertJBigDecimalTemplatesTestInput.java | 16 +---- .../AssertJBigDecimalTemplatesTestOutput.java | 24 ++----- 3 files changed, 19 insertions(+), 83 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java index 9de6dfd771..aea7797008 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJBigDecimalTemplates.java @@ -11,23 +11,19 @@ import org.assertj.core.api.BigDecimalAssert; /** - * Refaster templates that improve {@link BigDecimal} asserts written in AssertJ. + * Refaster templates related to AssertJ assertions over {@link BigDecimal}s. * - *

A few {@link BigDecimal} assertions are not rewritten. This is because the {@link BigDecimal} - * also uses scale to determine whether two values are equal. As a result, we cannot perform the - * following rewrites: - * - *

    - *
  • From {@link BigDecimalAssert#isEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and - * BigDecimal.ONE) to replace the parameter with either 0 or 1. - *
  • From {@link BigDecimalAssert#isNotEqualTo(Object)} (for values 0L, 1L, BigDecimal.ZERO, and - * BigDecimal.ONE) to replace the parameter with either 0 or 1. - *
+ *

Note that, contrary to collections of Refaster templates for other {@link + * org.assertj.core.api.NumberAssert} subtypes, these templates do not rewrite to/from {@link + * BigDecimalAssert#isEqualTo(Object)} and {@link BigDecimalAssert#isNotEqualTo(Object)}. This is + * because {@link BigDecimal#equals(Object)} considers not only the numeric value of compared + * instances, but also their scale. As a result various seemingly straightforward transformations + * would actually subtly change the assertion's semantics. */ final class AssertJBigDecimalTemplates { private AssertJBigDecimalTemplates() {} - static final class AbstractBigDecimalAssertIsEqualTo { + static final class AbstractBigDecimalAssertIsEqualByComparingTo { @BeforeTemplate AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert, BigDecimal n) { return Refaster.anyOf( @@ -37,11 +33,11 @@ AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert, @AfterTemplate AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert, BigDecimal n) { - return bigDecimalAssert.isEqualTo(n); + return bigDecimalAssert.isEqualByComparingTo(n); } } - static final class AbstractBigDecimalAssertIsNotEqualTo { + static final class AbstractBigDecimalAssertIsNotEqualByComparingTo { @BeforeTemplate AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert, BigDecimal n) { return Refaster.anyOf( @@ -51,43 +47,7 @@ AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert, @AfterTemplate AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert, BigDecimal n) { - return bigDecimalAssert.isNotEqualTo(n); - } - } - - static final class AbstractBigDecimalAssertIsZero { - @BeforeTemplate - AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isEqualTo(BigDecimal.ZERO); - } - - @AfterTemplate - AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isZero(); - } - } - - static final class AbstractBigDecimalAssertIsNotZero { - @BeforeTemplate - AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isNotEqualTo(BigDecimal.ZERO); - } - - @AfterTemplate - AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isNotZero(); - } - } - - static final class AbstractBigDecimalAssertIsOne { - @BeforeTemplate - AbstractBigDecimalAssert before(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isEqualTo(BigDecimal.ONE); - } - - @AfterTemplate - AbstractBigDecimalAssert after(AbstractBigDecimalAssert bigDecimalAssert) { - return bigDecimalAssert.isOne(); + return bigDecimalAssert.isNotEqualByComparingTo(n); } } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java index 3ca66fbd1b..57d87c293a 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestInput.java @@ -14,27 +14,15 @@ public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of(offset(0), withPercentage(0)); } - ImmutableSet> testAbstractBigDecimalAssertIsEqualTo() { + ImmutableSet> testAbstractBigDecimalAssertIsEqualByComparingTo() { return ImmutableSet.of( assertThat(BigDecimal.ZERO).isCloseTo(BigDecimal.ONE, offset(BigDecimal.ZERO)), assertThat(BigDecimal.ZERO).isCloseTo(BigDecimal.ONE, withPercentage(0))); } - ImmutableSet> testAbstractBigDecimalAssertIsNotEqualTo() { + ImmutableSet> testAbstractBigDecimalAssertIsNotEqualByComparingTo() { return ImmutableSet.of( assertThat(BigDecimal.ZERO).isNotCloseTo(BigDecimal.ONE, offset(BigDecimal.ZERO)), assertThat(BigDecimal.ZERO).isNotCloseTo(BigDecimal.ONE, withPercentage(0))); } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsZero() { - return assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ZERO); - } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsNotZero() { - return assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ZERO); - } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsOne() { - return assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ONE); - } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java index 0415c7391b..232626eea0 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/AssertJBigDecimalTemplatesTestOutput.java @@ -14,27 +14,15 @@ public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of(offset(0), withPercentage(0)); } - ImmutableSet> testAbstractBigDecimalAssertIsEqualTo() { + ImmutableSet> testAbstractBigDecimalAssertIsEqualByComparingTo() { return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ONE), - assertThat(BigDecimal.ZERO).isEqualTo(BigDecimal.ONE)); + assertThat(BigDecimal.ZERO).isEqualByComparingTo(BigDecimal.ONE), + assertThat(BigDecimal.ZERO).isEqualByComparingTo(BigDecimal.ONE)); } - ImmutableSet> testAbstractBigDecimalAssertIsNotEqualTo() { + ImmutableSet> testAbstractBigDecimalAssertIsNotEqualByComparingTo() { return ImmutableSet.of( - assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ONE), - assertThat(BigDecimal.ZERO).isNotEqualTo(BigDecimal.ONE)); - } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsZero() { - return assertThat(BigDecimal.ZERO).isZero(); - } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsNotZero() { - return assertThat(BigDecimal.ZERO).isNotZero(); - } - - AbstractBigDecimalAssert testAbstractBigDecimalAssertIsOne() { - return assertThat(BigDecimal.ZERO).isOne(); + assertThat(BigDecimal.ZERO).isNotEqualByComparingTo(BigDecimal.ONE), + assertThat(BigDecimal.ZERO).isNotEqualByComparingTo(BigDecimal.ONE)); } }