[LANG-1657] DiffBuilder: Type constraint for method append(...,

DiffResult) too strict #786

Solution from PR #786 but with more tests

src/changes/changes.xml

@@ -144,6 +144,7 @@ The <action> type attribute can be add,update,fix,remove.
     <action                   type="fix" dev="ggregory" due-to="Tobias Kiecker">Same Javadoc changes as [TEXT-234] #1223.</action>  
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">Remove duplicate static data in SerializationUtils.ClassLoaderAwareObjectInputStream.</action> 
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory, Henri Yandell, Fabrice Benhamouda">Reimplement RandomStringUtils on top of SecureRandom#getInstanceStrong() #1235.</action>
+    <action issue="LANG-1657" type="fix" dev="ggregory" due-to="Matthias Welz, Andrew Thomas, Gary Gregory">DiffBuilder: Type constraint for method append(..., DiffResult) too strict #786.</action> 
     <!-- UPDATE -->
     <action                   type="update" dev="sebb" due-to="Dependabot, Gary Gregory">Bump commons-parent from 64 to 71 #1194, #1233.</action>
     <action                   type="update" dev="ggregory" due-to="Dependabot">Bump org.codehaus.mojo:exec-maven-plugin from 3.1.1 to 3.3.0 #1175, #1224.</action>

src/main/java/org/apache/commons/lang3/builder/DiffBuilder.java

