diff --git a/CHANGELOG.md b/CHANGELOG.md index 95d1b3b3f..19fa350ab 100644 --- 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 index 3b89cf609..31e60c5b4 100644 --- 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 createFieldList() { } private List addFieldsFor(Class c) { - List result = new ArrayList<>(); + List fields = new ArrayList<>(); + List 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 result = new ArrayList<>(); + result.addAll(fields); + result.addAll(statics); return result; } } diff --git a/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SignificantFieldsTest.java b/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SignificantFieldsTest.java index ee6c54b53..1573b4387 100644 --- a/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SignificantFieldsTest.java +++ b/src/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 index a811a7089..607fea4f7 100644 --- 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 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 index ca882ae06..558660038 100644 --- 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 {