Request to change the generics definition of MatcherAssert.assertThat

[GoogleCodeExporter]

Hi Hamcrest.

I've been using Hamcrest for a long time (currently using 1.3.RC2) and in every 
single one of my tests, it's great. But MatcherAssert.assertThat keeps giving 
out compilation errors from time to time due to the way that the generic 
parameter is defined in that method. This causes me headache and I needed to 
find workarounds and therefore makes my code unclean.

Here is the signature of MatcherAssert.assertThat in 1.1:

    public static <T> void assertThat(T actual, Matcher<T> matcher)

Here is the improved signature of MatcherAssert.assertThat since 1.2:

    public static <T> void assertThat(T actual, Matcher<? super T> matcher)

The improvement made in 1.2 makes it more flexible, but it still won't solve 
the following problem (you get compilation error):

    CharSequence actual = "hello";
    String expected = "hello";
    assertThat(actual, is(expected));

That failed because "expected" (String) is a subtype of "actual" 
(CharSequence), which makes the generic requirement on the matcher parameter 
(which is <? super T>) to fail.

So here I'm suggesting a solution, change the generics definition of 
Matcher.assertThat to something like:

    public static <T, A extends T, E extends T> void assertThat2(A actual, Matcher<E> expected)

Using this with the previous example, you get T = CharSequence, A = 
CharSequence, E = String, and since both CharSequence and String are subtypes 
of CharSequence, your program will compile fine (just like the old 
assertEquals).

Thanks.

Original issue reported on code.google.com by llaec...@gmail.com on 11 Feb 2012 at 12:55

[GoogleCodeExporter]

Oops, place ignore the letter "2" in the suggested solution, it should be this 
instead: 

    public static <T, A extends T, E extends T> void assertThat(A actual, Matcher<E> expected)

Original comment by llaec...@gmail.com on 11 Feb 2012 at 12:56

[GoogleCodeExporter]

Have been talking to the JUnit guys as well 
https://github.com/KentBeck/junit/issues/378
And realized the solution could be made simpler:

    public static void assertThat(Object actual, Matcher<?> matcher)

Original comment by llaec...@gmail.com on 20 Feb 2012 at 10:03

[GoogleCodeExporter]

tag as Java

Original comment by t.denley on 12 May 2012 at 10:10

• Added labels: Java

[GoogleCodeExporter]

I fail to see how that is simpler or better. It completely removes type safety. 
The current implementation prevents me from doing something stupid like this:

assertThat("string", is(equalTo(String.class)));

Your proposed solution makes that legal.

Original comment by davidhar...@gmail.com on 2 Jun 2012 at 9:33

[GoogleCodeExporter]

Hi, :-)

 I've responded to your comment in the JUnit issue, so I just provide the link here instead of copying over: https://github.com/KentBeck/junit/issues/378#issuecomment-6081292

Original comment by llaec...@gmail.com on 2 Jun 2012 at 10:44

[GoogleCodeExporter]

I often encounter this problem with hamcrest's assertThat method, too.  I don't 
really understand how it helps to have:

public static <T> void assertThat(T actual, Matcher<? super T> expected)

As I see it, the likely usage scenario is this:

{code}
public CharSequence someValue(){
  return "hello"
}

assertThat(someValue(), is("hello"));
{code}

I would have thought this would need:

public static <T> void assertThat(T actual, Matcher<? extends T> expected)

In fact, I frequently add a utility method to my projects to support exactly 
that.

It is semantically correct for a return value to be a subclass of the method 
signature.  It is not correct for it to be a superclass.  Am I missing 
something here?

I'd really kind of like to see this fixed.  Alternatively, I'd be keen to 
understand the motivation for the <? super T> that is in 1.3.

Original comment by akn...@gmail.com on 4 Sep 2012 at 2:37

[GoogleCodeExporter]

OK, scratch that last comment.  I've worked it out. This example might be 
useful to others.  The crux is the choice between no type safety for:
assertThatExpectedExtendsActual(new Circle(), isShape(new Square()));

or type safety with one a choice of clunky syntax:
this.<Circle>assertThatExpectedExtendsActual(new Circle(), isShape(new 
Square()));
or
Shape circle = new Circle();
assertThatExpectedSuperActual(someShape(), isShape(circle));

FWIW, I agree with the choice - the <> stuff is uglier.  :-)

public class AssertUtil {
    public static <T> void assertThatExpectedExtendsActual(T actual, Matcher<? extends T> expected) {
        MatcherAssert.assertThat(actual, (Matcher<T>) expected);
    }

    public static <T> void assertThatExpectedSuperActual(T actual, Matcher<? super T> expected) {
        MatcherAssert.assertThat(actual, expected);
    }

    @Test
    public void test() {
        assertThatExpectedSuperActual(someShape(), isShape(new Circle()));  // does not compile
        Shape circle = new Circle();
        assertThatExpectedSuperActual(someShape(), isShape(circle));
        assertThatExpectedSuperActual(new Circle(), isShape(someShape()));
        assertThatExpectedExtendsActual(new Circle(), isShape(someShape()));
        assertThatExpectedExtendsActual(new Circle(), isShape(new Square()));
        this.<Circle>assertThatExpectedExtendsActual(new Circle(), isShape(new Square()));  // does not compile
        assertThatExpectedExtendsActual(someShape(), isShape(new Circle()));
        MatcherAssert.assertThat(new Circle(), isShape(someShape()));
        MatcherAssert.assertThat(someShape(), isShape(new Circle())); // does not compile

        assertThatExpectedExtendsActual(someCharSequence(), is("bob"));
        CharSequence bobAsCharSequence = "bob";
        assertThatExpectedSuperActual(someCharSequence(), is(bobAsCharSequence));
        MatcherAssert.assertThat(someCharSequence(), is("bob")); // does not compile
        MatcherAssert.assertThat(someCharSequence(), is(bobAsCharSequence));

    }

    private Shape someShape() {
        return new Square();
    }

    private CharSequence someCharSequence() {
        return "bob";
    }

    private static <T extends Shape> ShapeMatcher<T> isShape(T expected) {
        return new ShapeMatcher(expected);
    }

    private static class ShapeMatcher<T extends Shape> extends BaseMatcher<T> {

        private T expected;

        public ShapeMatcher(T expected) {
            this.expected = expected;
        }

        @Override
        public boolean matches(Object o) {
            return o.equals(expected);
        }

        @Override
        public void describeTo(Description description) {
            //To change body of implemented methods use File | Settings | File Templates.
        }
    }

    private static interface Shape{}

    private static class Square implements Shape{}

    private static class Circle implements Shape{}
}

Original comment by akn...@gmail.com on 4 Sep 2012 at 4:06