Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug mentioned in issue 201 —— bug of contains(contains(any class instance),empty()) #369

Open
wants to merge 1 commit into
base: v2.3-candidates
Choose a base branch
from

Conversation

HennyNile
Copy link

@HennyNile HennyNile commented Feb 13, 2022

Fixes #201
Hi! This description is somewhat verbose but sufficiently detailed. It would be my pleasure if you could read it. I am a student in class Software Engineering and I am trying to fix the bug mentioned in issue 201. After learning @brownian-motion and @tumbarumba's point of view and reading the source code, I found this bug is caused by the improper use of generics.
This bug is caused when we use contains(contains(<any class instance>),empty())) where empty() is org.hamcrest.Matchers.empty(). contains() has four override methods. Our expectation is that inter contains() will call method public static <E> org.hamcrest.Matcher<java.lang.Iterable<? extends E>> contains(E... items) with a calss instance as parameter, outer contains() will call method public static <E> org.hamcrest.Matcher<java.lang.Iterable<? extends E>> contains(org.hamcrest.Matcher<? super E>... itemMatcher) with two matchers as parameters and outer contains() should return an org.hamcrest.Matcher<java.lang.Iterable<? extends E>> which is used to match.
In fact, it is not what we expect. To call org.hamcrest.Matcher<java.lang.Iterable<? extends E>> contains(org.hamcrest.Matcher<? super E>... itemMatcher) parameters of outer contains() should all be Matcher<? super E> with same generic type E. Howerver, class of return variable of inner contains(<any class instance>) is Matcher<Iterable<? extends Integer>> and class of return variable of empty() is Matcher<Collection<?>>. In my IDE, it hints that type parameter E of outer contains() has incompatible upper bounds, which leads outer contains() call method public static <E> org.hamcrest.Matcher<java.lang.Iterable<? extends E>> contains(E... items) and embed parameter Matcher into another Matcher. There are two main ways to fix it without modifying resource code that is using contains(contains(<any class instance>),(Matcher)empty())) or contains(contains(<any class instance>),Matcher.<Class(corresponding to class instance)>empty())). Obviously, it's ugly and unamiable for beginner.
Based on all the things above, I fix this bug just by changing the return type of org.hamcrest.Matchers.empty() as org.hamcrest.Matcher and the initial return type is org.hamcrest.Matcher<Collection<? extends E>>. This won't introduce new bug because empty() is a static method in org.hamcrest.Matchers and has no parameter, which make empty() won't hold a type parameter without explicit declaration such as contains(contains(<any class instance>),Matcher.<Class(corresponding to class instance)>empty())). What's more , empty() just return a new InEmptyCollection() without type parameter in essence. In conclusion, I believe this modification can fix this bug and won't introduce new bug.

@nhojpatrick nhojpatrick self-assigned this Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
v2.3
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants