Repository: assertj/assertj (Java)

Issue:
`returns` and `doesNotReturn` should honor custom type comparators
#### Feature summary

Currently, comparators set via `usingComparatorForType` are ignored by `returns` and `doesNotReturn` assertions.

We could enhance the implementation so that custom comparators are also used with these two methods.

Source: https://stackoverflow.com/questions/73121345/

#### Example

```java
@Test
void test() {
    assertThat(new BigDecimal("10.00"))
        .usingComparatorForType(BigDecimalComparator.BIG_DECIMAL_COMPARATOR, BigDecimal.class)
        .returns(BigDecimal.TEN, BigDecimal::abs); // currently fails
}
```


Hints:

Patch:
diff --git a/assertj-core/src/main/java/org/assertj/core/api/AbstractObjectAssert.java b/assertj-core/src/main/java/org/assertj/core/api/AbstractObjectAssert.java
--- a/assertj-core/src/main/java/org/assertj/core/api/AbstractObjectAssert.java
+++ b/assertj-core/src/main/java/org/assertj/core/api/AbstractObjectAssert.java
@@ -31,6 +31,8 @@
 import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
 import org.assertj.core.description.Description;
 import org.assertj.core.groups.Tuple;
+import org.assertj.core.internal.ComparatorBasedComparisonStrategy;
+import org.assertj.core.internal.Objects;
 import org.assertj.core.internal.TypeComparators;
 import org.assertj.core.util.CheckReturnValue;
 import org.assertj.core.util.introspection.IntrospectionError;
