From 242604acb03f0a5f70d2181e21aa59800676e976 Mon Sep 17 00:00:00 2001 From: Joe Schmetzer Date: Sun, 22 Sep 2024 11:38:17 +1000 Subject: [PATCH] Allow matching against polymorphic collections (#422) This fix PECS rule (producer extends, consumer super) to the Hamcrest IsIterableContaining matcher, as well as dependant implementations. In this instance, a collection of items should be treated as a producer according to this rule, while a matcher acts as a consumer. There is also an extra test for type variance in hasEntry (#107) Closes #252 --- CHANGES.md | 19 +++- .../main/java/org/hamcrest/CoreMatchers.java | 8 +- .../src/main/java/org/hamcrest/Matchers.java | 8 +- .../hamcrest/collection/HasItemInArray.java | 2 +- .../IsIterableContainingInAnyOrder.java | 3 +- .../hamcrest/core/IsCollectionContaining.java | 12 +-- .../hamcrest/core/IsIterableContaining.java | 18 ++-- .../collection/IsMapContainingTest.java | 8 ++ .../core/IsCollectionContainingTest.java | 16 ++-- .../core/IsIterableContainingTest.java | 86 ++++++++++++++++--- 10 files changed, 136 insertions(+), 44 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 637771ae..f638223d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,22 @@ ## Version 3.1 (Unreleased) +### Breaking Changes + +* As a result of the bugfix to allow matching against polymorphic collections +([PR #422](https://github.com/hamcrest/JavaHamcrest/pull/422)), the signature of the +`hasItem` and `hasItems` methods has changed. Code relying on the exact signature of +these methods will need to be updated. The following methods are affected: + * `org.hamcrest.CoreMatchers.hasItem` + * `org.hamcrest.CoreMatchers.hasItems` + * `org.hamcrest.Matchers.hasItem` + * `org.hamcrest.Matchers.hasItems` + * `org.hamcrest.core.IsCollectionContaining.hasItem` + * `org.hamcrest.core.IsCollectionContaining.hasItems` + * `org.hamcrest.core.IsIterableContaining.hasItem` + * `org.hamcrest.core.IsIterableContaining.hasItems` + * TODO: decide if these breaking changes should trigger a major version upgrade (i.e v4.0) + ### Improvements * Javadoc improvements and cleanup ([PR #420](https://github.com/hamcrest/JavaHamcrest/pull/420)) @@ -10,7 +26,8 @@ ### Bugfixes -* TBD +* Allow matching against polymorphic collections ([#252](https://github.com/hamcrest/JavaHamcrest/issues/252), + [PR #422](https://github.com/hamcrest/JavaHamcrest/pull/422)) ## Version 3.0 (1st August 2024) diff --git a/hamcrest/src/main/java/org/hamcrest/CoreMatchers.java b/hamcrest/src/main/java/org/hamcrest/CoreMatchers.java index f2a66cc8..6dadcc52 100644 --- a/hamcrest/src/main/java/org/hamcrest/CoreMatchers.java +++ b/hamcrest/src/main/java/org/hamcrest/CoreMatchers.java @@ -231,7 +231,7 @@ public static org.hamcrest.Matcher anything(java.lang.String d * the matcher to apply to items provided by the examined {@link Iterable} * @return The matcher. */ - public static org.hamcrest.Matcher> hasItem(org.hamcrest.Matcher itemMatcher) { + public static org.hamcrest.Matcher> hasItem(org.hamcrest.Matcher itemMatcher) { return IsIterableContaining.hasItem(itemMatcher); } @@ -249,7 +249,7 @@ public static org.hamcrest.Matcher> hasItem(or * the item to compare against the items provided by the examined {@link Iterable} * @return The matcher. */ - public static org.hamcrest.Matcher> hasItem(T item) { + public static org.hamcrest.Matcher> hasItem(T item) { return IsIterableContaining.hasItem(item); } @@ -268,7 +268,7 @@ public static org.hamcrest.Matcher> hasItem(T * @return The matcher. */ @SafeVarargs - public static org.hamcrest.Matcher> hasItems(org.hamcrest.Matcher... itemMatchers) { + public static org.hamcrest.Matcher> hasItems(org.hamcrest.Matcher... itemMatchers) { return IsIterableContaining.hasItems(itemMatchers); } @@ -287,7 +287,7 @@ public static org.hamcrest.Matcher> hasItems(org.hamcr * @return The matcher. */ @SafeVarargs - public static org.hamcrest.Matcher> hasItems(T... items) { + public static org.hamcrest.Matcher> hasItems(T... items) { return IsIterableContaining.hasItems(items); } diff --git a/hamcrest/src/main/java/org/hamcrest/Matchers.java b/hamcrest/src/main/java/org/hamcrest/Matchers.java index 9768333a..d5260fb1 100644 --- a/hamcrest/src/main/java/org/hamcrest/Matchers.java +++ b/hamcrest/src/main/java/org/hamcrest/Matchers.java @@ -449,7 +449,7 @@ public static org.hamcrest.Matcher anything(java.lang.String d * the matcher to apply to items provided by the examined {@link Iterable} * @return The matcher. */ - public static org.hamcrest.Matcher> hasItem(org.hamcrest.Matcher itemMatcher) { + public static org.hamcrest.Matcher> hasItem(org.hamcrest.Matcher itemMatcher) { return IsIterableContaining.hasItem(itemMatcher); } @@ -467,7 +467,7 @@ public static org.hamcrest.Matcher> hasItem(or * the item to compare against the items provided by the examined {@link Iterable} * @return The matcher. */ - public static org.hamcrest.Matcher> hasItem(T item) { + public static org.hamcrest.Matcher> hasItem(T item) { return IsIterableContaining.hasItem(item); } @@ -486,7 +486,7 @@ public static org.hamcrest.Matcher> hasItem(T * @return The matcher. */ @SafeVarargs - public static org.hamcrest.Matcher> hasItems(org.hamcrest.Matcher... itemMatchers) { + public static org.hamcrest.Matcher> hasItems(org.hamcrest.Matcher... itemMatchers) { return IsIterableContaining.hasItems(itemMatchers); } @@ -505,7 +505,7 @@ public static org.hamcrest.Matcher> hasItems(org.hamcr * @return The matcher. */ @SafeVarargs - public static org.hamcrest.Matcher> hasItems(T... items) { + public static org.hamcrest.Matcher> hasItems(T... items) { return IsIterableContaining.hasItems(items); } diff --git a/hamcrest/src/main/java/org/hamcrest/collection/HasItemInArray.java b/hamcrest/src/main/java/org/hamcrest/collection/HasItemInArray.java index 102565b7..6e96ae21 100644 --- a/hamcrest/src/main/java/org/hamcrest/collection/HasItemInArray.java +++ b/hamcrest/src/main/java/org/hamcrest/collection/HasItemInArray.java @@ -16,7 +16,7 @@ public class HasItemInArray extends TypeSafeMatcher { private final Matcher elementMatcher; - private final TypeSafeDiagnosingMatcher> collectionMatcher; + private final TypeSafeDiagnosingMatcher> collectionMatcher; /** * Constructor, best called from {@link ArrayMatching}. diff --git a/hamcrest/src/main/java/org/hamcrest/collection/IsIterableContainingInAnyOrder.java b/hamcrest/src/main/java/org/hamcrest/collection/IsIterableContainingInAnyOrder.java index 84c744d4..ad1b951d 100644 --- a/hamcrest/src/main/java/org/hamcrest/collection/IsIterableContainingInAnyOrder.java +++ b/hamcrest/src/main/java/org/hamcrest/collection/IsIterableContainingInAnyOrder.java @@ -114,7 +114,8 @@ private boolean isMatched(S item) { */ @SafeVarargs public static Matcher> containsInAnyOrder(Matcher... itemMatchers) { - return containsInAnyOrder((Collection) Arrays.asList(itemMatchers)); + List> itemMatchersList = Arrays.asList(itemMatchers); + return containsInAnyOrder(itemMatchersList); } /** diff --git a/hamcrest/src/main/java/org/hamcrest/core/IsCollectionContaining.java b/hamcrest/src/main/java/org/hamcrest/core/IsCollectionContaining.java index f3279bbd..112bb02d 100644 --- a/hamcrest/src/main/java/org/hamcrest/core/IsCollectionContaining.java +++ b/hamcrest/src/main/java/org/hamcrest/core/IsCollectionContaining.java @@ -9,7 +9,7 @@ * @deprecated As of release 2.1, replaced by {@link IsIterableContaining}. */ @Deprecated -public class IsCollectionContaining extends TypeSafeDiagnosingMatcher> { +public class IsCollectionContaining extends TypeSafeDiagnosingMatcher> { private final IsIterableContaining delegate; @@ -26,7 +26,7 @@ public IsCollectionContaining(Matcher elementMatcher) { } @Override - protected boolean matchesSafely(Iterable collection, Description mismatchDescription) { + protected boolean matchesSafely(Iterable collection, Description mismatchDescription) { return delegate.matchesSafely(collection, mismatchDescription); } @@ -51,7 +51,7 @@ public void describeTo(Description description) { * the matcher to apply to items provided by the examined {@link Iterable} * @return The matcher. */ - public static Matcher> hasItem(Matcher itemMatcher) { + public static Matcher> hasItem(Matcher itemMatcher) { return IsIterableContaining.hasItem(itemMatcher); } @@ -70,7 +70,7 @@ public static Matcher> hasItem(Matcher itemMa * the item to compare against the items provided by the examined {@link Iterable} * @return The matcher. */ - public static Matcher> hasItem(T item) { + public static Matcher> hasItem(T item) { // Doesn't forward to hasItem() method so compiler can sort out generics. return IsIterableContaining.hasItem(item); } @@ -91,7 +91,7 @@ public static Matcher> hasItem(T item) { * @return The matcher. */ @SafeVarargs - public static Matcher> hasItems(Matcher... itemMatchers) { + public static Matcher> hasItems(Matcher... itemMatchers) { return IsIterableContaining.hasItems(itemMatchers); } @@ -111,7 +111,7 @@ public static Matcher> hasItems(Matcher... itemMatche * @return The matcher. */ @SafeVarargs - public static Matcher> hasItems(T... items) { + public static Matcher> hasItems(T... items) { return IsIterableContaining.hasItems(items); } diff --git a/hamcrest/src/main/java/org/hamcrest/core/IsIterableContaining.java b/hamcrest/src/main/java/org/hamcrest/core/IsIterableContaining.java index 070e3e95..67bb3e33 100644 --- a/hamcrest/src/main/java/org/hamcrest/core/IsIterableContaining.java +++ b/hamcrest/src/main/java/org/hamcrest/core/IsIterableContaining.java @@ -14,7 +14,7 @@ * Tests if an iterable contains matching elements. * @param the type of items in the iterable */ -public class IsIterableContaining extends TypeSafeDiagnosingMatcher> { +public class IsIterableContaining extends TypeSafeDiagnosingMatcher> { private final Matcher elementMatcher; @@ -31,7 +31,7 @@ public IsIterableContaining(Matcher elementMatcher) { } @Override - protected boolean matchesSafely(Iterable collection, Description mismatchDescription) { + protected boolean matchesSafely(Iterable collection, Description mismatchDescription) { if (isEmpty(collection)) { mismatchDescription.appendText("was empty"); return false; @@ -56,7 +56,7 @@ protected boolean matchesSafely(Iterable collection, Description mism return false; } - private boolean isEmpty(Iterable iterable) { + private boolean isEmpty(Iterable iterable) { return ! iterable.iterator().hasNext(); } @@ -81,7 +81,7 @@ public void describeTo(Description description) { * the matcher to apply to items provided by the examined {@link Iterable} * @return The matcher. */ - public static Matcher> hasItem(Matcher itemMatcher) { + public static Matcher> hasItem(Matcher itemMatcher) { return new IsIterableContaining<>(itemMatcher); } @@ -99,7 +99,7 @@ public static Matcher> hasItem(Matcher itemMa * the item to compare against the items provided by the examined {@link Iterable} * @return The matcher. */ - public static Matcher> hasItem(T item) { + public static Matcher> hasItem(T item) { // Doesn't forward to hasItem() method so compiler can sort out generics. return new IsIterableContaining<>(equalTo(item)); } @@ -119,8 +119,8 @@ public static Matcher> hasItem(T item) { * @return The matcher. */ @SafeVarargs - public static Matcher> hasItems(Matcher... itemMatchers) { - List>> all = new ArrayList<>(itemMatchers.length); + public static Matcher> hasItems(Matcher... itemMatchers) { + List>> all = new ArrayList<>(itemMatchers.length); for (Matcher elementMatcher : itemMatchers) { // Doesn't forward to hasItem() method so compiler can sort out generics. @@ -145,8 +145,8 @@ public static Matcher> hasItems(Matcher... itemMatche * @return The matcher. */ @SafeVarargs - public static Matcher> hasItems(T... items) { - List>> all = new ArrayList<>(items.length); + public static Matcher> hasItems(T... items) { + List>> all = new ArrayList<>(items.length); for (T item : items) { all.add(hasItem(item)); } diff --git a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java index 9f842585..762b5388 100644 --- a/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java +++ b/hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.TreeMap; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsMapContaining.hasEntry; import static org.hamcrest.core.IsAnything.anything; import static org.hamcrest.core.IsEqual.equalTo; @@ -47,4 +48,11 @@ public void testHasReadableDescription() { assertDescription("map containing [\"a\"-><2>]", hasEntry(equalTo("a"), (equalTo(2)))); } + public void testTypeVariance() { + Map m = new HashMap<>(); + Integer foo = 6; + m.put("foo", foo); + assertThat(m, hasEntry("foo", foo)); + } + } diff --git a/hamcrest/src/test/java/org/hamcrest/core/IsCollectionContainingTest.java b/hamcrest/src/test/java/org/hamcrest/core/IsCollectionContainingTest.java index f1540198..4f32ca37 100644 --- a/hamcrest/src/test/java/org/hamcrest/core/IsCollectionContainingTest.java +++ b/hamcrest/src/test/java/org/hamcrest/core/IsCollectionContainingTest.java @@ -23,17 +23,17 @@ protected Matcher createMatcher() { } public void testMatchesACollectionThatContainsAnElementMatchingTheGivenMatcher() { - Matcher> itemMatcher = hasItem(equalTo("a")); + Matcher> itemMatcher = hasItem(equalTo("a")); assertMatches("should match list that contains 'a'", itemMatcher, asList("a", "b", "c")); } public void testDoesNotMatchCollectionThatDoesntContainAnElementMatchingTheGivenMatcher() { - final Matcher> matcher1 = hasItem(mismatchable("a")); + final Matcher> matcher1 = hasItem(mismatchable("a")); assertMismatchDescription("mismatches were: [mismatched: b, mismatched: c]", matcher1, asList("b", "c")); - final Matcher> matcher2 = hasItem(equalTo("a")); + final Matcher> matcher2 = hasItem(equalTo("a")); assertMismatchDescription("was empty", matcher2, new ArrayList()); } @@ -55,27 +55,27 @@ public void testCanMatchItemWhenCollectionHoldsSuperclass() // Issue 24 @SuppressWarnings("unchecked") public void testMatchesAllItemsInCollection() { - final Matcher> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("should match list containing all items", matcher1, asList("a", "b", "c")); - final Matcher> matcher2 = hasItems("a", "b", "c"); + final Matcher> matcher2 = hasItems("a", "b", "c"); assertMatches("should match list containing all items (without matchers)", matcher2, asList("a", "b", "c")); - final Matcher> matcher3 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher3 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("should match list containing all items in any order", matcher3, asList("c", "b", "a")); - final Matcher> matcher4 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher4 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("should match list containing all items plus others", matcher4, asList("e", "c", "b", "a", "d")); - final Matcher> matcher5 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher5 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertDoesNotMatch("should not match list unless it contains all items", matcher5, asList("e", "c", "b", "d")); // 'a' missing diff --git a/hamcrest/src/test/java/org/hamcrest/core/IsIterableContainingTest.java b/hamcrest/src/test/java/org/hamcrest/core/IsIterableContainingTest.java index b20942be..2f5eb5b3 100644 --- a/hamcrest/src/test/java/org/hamcrest/core/IsIterableContainingTest.java +++ b/hamcrest/src/test/java/org/hamcrest/core/IsIterableContainingTest.java @@ -6,14 +6,17 @@ import org.junit.Test; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.Set; import static java.util.Arrays.asList; +import static java.util.Collections.singleton; import static org.hamcrest.AbstractMatcherTest.*; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.core.IsIterableContaining.hasItem; import static org.hamcrest.core.IsIterableContaining.hasItems; -import static org.hamcrest.core.IsEqual.equalTo; public final class IsIterableContainingTest { @@ -27,14 +30,14 @@ public final class IsIterableContainingTest { @Test public void matchesACollectionThatContainsAnElementForTheGivenMatcher() { - final Matcher> itemMatcher = hasItem(equalTo("a")); + final Matcher> itemMatcher = hasItem(equalTo("a")); assertMatches("list containing 'a'", itemMatcher, asList("a", "b", "c")); } @Test public void doesNotMatchCollectionWithoutAnElementForGivenMatcher() { - final Matcher> matcher = hasItem(mismatchable("a")); + final Matcher> matcher = hasItem(mismatchable("a")); assertMismatchDescription("mismatches were: [mismatched: b, mismatched: c]", matcher, asList("b", "c")); assertMismatchDescription("was empty", matcher, new ArrayList()); @@ -62,31 +65,31 @@ public final class IsIterableContainingTest { @SuppressWarnings("unchecked") @Test public void matchesMultipleItemsInCollection() { - final Matcher> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("list containing all items", matcher1, asList("a", "b", "c")); - final Matcher> matcher2 = hasItems("a", "b", "c"); + final Matcher> matcher2 = hasItems("a", "b", "c"); assertMatches("list containing all items (without matchers)", matcher2, asList("a", "b", "c")); - final Matcher> matcher3 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher3 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("list containing all items in any order", matcher3, asList("c", "b", "a")); - final Matcher> matcher4 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher4 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertMatches("list containing all items plus others", matcher4, asList("e", "c", "b", "a", "d")); - final Matcher> matcher5 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); + final Matcher> matcher5 = hasItems(equalTo("a"), equalTo("b"), equalTo("c")); assertDoesNotMatch("not match list unless it contains all items", matcher5, asList("e", "c", "b", "d")); // 'a' missing } @Test public void reportsMismatchWithAReadableDescriptionForMultipleItems() { - final Matcher> matcher = hasItems(3, 4); + final Matcher> matcher = hasItems(3, 4); assertMismatchDescription("a collection containing <4> mismatches were: [was <1>, was <2>, was <3>]", matcher, asList(1, 2, 3)); } - private static Matcher mismatchable(final String string) { + private static Matcher mismatchable(final String string) { return new TypeSafeDiagnosingMatcher() { @Override protected boolean matchesSafely(String item, Description mismatchDescription) { @@ -104,4 +107,67 @@ public void describeTo(Description description) { }; } + @Test public void + matchesPolymorphicTypes() { + Collection dogs = singleton(new Dog("Spot")); + Animal spotAsAnimal = new Dog("Spot"); + assertMatches(hasItem(spotAsAnimal), dogs); + Dog spotAsDog = new Dog("Spot"); + assertMatches(hasItem(spotAsDog), dogs); + + Collection animals = asList( + new Dog("Fido"), new Cat("Whiskers")); + Dog fido = new Dog("Fido"); + Matcher> dogsMatcher = hasItem(fido); + assertMatches(dogsMatcher, animals); + + Cat whiskers = new Cat("Whiskers"); + assertMatches(hasItem(whiskers), animals); + + Matcher> iterableMatcher = hasItems(fido, whiskers); + assertMatches(iterableMatcher, animals); + assertMatches(not(hasItem(spotAsAnimal)), animals); + } + + abstract static class Animal { + private final String name; + + Animal(String name) { + this.name = name; + } + + public String name() { + return this.name; + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + + if (getClass() != obj.getClass()) { + return false; + } + + return name.equals(((Animal) obj).name); + } + + @Override + public int hashCode() { + return name.hashCode(); + } + } + + static class Dog extends Animal { + public Dog(String name) { + super(name); + } + } + + static class Cat extends Animal { + public Cat(String name) { + super(name); + } + } }