Field by field comparison was ignoring field and type comparators whe…

…n one of the field to compare was null.

Avoid comparison if both fields are the same (bypassing any comparators)
Make FloatComparator and DoubleComparator handle better null Float/Double

src/main/java/org/assertj/core/internal/DeepDifference.java

@@ -147,7 +147,8 @@ public static List<Difference> determineDifferences(Object a, Object b,
                                                       Map<String, Comparator<?>> comparatorByPropertyOrField,
                                                       TypeComparators comparatorByType) {
     // replace null comparators groups by empty one to simplify code afterwards
-    comparatorByPropertyOrField = comparatorByPropertyOrField == null ? new HashMap<String, Comparator<?>>()
+    comparatorByPropertyOrField = comparatorByPropertyOrField == null
+        ? new HashMap<String, Comparator<?>>()
         : comparatorByPropertyOrField;
     comparatorByType = comparatorByType == null ? new TypeComparators() : comparatorByType;
     return determineDifferences(a, b, null, comparatorByPropertyOrField, comparatorByType);
@@ -172,17 +173,17 @@ private static List<Difference> determineDifferences(Object a, Object b, List<St
         continue;
       }
 
-      if (key1 == null || key2 == null) {
-        differences.add(new Difference(currentPath, key1, key2));
-        continue;
-      }
-
       if (hasCustomComparator(dualKey, comparatorByPropertyOrField, comparatorByType)) {
         if (propertyOrFieldValuesAreEqual(key1, key2, dualKey.getConcatenatedPath(),
                                           comparatorByPropertyOrField, comparatorByType))
           continue;
       }
 
+      if (key1 == null || key2 == null) {
+        differences.add(new Difference(currentPath, key1, key2));
+        continue;
+      }
+
       if (key1 instanceof Collection) {
         if (!(key2 instanceof Collection)) {
           differences.add(new Difference(currentPath, key1, key2));
@@ -315,13 +316,11 @@ private static List<Difference> determineDifferences(Object a, Object b, List<St
 
   private static boolean hasCustomComparator(DualKey dualKey, Map<String, Comparator<?>> comparatorByPropertyOrField,
                                              TypeComparators comparatorByType) {
-    if (dualKey.key1.getClass() == dualKey.key2.getClass()) {
-      String fieldName = dualKey.getConcatenatedPath();
-      return comparatorByPropertyOrField.containsKey(fieldName)
-          ? comparatorByPropertyOrField.get(fieldName) != null
-          : comparatorByType.get(dualKey.key1.getClass()) != null;
-    }
-    return false;
+    String fieldName = dualKey.getConcatenatedPath();
+    if (comparatorByPropertyOrField.containsKey(fieldName)) return true;
+    // we know that dualKey.key1 != dualKey.key2 at this point, so one the key is not null
+    Class<?> keyType = dualKey.key1 != null ? dualKey.key1.getClass() : dualKey.key2.getClass();
+    return comparatorByType.get(keyType) != null;
   }
 
   private static Deque<DualKey> initStack(Object a, Object b, List<String> parentPath,

src/main/java/org/assertj/core/internal/Objects.java

@@ -651,12 +651,16 @@ private <A> ByFieldsComparison isEqualToIgnoringGivenFields(A actual, A other,
   static boolean propertyOrFieldValuesAreEqual(Object actualFieldValue, Object otherFieldValue, String fieldName,
                                                Map<String, Comparator<?>> comparatorByPropertyOrField,
                                                TypeComparators comparatorByType) {
-    if (actualFieldValue != null && otherFieldValue != null
-        && actualFieldValue.getClass() == otherFieldValue.getClass()) {
-      Comparator fieldComparator = comparatorByPropertyOrField.containsKey(fieldName)
-          ? comparatorByPropertyOrField.get(fieldName) : comparatorByType.get(actualFieldValue.getClass());
-      if (fieldComparator != null) return fieldComparator.compare(actualFieldValue, otherFieldValue) == 0;
-    }
+    // no need to look into comparators if objects are the same
+    if (actualFieldValue == otherFieldValue) return true;
+    // check field comparators as they take precedence over type comparators
+    Comparator fieldComparator = comparatorByPropertyOrField.get(fieldName);
+    if (fieldComparator != null) return fieldComparator.compare(actualFieldValue, otherFieldValue) == 0;
+    // check if a type comparators exist for the field type
+    Class fieldType = actualFieldValue != null ? actualFieldValue.getClass() : otherFieldValue.getClass();
+    Comparator typeComparator = comparatorByType.get(fieldType);
+    if (typeComparator != null) return typeComparator.compare(actualFieldValue, otherFieldValue) == 0;
+    // default comparison using equals
     return org.assertj.core.util.Objects.areEqual(actualFieldValue, otherFieldValue);
   }
 
@@ -673,7 +677,8 @@ private <A> boolean canReadFieldValue(Field field, A actual) {
    * @throws AssertionError if actual is {@code null}.
    * @throws AssertionError if some of the fields of the actual object are null.
    */
-  public <A> void assertHasNoNullFieldsOrPropertiesExcept(AssertionInfo info, A actual, String... propertiesOrFieldsToIgnore) {
+  public <A> void assertHasNoNullFieldsOrPropertiesExcept(AssertionInfo info, A actual,
+                                                          String... propertiesOrFieldsToIgnore) {
     assertNotNull(info, actual);
     Set<Field> declaredFieldsIncludingInherited = getDeclaredFieldsIncludingInherited(actual.getClass());
     List<String> nullFieldNames = new LinkedList<>();

src/main/java/org/assertj/core/util/DoubleComparator.java

@@ -24,10 +24,10 @@ public DoubleComparator(double epsilon) {
 
   @Override
   public int compare(Double x, Double y) {
-    if (closeEnough(x, y, epsilon))return 0;
+    if (closeEnough(x, y, epsilon)) return 0;
     return x < y ? -1 : 1;
   }
-  
+
   public double getEpsilon() {
     return epsilon;
   }
@@ -41,23 +41,24 @@ private static boolean complexCloseEnough(double a, double b, double epsilon) {
     final double absB = Math.abs(b);
     final double diff = Math.abs(a - b);
 
-    if (a == b) {
-      // shortcut, handles infinities
-      return true;
-    } else if (a == 0 || b == 0 || diff < Double.MIN_NORMAL) {
+    // shortcut, handles infinities
+    if (a == b) return true;
+    if (a == 0 || b == 0 || diff < Double.MIN_NORMAL) {
       // a or b is zero or both are extremely close to it
       // relative error is less meaningful here
       return diff < (epsilon * Double.MIN_NORMAL);
-    } else { // use relative error
-      return diff / Math.min((absA + absB), Double.MAX_VALUE) < epsilon;
     }
+    // use relative error
+    return diff / Math.min((absA + absB), Double.MAX_VALUE) < epsilon;
   }
 
-  private static boolean closeEnough(double a, double b, double epsilon) {
-    // a == b handles infinities
-    return a == b || Math.abs(a - b) < epsilon;
+  private static boolean closeEnough(Double x, Double y, double epsilon) {
+    // x == y handles infinities
+    if (x == y) return true;
+    if (x == null || y == null) return false;
+    return Math.abs(x - y) < epsilon;
   }
-  
+
   @Override
   public int hashCode() {
     final int prime = 31;
@@ -73,9 +74,7 @@ public boolean equals(Object obj) {
     if (obj == null) return false;
     if (!(obj instanceof DoubleComparator)) return false;
     DoubleComparator other = (DoubleComparator) obj;
-    if (Double.doubleToLongBits(epsilon) != Double.doubleToLongBits(other.epsilon)) return false;
-    return true;
+    return Double.doubleToLongBits(epsilon) == Double.doubleToLongBits(other.epsilon) ? true : false;
   }
 
-
 }

src/main/java/org/assertj/core/util/FloatComparator.java

@@ -33,8 +33,10 @@ public int compare(Float x, Float y) {
   }
 
   private boolean closeEnough(Float x, Float y, float epsilon) {
-    // a == b handles infinities
-    return x == y || Math.abs(x - y) < epsilon;
+    // x == y handles infinities
+    if (x == y) return true;
+    if (x == null || y == null) return false;
+    return Math.abs(x - y) < epsilon;
   }
 
   /**
@@ -46,16 +48,15 @@ private static boolean complexCloseEnough(float a, float b, float epsilon) {
     final float absB = Math.abs(b);
     final float diff = Math.abs(a - b);
 
-    if (a == b) {
-      // shortcut, handles infinities
-      return true;
-    } else if (a == 0 || b == 0 || diff < Float.MIN_NORMAL) {
+    // shortcut, handles infinities
+    if (a == b) return true;
+    if (a == 0 || b == 0 || diff < Float.MIN_NORMAL) {
       // a or b is zero or both are extremely close to it
       // relative error is less meaningful here
       return diff < (epsilon * Float.MIN_NORMAL);
-    } else { // use relative error
-      return diff / Math.min((absA + absB), Float.MAX_VALUE) < epsilon;
     }
+    // use relative error
+    return diff / Math.min((absA + absB), Float.MAX_VALUE) < epsilon;
   }
 
   @Override
@@ -69,8 +70,7 @@ public boolean equals(Object obj) {
     if (obj == null) return false;
     if (!(obj instanceof FloatComparator)) return false;
     FloatComparator other = (FloatComparator) obj;
-    if (Float.floatToIntBits(epsilon) != Float.floatToIntBits(other.epsilon)) return false;
-    return true;
+    return Float.floatToIntBits(epsilon) == Float.floatToIntBits(other.epsilon) ? true : false;
   }
 
 }

src/test/java/org/assertj/core/api/atomic/referencearray/AtomicReferenceArrayAssert_usingElementComparatorIgnoringFields_Test.java

@@ -13,7 +13,7 @@
 package org.assertj.core.api.atomic.referencearray;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
+import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_STRING;
 
 import java.util.Comparator;
 
@@ -56,7 +56,7 @@ public void should_be_able_to_use_a_comparator_for_specified_fields_of_elements_
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingElementComparatorIgnoringFields("lightSaberColor")
                                      .contains(other);
   }
@@ -71,7 +71,7 @@ public int compare(String o1, String o2) {
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingComparatorForElementFieldsWithType(comparator, String.class)
                                      .usingElementComparatorIgnoringFields("lightSaberColor")
                                      .contains(other);
@@ -82,7 +82,7 @@ public void should_be_able_to_use_a_comparator_for_element_fields_with_specified
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "blue");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS, String.class)
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS_STRING, String.class)
                                      .usingElementComparatorIgnoringFields("name")
                                      .contains(other);
   }

src/test/java/org/assertj/core/api/atomic/referencearray/AtomicReferenceArrayAssert_usingElementComparatorOnFields_Test.java

@@ -13,7 +13,7 @@
 package org.assertj.core.api.atomic.referencearray;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
+import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_STRING;
 
 import java.util.Comparator;
 
@@ -55,7 +55,7 @@ public void should_be_able_to_use_a_comparator_for_specified_fields_of_elements_
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingElementComparatorOnFields("name", "lightSaberColor")
                                      .contains(other);
   }
@@ -70,7 +70,7 @@ public int compare(String o1, String o2) {
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingComparatorForElementFieldsWithType(comparator, String.class)
                                      .usingElementComparatorOnFields("name", "lightSaberColor")
                                      .contains(other);
@@ -81,7 +81,7 @@ public void should_be_able_to_use_a_comparator_for_element_fields_with_specified
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "blue");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS, String.class)
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS_STRING, String.class)
                                      .usingElementComparatorOnFields("name", "lightSaberColor")
                                      .contains(other);
   }

src/test/java/org/assertj/core/api/atomic/referencearray/AtomicReferenceArrayAssert_usingFieldByFieldElementComparator_Test.java

@@ -14,7 +14,7 @@
 
 import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
+import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_STRING;
 import static org.assertj.core.test.TestFailures.failBecauseExpectedAssertionErrorWasNotThrown;
 import static org.assertj.core.util.Arrays.array;
 
@@ -185,7 +185,7 @@ public void should_be_able_to_use_a_comparator_for_specified_fields_of_elements_
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                              .usingFieldByFieldElementComparator()
                              .contains(other);
   }
@@ -200,7 +200,7 @@ public int compare(String o1, String o2) {
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                              .usingComparatorForElementFieldsWithType(comparator, String.class)
                              .usingFieldByFieldElementComparator()
                              .contains(other);
@@ -211,7 +211,7 @@ public void should_be_able_to_use_a_comparator_for_element_fields_with_specified
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "blue");
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS, String.class)
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS_STRING, String.class)
                              .usingFieldByFieldElementComparator()
                              .contains(other);
   }

src/test/java/org/assertj/core/api/atomic/referencearray/AtomicReferenceArrayAssert_usingRecursiveFieldByFieldElementComparator_Test.java

@@ -14,7 +14,7 @@
 
 import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
+import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_STRING;
 import static org.assertj.core.test.TestFailures.failBecauseExpectedAssertionErrorWasNotThrown;
 
 import java.util.Comparator;
@@ -127,7 +127,7 @@ public int compare(String o1, String o2) {
     Foo actual = new Foo("1", new Bar(1));
     Foo other = new Foo("2", new Bar(1));
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "id")
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "id")
                                      .usingComparatorForElementFieldsWithType(comparator, String.class)
                                      .usingRecursiveFieldByFieldElementComparator()
                                      .contains(other);
@@ -138,7 +138,7 @@ public void should_be_able_to_use_a_comparator_for_element_fields_with_specified
     Foo actual = new Foo("1", new Bar(1));
     Foo other = new Foo("2", new Bar(1));
 
-    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS, String.class)
+    assertThat(atomicArrayOf(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS_STRING, String.class)
                                      .usingRecursiveFieldByFieldElementComparator()
                                      .contains(other);
   }

src/test/java/org/assertj/core/api/iterable/IterableAssert_usingElementComparatorIgnoringFields_Test.java

@@ -14,7 +14,7 @@
 
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
+import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_STRING;
 
 import java.util.Comparator;
 
@@ -56,7 +56,7 @@ public void should_be_able_to_use_a_comparator_for_specified_fields_of_elements_
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingElementComparatorIgnoringFields("lightSaberColor")
                                      .contains(other);
   }
@@ -71,7 +71,7 @@ public int compare(String o1, String o2) {
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "green");
 
-    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS, "name")
+    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithNames(ALWAY_EQUALS_STRING, "name")
                                      .usingComparatorForElementFieldsWithType(comparator, String.class)
                                      .usingElementComparatorIgnoringFields("lightSaberColor")
                                      .contains(other);
@@ -82,7 +82,7 @@ public void should_be_able_to_use_a_comparator_for_element_fields_with_specified
     Jedi actual = new Jedi("Yoda", "green");
     Jedi other = new Jedi("Luke", "blue");
 
-    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS, String.class)
+    assertThat(singletonList(actual)).usingComparatorForElementFieldsWithType(ALWAY_EQUALS_STRING, String.class)
                                      .usingElementComparatorIgnoringFields("name")
                                      .contains(other);
   }