@@ -55,8 +57,8 @@
 public abstract class AbstractObjectAssert<SELF extends AbstractObjectAssert<SELF, ACTUAL>, ACTUAL>
     extends AbstractAssert<SELF, ACTUAL> {
 
-  private Map<String, Comparator<?>> comparatorByPropertyOrField = new TreeMap<>();
-  private TypeComparators comparatorByType;
+  private Map<String, Comparator<?>> comparatorsByPropertyOrField = new TreeMap<>();
+  private TypeComparators comparatorsByType;
 
   public AbstractObjectAssert(ACTUAL actual, Class<?> selfType) {
     super(actual, selfType);
@@ -143,7 +145,7 @@ public SELF as(String description, Object... args) {
    */
   @Deprecated
   public SELF isEqualToIgnoringNullFields(Object other) {
-    objects.assertIsEqualToIgnoringNullFields(info, actual, other, comparatorByPropertyOrField, getComparatorsByType());
+    objects.assertIsEqualToIgnoringNullFields(info, actual, other, comparatorsByPropertyOrField, getComparatorsByType());
     return myself;
   }
 
@@ -219,7 +221,7 @@ public SELF isEqualToIgnoringNullFields(Object other) {
    */
   @Deprecated
   public SELF isEqualToComparingOnlyGivenFields(Object other, String... propertiesOrFieldsUsedInComparison) {
-    objects.assertIsEqualToComparingOnlyGivenFields(info, actual, other, comparatorByPropertyOrField, getComparatorsByType(),
+    objects.assertIsEqualToComparingOnlyGivenFields(info, actual, other, comparatorsByPropertyOrField, getComparatorsByType(),
                                                     propertiesOrFieldsUsedInComparison);
     return myself;
   }
@@ -291,7 +293,7 @@ public SELF isEqualToComparingOnlyGivenFields(Object other, String... properties
    */
   @Deprecated
   public SELF isEqualToIgnoringGivenFields(Object other, String... propertiesOrFieldsToIgnore) {
-    objects.assertIsEqualToIgnoringGivenFields(info, actual, other, comparatorByPropertyOrField, getComparatorsByType(),
+    objects.assertIsEqualToIgnoringGivenFields(info, actual, other, comparatorsByPropertyOrField, getComparatorsByType(),
                                                propertiesOrFieldsToIgnore);
     return myself;
   }
@@ -479,14 +481,14 @@ public SELF hasAllNullFieldsOrPropertiesExcept(String... propertiesOrFieldsToIgn
    */
   @Deprecated
   public SELF isEqualToComparingFieldByField(Object other) {
-    objects.assertIsEqualToIgnoringGivenFields(info, actual, other, comparatorByPropertyOrField, getComparatorsByType());
+    objects.assertIsEqualToIgnoringGivenFields(info, actual, other, comparatorsByPropertyOrField, getComparatorsByType());
     return myself;
   }
 
   // lazy init TypeComparators
   protected TypeComparators getComparatorsByType() {
-    if (comparatorByType == null) comparatorByType = defaultTypeComparators();
-    return comparatorByType;
+    if (comparatorsByType == null) comparatorsByType = defaultTypeComparators();
+    return comparatorsByType;
   }
 
   /**
@@ -535,14 +537,14 @@ protected TypeComparators getComparatorsByType() {
    *                  .isEqualToComparingFieldByField(reallyTallFrodo);</code></pre>
    *
    * @param <T> the type of values to compare.
-   * @param comparator the {@link java.util.Comparator} to use
+   * @param comparator the {@link Comparator} to use
    * @param propertiesOrFields the names of the properties and/or fields the comparator should be used for
    * @return {@code this} assertions object
    */
   @CheckReturnValue
   public <T> SELF usingComparatorForFields(Comparator<T> comparator, String... propertiesOrFields) {
     for (String propertyOrField : propertiesOrFields) {
-      comparatorByPropertyOrField.put(propertyOrField, comparator);
+      comparatorsByPropertyOrField.put(propertyOrField, comparator);
     }
     return myself;
   }
@@ -553,7 +555,9 @@ public <T> SELF usingComparatorForFields(Comparator<T> comparator, String... pro
    * <p>
    * Comparators specified by {@link #usingComparatorForFields} have precedence over comparators specified by this method.
    * <p>
-   * The comparators specified by this method are only used for field by field comparison like {@link #isEqualToComparingFieldByField(Object)}.
+   * The comparators specified by this method are used for field by field comparison like
+   * {@link #isEqualToComparingFieldByField(Object)}, but also for {@link #returns(Object, Function)} and
+   * {@link #doesNotReturn(Object, Function)} assertions.
    * <p>
    * Example:
    * <pre><code class='java'> public class TolkienCharacter {
@@ -597,10 +601,13 @@ public <T> SELF usingComparatorForFields(Comparator<T> comparator, String... pro
    * <li>The comparator of an interface implemented by the given {@code type}</li>
    * </ol>
    *
-   * @param comparator the {@link java.util.Comparator} to use
-   * @param type the {@link java.lang.Class} of the type the comparator should be used for
+   * @param comparator the {@link Comparator} to use
+   * @param type the {@link Class} of the type the comparator should be used for
    * @param <T> the type of objects that the comparator should be used for
    * @return {@code this} assertions object
+   * @see #isEqualToComparingFieldByField(Object)
+   * @see #returns(Object, Function)
+   * @see #doesNotReturn(Object, Function)
    */
   @CheckReturnValue
   public <T> SELF usingComparatorForType(Comparator<? super T> comparator, Class<T> type) {
@@ -1066,7 +1073,7 @@ public <T> AbstractObjectAssert<?, T> extracting(Function<? super ACTUAL, T> ext
    */
   @Deprecated
   public SELF isEqualToComparingFieldByFieldRecursively(Object other) {
-    objects.assertIsEqualToComparingFieldByFieldRecursively(info, actual, other, comparatorByPropertyOrField,
+    objects.assertIsEqualToComparingFieldByFieldRecursively(info, actual, other, comparatorsByPropertyOrField,
                                                             getComparatorsByType());
     return myself;
   }
@@ -1077,6 +1084,8 @@ public SELF isEqualToComparingFieldByFieldRecursively(Object other) {
    * <p>
    * Wrapping the given {@link Function} with {@link Assertions#from(Function)} makes the assertion more readable.
    * <p>
+   * The assertion supports custom comparators, configurable with {@link #usingComparatorForType(Comparator, Class)}.
+   * <p>
    * Example:
    * <pre><code class="java"> // from is not mandatory but it makes the assertions more readable
    * assertThat(frodo).returns("Frodo", from(TolkienCharacter::getName))
@@ -1088,9 +1097,12 @@ public SELF isEqualToComparingFieldByFieldRecursively(Object other) {
    * @param <T> the expected value type the given {@code method} returns.
    * @return {@code this} assertion object.
    * @throws NullPointerException if given {@code from} function is null
+   *
+   * @see #usingComparatorForType(Comparator, Class)
    */
   public <T> SELF returns(T expected, Function<ACTUAL, T> from) {
     requireNonNull(from, "The given getter method/Function must not be null");
+    Objects objects = getComparatorBasedObjectAssertions(expected.getClass());
     objects.assertEqual(info, from.apply(actual), expected);
     return myself;
   }
@@ -1101,6 +1113,8 @@ public <T> SELF returns(T expected, Function<ACTUAL, T> from) {
    * <p>
    * Wrapping the given {@link Function} with {@link Assertions#from(Function)} makes the assertion more readable.
    * <p>
+   * The assertion supports custom comparators, configurable with {@link #usingComparatorForType(Comparator, Class)}.
+   * <p>
    * Example:
    * <pre><code class="java"> // from is not mandatory but it makes the assertions more readable
    * assertThat(frodo).doesNotReturn("Bilbo", from(TolkienCharacter::getName))
@@ -1114,13 +1128,23 @@ public <T> SELF returns(T expected, Function<ACTUAL, T> from) {
    * @throws NullPointerException if given {@code from} function is null
    *
    * @since 3.22.0
+   * @see #usingComparatorForType(Comparator, Class)
    */
   public <T> SELF doesNotReturn(T expected, Function<ACTUAL, T> from) {
     requireNonNull(from, "The given getter method/Function must not be null");
+    Objects objects = getComparatorBasedObjectAssertions(expected.getClass());
     objects.assertNotEqual(info, from.apply(actual), expected);
     return myself;
   }
 
+  private Objects getComparatorBasedObjectAssertions(Class<?> type) {
+    TypeComparators comparatorsByType = getComparatorsByType();
+    if (comparatorsByType.hasComparatorForType(type)) {
+      return new Objects(new ComparatorBasedComparisonStrategy(comparatorsByType.getComparatorForType(type)));
+    }
+    return objects;
+  }
+
   /**
    * Enable using a recursive field by field comparison strategy when calling the chained {@link RecursiveComparisonAssert#isEqualTo(Object) isEqualTo} assertion.
    * <p>
@@ -1199,7 +1223,7 @@ public RecursiveComparisonAssert<?> usingRecursiveComparison() {
    */
   @Override
   public RecursiveComparisonAssert<?> usingRecursiveComparison(RecursiveComparisonConfiguration recursiveComparisonConfiguration) {
-    return super.usingRecursiveComparison(recursiveComparisonConfiguration).withTypeComparators(comparatorByType);
+    return super.usingRecursiveComparison(recursiveComparisonConfiguration).withTypeComparators(comparatorsByType);
   }
 
   /**
@@ -1305,19 +1329,19 @@ protected <T> AbstractObjectAssert<?, T> newObjectAssert(T objectUnderTest) {
   SELF withAssertionState(AbstractAssert assertInstance) {
     if (assertInstance instanceof AbstractObjectAssert) {
       AbstractObjectAssert objectAssert = (AbstractObjectAssert) assertInstance;
-      return (SELF) super.withAssertionState(assertInstance).withTypeComparator(objectAssert.comparatorByType)
-                                                            .withComparatorByPropertyOrField(objectAssert.comparatorByPropertyOrField);
+      return (SELF) super.withAssertionState(assertInstance).withTypeComparator(objectAssert.comparatorsByType)
+                                                            .withComparatorByPropertyOrField(objectAssert.comparatorsByPropertyOrField);
     }
     return super.withAssertionState(assertInstance);
   }
 
   SELF withTypeComparator(TypeComparators comparatorsByType) {
-    this.comparatorByType = comparatorsByType;
+    this.comparatorsByType = comparatorsByType;
     return myself;
   }
 
   SELF withComparatorByPropertyOrField(Map<String, Comparator<?>> comparatorsToPropaget) {
-    this.comparatorByPropertyOrField = comparatorsToPropaget;
+    this.comparatorsByPropertyOrField = comparatorsToPropaget;
     return myself;
   }
 


Test patch:
diff --git a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_doesNotReturn_Test.java b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_doesNotReturn_Test.java
--- a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_doesNotReturn_Test.java
+++ b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_doesNotReturn_Test.java
@@ -12,14 +12,18 @@
  */
 package org.assertj.core.api.object;
 
+import static java.lang.String.CASE_INSENSITIVE_ORDER;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.assertj.core.api.Assertions.from;
 import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.error.ShouldNotBeEqual.shouldNotBeEqual;
+import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
 import static org.mockito.Mockito.verify;
 
 import org.assertj.core.api.ObjectAssert;
 import org.assertj.core.api.ObjectAssertBaseTest;
+import org.assertj.core.internal.ComparatorBasedComparisonStrategy;
 import org.assertj.core.test.Jedi;
 import org.junit.jupiter.api.Test;
 
@@ -53,4 +57,17 @@ void perform_assertion_like_users() {
                     .doesNotReturn("Luke", Jedi::getName);
   }
 
+  @Test
+  void should_honor_custom_type_comparator() {
+    // GIVEN
+    Jedi yoda = new Jedi("Yoda", "Green");
+    // WHEN
+    AssertionError assertionError = expectAssertionError(() -> assertThat(yoda).usingComparatorForType(CASE_INSENSITIVE_ORDER,
+                                                                                                       String.class)
+                                                                               .doesNotReturn("YODA", from(Jedi::getName)));
+    // THEN
+    then(assertionError).hasMessage(shouldNotBeEqual("Yoda", "YODA",
+                                                     new ComparatorBasedComparisonStrategy(CASE_INSENSITIVE_ORDER)).create());
+  }
+
 }
diff --git a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_extracting_with_Function_Test.java b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_extracting_with_Function_Test.java
--- a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_extracting_with_Function_Test.java
+++ b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_extracting_with_Function_Test.java
@@ -115,12 +115,12 @@ private static Objects comparatorOf(AbstractObjectAssert<?, ?> assertion) {
   }
 
   private static TypeComparators comparatorsByTypeOf(AbstractObjectAssert<?, ?> assertion) {
-    return (TypeComparators) PropertyOrFieldSupport.EXTRACTION.getValueOf("comparatorByType", assertion);
+    return (TypeComparators) PropertyOrFieldSupport.EXTRACTION.getValueOf("comparatorsByType", assertion);
   }
 
   @SuppressWarnings("unchecked")
   private static Map<String, Comparator<?>> comparatorByPropertyOrFieldOf(AbstractObjectAssert<?, ?> assertion) {
-    return (Map<String, Comparator<?>>) PropertyOrFieldSupport.EXTRACTION.getValueOf("comparatorByPropertyOrField", assertion);
+    return (Map<String, Comparator<?>>) PropertyOrFieldSupport.EXTRACTION.getValueOf("comparatorsByPropertyOrField", assertion);
   }
 
   @Override
diff --git a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_returns_Test.java b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_returns_Test.java
--- a/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_returns_Test.java
+++ b/assertj-core/src/test/java/org/assertj/core/api/object/ObjectAssert_returns_Test.java
@@ -12,6 +12,7 @@
  */
 package org.assertj.core.api.object;
 
+import static java.lang.String.CASE_INSENSITIVE_ORDER;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.assertj.core.api.Assertions.from;
@@ -56,4 +57,13 @@ void perform_assertion_like_users() {
                     .returns("Yoda", Jedi::getName);
   }
 
+  @Test
+  void should_honor_custom_type_comparator() {
+    // GIVEN
+    Jedi yoda = new Jedi("Yoda", "Green");
+    // WHEN/THEN
+    assertThat(yoda).usingComparatorForType(CASE_INSENSITIVE_ORDER, String.class)
+                    .returns("YODA", from(Jedi::getName));
+  }
+
 }