Skip to content

Expose paramater type info where available to help with Java 7 issues.#26

Open
louisth wants to merge 1 commit intohamcrest:masterfrom
louisth:master
Open

Expose paramater type info where available to help with Java 7 issues.#26
louisth wants to merge 1 commit intohamcrest:masterfrom
louisth:master

Conversation

@louisth
Copy link
Copy Markdown

@louisth louisth commented Jan 18, 2013

Lots of matchers know what type of object they are dealing with, but there's currently no way to get at that information. This type information would be really helpful for some Hamcrest clients, such as JMock, in dealing with the new typing issues introduced in Java 7. (Specifically, with this fix JMock clients won't have to change their tests when JMock is made Java 7 compilable.) I propose adding a getParameterType method to Matcher that lets the matchers that know their type expose that information (and other Matchers can just return null). BaseMatcher provides the default implementation so clients do not have to make any code changes.

I'm bundling two other small fixes. AnyOf and AllOf now have the same base class, and the mismatch description for TypeSafeDiagnosingMatcher now matches TypeSafeMatcher. If this is a problem, I'd be happy to resubmit these separately.

@sf105
Copy link
Copy Markdown
Member

sf105 commented Jan 19, 2013

We probably need to do this at some point. My preference is to pull up the implementation from TypeSafeMatcher. I think the implementation might be better if the default value is Object.class rather than null

@louisth
Copy link
Copy Markdown
Author

louisth commented Jan 31, 2013

TypeSafeMatcher looks at the raw parameter type of one of the methods. This works when the derived class provides a method with the desired type explicitely coded (e.g., StringContainsInOrder), but doesn't work in all cases. For example, IsNot needs to delegate to the matcher it wraps. Even clearer, IsEqual works for any object type and so the matches method has an Object raw parameter type, but a better guess for the effective template parameter type (which JMock needs) is to look at the type of the expectedValue. (OrderingComparison is a similar example.) If we did change the base behavior, we could always overrride to get the needed behavior in these cases of course. I guess I just don't see much difference between pulling the raw-type-sniffing code from TypeSafeMatcher up to BaseMatcher and actually making TypeSafeMatcher the standard base class (pull everything up?). I'm not even sure that there are any classes that COULD benefit from TypeSafeMatcher that do not already extend it. I was trying to be good and went with a smaller change.

Why not return Object.class by default? Because the parameter type provided by this method is truly a guess, I thought it made more sense to return null to indicate that the matcher is choosing not to hazard a guess (either because it's not implemented (BaseMatcher) or because it has no information available (IsNull)) and return Object.class when the matcher really does think its parameter type is Object. I was trying to be more accurate in case some library besides JMock needed to use this feature in the future.

Also: Make AnyOf and AllOf have the same base class. Make the mismatch description for TypeSafeDiagnosingMatcher match TypeSafeMatcher.
@nhojpatrick
Copy link
Copy Markdown
Member

@louisth fancy rebasing this from master, as hamcrest-core and hamcrest-library have been refactored a lot and also deprecated, so that everything is just in hamcrest.

@nhojpatrick
Copy link
Copy Markdown
Member

Going to try and kick start hamcrest, so if you want to get it merged, please rebase from the branch v2.3-candidates.
Still trying to understand how has permissions to perform a release.

@nhojpatrick nhojpatrick force-pushed the master branch 2 times, most recently from 9bc653b to e9f7fc8 Compare February 13, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Awaiting rebase

Development

Successfully merging this pull request may close these issues.

3 participants