Skip to content

Commit

Permalink
Allow matching against polymorphic collections (hamcrest#422)
Browse files Browse the repository at this point in the history 
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 (hamcrest#107)

Closes hamcrest#252
  • Loading branch information
@tumbarumba
tumbarumba authored Sep 22, 2024
1 parent d11ad94 commit 242604a
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 44 deletions.
19 changes: 18 additions & 1 deletion CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions hamcrest/src/main/java/org/hamcrest/CoreMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public static org.hamcrest.Matcher<java.lang.Object> anything(java.lang.String d
* the matcher to apply to items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(org.hamcrest.Matcher<? super T> itemMatcher) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItem(org.hamcrest.Matcher<? super T> itemMatcher) {
return IsIterableContaining.hasItem(itemMatcher);
}

Expand All @@ -249,7 +249,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(or
* the item to compare against the items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(T item) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItem(T item) {
return IsIterableContaining.hasItem(item);
}

Expand All @@ -268,7 +268,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(T
* @return The matcher.
*/
@SafeVarargs
public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(org.hamcrest.Matcher<? super T>... itemMatchers) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItems(org.hamcrest.Matcher<? super T>... itemMatchers) {
return IsIterableContaining.hasItems(itemMatchers);
}

Expand All @@ -287,7 +287,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(org.hamcr
* @return The matcher.
*/
@SafeVarargs
public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(T... items) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItems(T... items) {
return IsIterableContaining.hasItems(items);
}

Expand Down
8 changes: 4 additions & 4 deletions hamcrest/src/main/java/org/hamcrest/Matchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ public static org.hamcrest.Matcher<java.lang.Object> anything(java.lang.String d
* the matcher to apply to items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(org.hamcrest.Matcher<? super T> itemMatcher) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItem(org.hamcrest.Matcher<? super T> itemMatcher) {
return IsIterableContaining.hasItem(itemMatcher);
}

Expand All @@ -467,7 +467,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(or
* the item to compare against the items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(T item) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItem(T item) {
return IsIterableContaining.hasItem(item);
}

Expand All @@ -486,7 +486,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<? super T>> hasItem(T
* @return The matcher.
*/
@SafeVarargs
public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(org.hamcrest.Matcher<? super T>... itemMatchers) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItems(org.hamcrest.Matcher<? super T>... itemMatchers) {
return IsIterableContaining.hasItems(itemMatchers);
}

Expand All @@ -505,7 +505,7 @@ public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(org.hamcr
* @return The matcher.
*/
@SafeVarargs
public static <T> org.hamcrest.Matcher<java.lang.Iterable<T>> hasItems(T... items) {
public static <T> org.hamcrest.Matcher<java.lang.Iterable<? extends T>> hasItems(T... items) {
return IsIterableContaining.hasItems(items);
}

Expand Down
2 changes: 1 addition & 1 deletion hamcrest/src/main/java/org/hamcrest/collection/HasItemInArray.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
public class HasItemInArray<T> extends TypeSafeMatcher<T[]> {

private final Matcher<? super T> elementMatcher;
private final TypeSafeDiagnosingMatcher<Iterable<? super T>> collectionMatcher;
private final TypeSafeDiagnosingMatcher<Iterable<? extends T>> collectionMatcher;

/**
* Constructor, best called from {@link ArrayMatching}.
Expand Down
3 changes: 2 additions & 1 deletion hamcrest/src/main/java/org/hamcrest/collection/IsIterableContainingInAnyOrder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ private boolean isMatched(S item) {
*/
@SafeVarargs
public static <T> Matcher<Iterable<? extends T>> containsInAnyOrder(Matcher<? super T>... itemMatchers) {
return containsInAnyOrder((Collection) Arrays.asList(itemMatchers));
List<Matcher<? super T>> itemMatchersList = Arrays.asList(itemMatchers);
return containsInAnyOrder(itemMatchersList);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions hamcrest/src/main/java/org/hamcrest/core/IsCollectionContaining.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @deprecated As of release 2.1, replaced by {@link IsIterableContaining}.
*/
@Deprecated
public class IsCollectionContaining<T> extends TypeSafeDiagnosingMatcher<Iterable<? super T>> {
public class IsCollectionContaining<T> extends TypeSafeDiagnosingMatcher<Iterable<? extends T>> {

private final IsIterableContaining<T> delegate;

Expand All @@ -26,7 +26,7 @@ public IsCollectionContaining(Matcher<? super T> elementMatcher) {
}

@Override
protected boolean matchesSafely(Iterable<? super T> collection, Description mismatchDescription) {
protected boolean matchesSafely(Iterable<? extends T> collection, Description mismatchDescription) {
return delegate.matchesSafely(collection, mismatchDescription);
}

Expand All @@ -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 <T> Matcher<Iterable<? super T>> hasItem(Matcher<? super T> itemMatcher) {
public static <T> Matcher<Iterable<? extends T>> hasItem(Matcher<? super T> itemMatcher) {
return IsIterableContaining.hasItem(itemMatcher);
}

Expand All @@ -70,7 +70,7 @@ public static <T> Matcher<Iterable<? super T>> hasItem(Matcher<? super T> itemMa
* the item to compare against the items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> Matcher<Iterable<? super T>> hasItem(T item) {
public static <T> Matcher<Iterable<? extends T>> hasItem(T item) {
// Doesn't forward to hasItem() method so compiler can sort out generics.
return IsIterableContaining.hasItem(item);
}
Expand All @@ -91,7 +91,7 @@ public static <T> Matcher<Iterable<? super T>> hasItem(T item) {
* @return The matcher.
*/
@SafeVarargs
public static <T> Matcher<Iterable<T>> hasItems(Matcher<? super T>... itemMatchers) {
public static <T> Matcher<Iterable<? extends T>> hasItems(Matcher<? super T>... itemMatchers) {
return IsIterableContaining.hasItems(itemMatchers);
}

Expand All @@ -111,7 +111,7 @@ public static <T> Matcher<Iterable<T>> hasItems(Matcher<? super T>... itemMatche
* @return The matcher.
*/
@SafeVarargs
public static <T> Matcher<Iterable<T>> hasItems(T... items) {
public static <T> Matcher<Iterable<? extends T>> hasItems(T... items) {
return IsIterableContaining.hasItems(items);
}

Expand Down
18 changes: 9 additions & 9 deletions hamcrest/src/main/java/org/hamcrest/core/IsIterableContaining.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* Tests if an iterable contains matching elements.
* @param <T> the type of items in the iterable
*/
public class IsIterableContaining<T> extends TypeSafeDiagnosingMatcher<Iterable<? super T>> {
public class IsIterableContaining<T> extends TypeSafeDiagnosingMatcher<Iterable<? extends T>> {

private final Matcher<? super T> elementMatcher;

Expand All @@ -31,7 +31,7 @@ public IsIterableContaining(Matcher<? super T> elementMatcher) {
}

@Override
protected boolean matchesSafely(Iterable<? super T> collection, Description mismatchDescription) {
protected boolean matchesSafely(Iterable<? extends T> collection, Description mismatchDescription) {
if (isEmpty(collection)) {
mismatchDescription.appendText("was empty");
return false;
Expand All @@ -56,7 +56,7 @@ protected boolean matchesSafely(Iterable<? super T> collection, Description mism
return false;
}

private boolean isEmpty(Iterable<? super T> iterable) {
private boolean isEmpty(Iterable<? extends T> iterable) {
return ! iterable.iterator().hasNext();
}

Expand All @@ -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 <T> Matcher<Iterable<? super T>> hasItem(Matcher<? super T> itemMatcher) {
public static <T> Matcher<Iterable<? extends T>> hasItem(Matcher<? super T> itemMatcher) {
return new IsIterableContaining<>(itemMatcher);
}

Expand All @@ -99,7 +99,7 @@ public static <T> Matcher<Iterable<? super T>> hasItem(Matcher<? super T> itemMa
* the item to compare against the items provided by the examined {@link Iterable}
* @return The matcher.
*/
public static <T> Matcher<Iterable<? super T>> hasItem(T item) {
public static <T> Matcher<Iterable<? extends T>> hasItem(T item) {
// Doesn't forward to hasItem() method so compiler can sort out generics.
return new IsIterableContaining<>(equalTo(item));
}
Expand All @@ -119,8 +119,8 @@ public static <T> Matcher<Iterable<? super T>> hasItem(T item) {
* @return The matcher.
*/
@SafeVarargs
public static <T> Matcher<Iterable<T>> hasItems(Matcher<? super T>... itemMatchers) {
List<Matcher<? super Iterable<T>>> all = new ArrayList<>(itemMatchers.length);
public static <T> Matcher<Iterable<? extends T>> hasItems(Matcher<? super T>... itemMatchers) {
List<Matcher<? super Iterable<? extends T>>> all = new ArrayList<>(itemMatchers.length);

for (Matcher<? super T> elementMatcher : itemMatchers) {
// Doesn't forward to hasItem() method so compiler can sort out generics.
Expand All @@ -145,8 +145,8 @@ public static <T> Matcher<Iterable<T>> hasItems(Matcher<? super T>... itemMatche
* @return The matcher.
*/
@SafeVarargs
public static <T> Matcher<Iterable<T>> hasItems(T... items) {
List<Matcher<? super Iterable<T>>> all = new ArrayList<>(items.length);
public static <T> Matcher<Iterable<? extends T>> hasItems(T... items) {
List<Matcher<? super Iterable<? extends T>>> all = new ArrayList<>(items.length);
for (T item : items) {
all.add(hasItem(item));
}
Expand Down
8 changes: 8 additions & 0 deletions hamcrest/src/test/java/org/hamcrest/collection/IsMapContainingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,4 +48,11 @@ public void testHasReadableDescription() {
assertDescription("map containing [\"a\"-><2>]", hasEntry(equalTo("a"), (equalTo(2))));
}

public void testTypeVariance() {
Map<String, Number> m = new HashMap<>();
Integer foo = 6;
m.put("foo", foo);
assertThat(m, hasEntry("foo", foo));
}

}
16 changes: 8 additions & 8 deletions hamcrest/src/test/java/org/hamcrest/core/IsCollectionContainingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ protected Matcher<?> createMatcher() {
}

public void testMatchesACollectionThatContainsAnElementMatchingTheGivenMatcher() {
Matcher<Iterable<? super String>> itemMatcher = hasItem(equalTo("a"));
Matcher<Iterable<? extends String>> itemMatcher = hasItem(equalTo("a"));

assertMatches("should match list that contains 'a'",
itemMatcher, asList("a", "b", "c"));
}

public void testDoesNotMatchCollectionThatDoesntContainAnElementMatchingTheGivenMatcher() {
final Matcher<Iterable<? super String>> matcher1 = hasItem(mismatchable("a"));
final Matcher<Iterable<? extends String>> matcher1 = hasItem(mismatchable("a"));
assertMismatchDescription("mismatches were: [mismatched: b, mismatched: c]", matcher1, asList("b", "c"));

final Matcher<Iterable<? super String>> matcher2 = hasItem(equalTo("a"));
final Matcher<Iterable<? extends String>> matcher2 = hasItem(equalTo("a"));
assertMismatchDescription("was empty", matcher2, new ArrayList<String>());
}

Expand All @@ -55,27 +55,27 @@ public void testCanMatchItemWhenCollectionHoldsSuperclass() // Issue 24

@SuppressWarnings("unchecked")
public void testMatchesAllItemsInCollection() {
final Matcher<Iterable<String>> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c"));
final Matcher<Iterable<? extends String>> matcher1 = hasItems(equalTo("a"), equalTo("b"), equalTo("c"));
assertMatches("should match list containing all items",
matcher1,
asList("a", "b", "c"));

final Matcher<Iterable<String>> matcher2 = hasItems("a", "b", "c");
final Matcher<Iterable<? extends String>> matcher2 = hasItems("a", "b", "c");
assertMatches("should match list containing all items (without matchers)",
matcher2,
asList("a", "b", "c"));

final Matcher<Iterable<String>> matcher3 = hasItems(equalTo("a"), equalTo("b"), equalTo("c"));
final Matcher<Iterable<? extends String>> 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<Iterable<String>> matcher4 = hasItems(equalTo("a"), equalTo("b"), equalTo("c"));
final Matcher<Iterable<? extends String>> 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<Iterable<String>> matcher5 = hasItems(equalTo("a"), equalTo("b"), equalTo("c"));
final Matcher<Iterable<? extends String>> 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
Expand Down
Loading

0 comments on commit 242604a

Please sign in to comment.