kinow commented on code in PR #838:
URL: https://github.com/apache/commons-lang/pull/838#discussion_r1103699647
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -131,7 +167,15 @@ private boolean accept(final Field field) {
if (Modifier.isTransient(field.getModifiers())) {
return false;
}
- return !Modifier.isStatic(field.getModifiers());
+ if (Modifier.isStatic(field.getModifiers())) {
+ return false;
+ }
+ if (this.excludeFieldNames != null
Review Comment:
>In case you do call setExcludeFieldNames with null, it's still safe here,
guess that's the main reason.
But this if condition is still unreachable even after calling
setExcludeFieldNames with null, so we can drop it too (as condition cannot be
reached for users nor via test code, I think?).
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -128,4 +133,44 @@ public void test_difference_in_inherited_field() {
final DiffResult list = firstObject.diff(secondObject);
assertEquals(1, list.getNumberOfDiffs());
}
+
+ @Test
+ public void test_no_differences_excluded_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.excludedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void test_no_differences_diff_exclude_annotated_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.annotatedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
Review Comment:
Maybe add a test that does both `annotatedField` and `excludedField` being
excluded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
