#159 Processes non-static fields before static fields so static field…

…s don't obscure issues in non-static fields
jqno · Sep 6, 2020 · 90d0620 · 90d0620
1 parent 50ae101
commit 90d0620
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 ### Changed
 - Added more helpful error message explaining why EqualsVerifier can't verify subclasses of `java.util.ArrayList`. ([Issue 341](https://github.com/jqno/equalsverifier/issues/341))
+- Changes order of processing fields (non-statics first, statics later) so static fields don't obscure issues that are really in non-static fields. ([Issue 159](https://github.com/jqno/equalsverifier/issues/159))
 
 ### Fixed
 - Added prefab values for `java.net.URL`. ([Issue 340](https://github.com/jqno/equalsverifier/issues/340))

diff --git a/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldIterable.java b/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldIterable.java
@@ -1,6 +1,7 @@
 package nl.jqno.equalsverifier.internal.reflection;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -66,14 +67,22 @@ private List<Field> createFieldList() {
     }
 
     private List<Field> addFieldsFor(Class<?> c) {
-        List<Field> result = new ArrayList<>();
+        List<Field> fields = new ArrayList<>();
+        List<Field> statics = new ArrayList<>();
 
         for (Field field : c.getDeclaredFields()) {
             if (!field.isSynthetic() && !"__cobertura_counters".equals(field.getName())) {
-                result.add(field);
+                if (Modifier.isStatic(field.getModifiers())) {
+                    statics.add(field);
+                } else {
+                    fields.add(field);
+                }
             }
         }
 
+        List<Field> result = new ArrayList<>();
+        result.addAll(fields);
+        result.addAll(statics);
         return result;
     }
 }
diff --git a/...test/java/nl/jqno/equalsverifier/integration/extended_contract/SignificantFieldsTest.java b/...test/java/nl/jqno/equalsverifier/integration/extended_contract/SignificantFieldsTest.java
@@ -324,6 +324,15 @@ public void fail_whenNonNullFieldIsEqualToNullField() {
         EqualsVerifier.forClass(BugWhenFieldIsNull.class).verify();
     }
 
+    @Test
+    public void giveCorrectMessage_whenStaticFieldIsNotUsed_givenStaticFieldIsFirstField() {
+        // See https://github.com/jqno/equalsverifier/issues/159
+        expectFailure("Significant fields", "equals does not use i");
+        EqualsVerifier.forClass(IdentityIntContainer.class)
+                .suppress(Warning.IDENTICAL_COPY)
+                .verify();
+    }
+
     static final class ConstantHashCode {
         private final int x;
         private final int y;
@@ -686,4 +695,23 @@ public int hashCode() {
             return Objects.hashCode(s);
         }
     }
+
+    public static final class IdentityIntContainer {
+        public static final int WHATEVER = 1;
+        private final int i;
+
+        private IdentityIntContainer(int i) {
+            this.i = i;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            return super.equals(obj);
+        }
+
+        @Override
+        public int hashCode() {
+            return i;
+        }
+    }
 }
diff --git a/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldIterableTest.java b/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldIterableTest.java
@@ -92,6 +92,17 @@ public void classInTheMiddleHasNoFields() throws NoSuchFieldException {
         assertEquals(expected, actual);
     }
 
+    @Test
+    public void orderingTest() {
+        FieldIterable iterable = FieldIterable.of(UnorderedFieldContainer.class);
+        List<String> actual = new ArrayList<>();
+        for (Field f : iterable) {
+            actual.add(f.getName());
+        }
+
+        assertEquals(Arrays.asList("one", "two", "THREE", "FOUR"), actual);
+    }
+
     @Test
     public void interfaceTest() {
         FieldIterable iterable = FieldIterable.of(Interface.class);

diff --git a/src/test/java/nl/jqno/equalsverifier/testhelpers/types/TypeHelper.java b/src/test/java/nl/jqno/equalsverifier/testhelpers/types/TypeHelper.java
@@ -169,6 +169,14 @@ public static class SubEmptySubFieldContainer extends EmptySubFieldContainer {
         public long field = 0;
     }
 
+    @SuppressWarnings("unused")
+    public static class UnorderedFieldContainer {
+        public static final int THREE = 3;
+        public final int one = 1;
+        private static final int FOUR = 4;
+        private final int two = 2;
+    }
+
     public interface Interface {}
 
     public abstract static class AbstractClass {