@@ -368,7 +368,7 @@ public class DiffBuilder<T> implements Builder<DiffResult<T>> {
      * @throws NullPointerException if field name is {@code null} or diffResult is {@code null}
      * @since 3.5
      */
-    public DiffBuilder<T> append(final String fieldName, final DiffResult<T> diffResult) {
+    public DiffBuilder<T> append(final String fieldName, final DiffResult<?> diffResult) {
         Objects.requireNonNull(diffResult, "diffResult");
         if (equals) {
             return this;

src/test/java/org/apache/commons/lang3/builder/DiffBuilderTest.java

@@ -26,6 +26,8 @@ import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.List;
+
 import org.apache.commons.lang3.AbstractLangTest;
 import org.apache.commons.lang3.ArrayUtils;
 import org.hamcrest.Matcher;
@@ -36,7 +38,40 @@ import org.junit.jupiter.api.Test;
  */
 public class DiffBuilderTest extends AbstractLangTest {
 
+
+    /**
+     * Test fixture.
+     */
+    private static class NestedTypeTestClass implements Diffable<NestedTypeTestClass> {
+
+        private final ToStringStyle style = SHORT_STYLE;
+        private boolean booleanField = true;
+
+        @Override
+        public DiffResult<NestedTypeTestClass> diff(final NestedTypeTestClass obj) {
+            // @formatter:off
+            return new DiffBuilder<>(this, obj, style)
+                    .append("boolean", booleanField, obj.booleanField)
+                    .build();
+            // @formatter:on
+        }
+
+        @Override
+        public boolean equals(final Object obj) {
+            return EqualsBuilder.reflectionEquals(this, obj, false);
+        }
+
+        @Override
+        public int hashCode() {
+            return HashCodeBuilder.reflectionHashCode(this, false);
+        }
+    }
+
+    /**
+     * Test fixture.
+     */
     private static final class TypeTestClass implements Diffable<TypeTestClass> {
+
         private ToStringStyle style = SHORT_STYLE;
         private boolean booleanField = true;
         private boolean[] booleanArrayField = { true };
@@ -56,6 +91,7 @@ public class DiffBuilderTest extends AbstractLangTest {
         private short[] shortArrayField = { 1 };
         private Object objectField;
         private Object[] objectArrayField = { null };
+        private final NestedTypeTestClass nestedDiffableField = new NestedTypeTestClass();
 
         @Override
         public DiffResult<TypeTestClass> diff(final TypeTestClass obj) {
@@ -79,6 +115,7 @@ public class DiffBuilderTest extends AbstractLangTest {
                     .append("shortArray", shortArrayField, obj.shortArrayField)
                     .append("objectField", objectField, obj.objectField)
                     .append("objectArrayField", objectArrayField, obj.objectArrayField)
+                    .append("nestedDiffableField", nestedDiffableField.diff(obj.nestedDiffableField))
                     .build();
             // @formatter:on
         }
@@ -160,7 +197,7 @@ public class DiffBuilderTest extends AbstractLangTest {
                 .append("foo", new short[] { 1 }, new short[] { 1 })
                 .append("foo", new Object[] { 1, "two" }, new Object[] { 1, "two" })
                 .build();
-        // @formatter:off
+        // @formatter:on
         assertEquals(0, list.getNumberOfDiffs());
     }
 
@@ -178,7 +215,7 @@ public class DiffBuilderTest extends AbstractLangTest {
                 .append("foo", new short[] { 1 }, new short[] { 1 })
                 .append("foo", new Object[] { 1, "two" }, new Object[] { 1, "two" })
                 .build();
-        // @formatter:off
+        // @formatter:on
         assertEquals(0, list.getNumberOfDiffs());
     }
 
@@ -323,6 +360,60 @@ public class DiffBuilderTest extends AbstractLangTest {
         assertArrayEquals(ArrayUtils.toObject(class2.longArrayField), (Object[]) diff.getRight());
     }
 
+    @Test
+    public void testNestedDiffableNo() {
+        final TypeTestClass class1 = new TypeTestClass();
+        final TypeTestClass class2 = new TypeTestClass();
+        final DiffResult<TypeTestClass> list = class1.diff(class2);
+        assertEquals(0, list.getNumberOfDiffs());
+        final List<Diff<?>> diff = list.getDiffs();
+        assertTrue(diff.isEmpty());
+    }
+
+    @Test
+    public void testNestedDiffableYesNestedOnly() {
+        final TypeTestClass class1 = new TypeTestClass();
+        final TypeTestClass class2 = new TypeTestClass();
+        class2.nestedDiffableField.booleanField = false;
+        final DiffResult<TypeTestClass> list = class1.diff(class2);
+        assertEquals(1, list.getNumberOfDiffs());
+        final Diff<?> diff = list.getDiffs().get(0);
+        assertEquals(Object.class, diff.getType());
+        assertEquals(Boolean.TRUE, diff.getLeft());
+        assertEquals(Boolean.FALSE, diff.getRight());
+    }
+
+    @Test
+    public void testNestedDiffableYesNestedNot() {
+        final TypeTestClass class1 = new TypeTestClass();
+        final TypeTestClass class2 = new TypeTestClass();
+        class2.intField = 9;
+        final DiffResult<TypeTestClass> list = class1.diff(class2);
+        assertEquals(1, list.getNumberOfDiffs());
+        final Diff<?> diff = list.getDiffs().get(0);
+        assertEquals(Integer.class, diff.getType());
+        assertEquals(1, diff.getLeft());
+        assertEquals(9, diff.getRight());
+    }
+
+    @Test
+    public void testNestedDiffableYesNestedYes() {
+        final TypeTestClass class1 = new TypeTestClass();
+        final TypeTestClass class2 = new TypeTestClass();
+        class2.intField = 9;
+        class2.nestedDiffableField.booleanField = false;
+        final DiffResult<TypeTestClass> list = class1.diff(class2);
+        assertEquals(2, list.getNumberOfDiffs());
+        final Diff<?> diff0 = list.getDiffs().get(0);
+        assertEquals(Integer.class, diff0.getType());
+        assertEquals(1, diff0.getLeft());
+        assertEquals(9, diff0.getRight());
+        final Diff<?> diff1 = list.getDiffs().get(1);
+        assertEquals(Object.class, diff1.getType());
+        assertEquals(Boolean.TRUE, diff1.getLeft());
+        assertEquals(Boolean.FALSE, diff1.getRight());
+    }
+
     @Test
     public void testNullLhs() {
         assertThrows(NullPointerException.class, () -> new DiffBuilder<>(null, this, ToStringStyle.DEFAULT_STYLE));