unruly / java-8-matchers

Hamcrest Matchers for Java 8 features
MIT License
23 stars 5 forks source link

We should support building matchers with lambdas #9

Closed CliveEvans closed 7 years ago

CliveEvans commented 8 years ago

This code works, for example, but needs a describeTo that isn't rubbish ...

@Test
public void shouldBeAbleToBuildAMatcherUsingALambda() throws Exception {
    List<String> strings = Arrays.asList("one", "two");

    assertThat(strings, with(l -> l.get(0), equalTo("one")));
}

private <U,V> Matcher<U> with(Function<U,V> transform, Matcher<V> matcher){
    return new TypeSafeMatcher<U>() {
        @Override 
        protected boolean matchesSafely(U item) {
            return matcher.matches(transform.apply(item));
        }

        @Override 
        public void describeTo(Description description) {
            description.appendText("a Object that matches")
                    .appendDescriptionOf(matcher)
                    .appendText("after being transformed");
        }
    };
}
runeflobakk commented 7 years ago

I have made one of these, with pretty nice description and mismatch-description, which I would be glad to contribute if your are interested. It has been used for a quite a while on a project I am part of for a client.

For constructing descriptions, it uses TypeTools to resolve the generic types of the lambda. You can also pass a more semantically meaningful text if the types alone does not themselves provide sufficient information in the case of assertion failures.

I can make a pull request for it, but I first wanted to ask how you would feel about adding a dependency on TypeTools?

Also I would imagine the static methods for this to be placed in a new class, what would you like to name it? As they are not bound to any concepts as the existing OptionalMatchers or StreamMatchers, I would think some kind of generically named class would be appropriate. BaseMatchers? LambdaMatchers? Java8Matchers? I think myself I would prefer Java8Matchers.

CliveEvans commented 7 years ago

I'm torn. It sounds awesome, but one of the things I like about this library is just how small and self-contained it is. I worry (probably needlessly) that if we start bringing in external dependencies, we'll end up bringing in the kitchen sink.

Or guava.

I'm probably worrying needlessly. @bruceeddy what do you think?

runeflobakk commented 7 years ago

I know the feeling, trust me. I am too become very restrictive with bringing in dependencies into libraries.

I can of course investigate how difficult it would be to do the reflection myself to resolve the type parameters. It's just that doing reflection on generics is so damn difficult 😓

CliveEvans commented 7 years ago

I've looked at the source of Type Tools now. It's tiny. I've also been doing a certain amount of reflection this last fortnight.

What I've learned from this is that it's not that hard. Also that there are loads of stupid edge cases, and testing is a pain. If somebody else has done the work, and it's fully self-contained, then it makes sense not to duplicate the effort.

My vote is we go with it. @bruceeddy gets final say, it's @unruly-oss code, and I don't work for them any more. (I'll accept pull requests that don't set precedent, but this isn't one of those)

runeflobakk commented 7 years ago

I started looking at resolving the type arguments myself without TypeTools, and started digging with what's available in the JDK reflection API. It is quite... tedious. And a little bit hacky as well, as I found out, since I will not be able to get the ParameterizedType for the lambda through standard means. After some googling I found this, which explains it http://stackoverflow.com/a/25612775. The author of TypeTools also explains in another post in the same thread that he has implemented this in TypeTools.

I have done some reflection on generics earlier, and as you say, there are quite a few corner cases to consider to get it right. Or, I would actually not even view them as corner-cases, but the actual space of cases to handle is often larger than one would expect, considering multiple levels of inheritance, wildcards, etc.

I will just make a pull-request for the matchers with the TypeTools dependency, and we can discuss over the code :)

benjiman commented 7 years ago

What information do you want to reflect off the lambda? Since we can modify the type of the lambda (as it's in our code) we can make our lives considerably easier by making the lambda serializable, then we can reflect on SerializedLambda to get type information

java.lang.AssertionError: 
Expected: a List that matches "fail" after being transformed by List -> String
     but: was <[one, two]>
Expected :a List that matches "fail" after being transformed by List -> String

If compiled with -parameters you could additionally get the lambda parameter names.

package co.unruly.matchers;

import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Test;

import java.io.Serializable;
import java.lang.invoke.SerializedLambda;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;

import static java.util.Arrays.asList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;

public class LambdaMatchersTest {

    @Test
    public void shouldBeAbleToBuildAMatcherUsingALambda() throws Exception {
        List<String> strings = asList("one", "two");

        assertThat(strings, with(l -> l.get(0), equalTo("fail")));
    }

    private <U, V> Matcher<U> with(DescribableFunction<U, V> transform, Matcher<V> matcher) {
        return new TypeSafeMatcher<U>() {
            @Override
            protected boolean matchesSafely(U item) {
                return matcher.matches(transform.apply(item));
            }

            @Override
            public void describeTo(Description description) {
                description.appendText("a " + transform.parameter(0).getType().getSimpleName() + " that matches ")
                        .appendDescriptionOf(matcher)
                        .appendText(" after being transformed by " + transform.parameter(0).getType().getSimpleName() + " -> " + transform.returnType().getSimpleName());
            }
        };

    }
}

interface DescribableFunction<T, R> extends Function<T, R>, MethodFinder {}

interface MethodFinder extends Serializable {
    default SerializedLambda serialized() {
        try {
            Method replaceMethod = getClass().getDeclaredMethod("writeReplace");
            replaceMethod.setAccessible(true);
            return (SerializedLambda) replaceMethod.invoke(this);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

    default Class<?> getContainingClass() {
        try {
            String className = serialized().getImplClass().replaceAll("/", ".");
            return Class.forName(className);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

    default Method method() {
        SerializedLambda lambda = serialized();
        Class<?> containingClass = getContainingClass();
        return asList(containingClass.getDeclaredMethods())
                .stream()
                .filter(method -> Objects.equals(method.getName(), lambda.getImplMethodName()))
                .findFirst()
                .orElseThrow(UnableToGuessMethodException::new);
    }

    default Parameter parameter(int n) {
        return method().getParameters()[n];
    }

    default Class<?> returnType() {
        return method().getReturnType();
    }

    class UnableToGuessMethodException extends RuntimeException {}
}
runeflobakk commented 7 years ago

Great! That indeed looks like exactly what I need! I didn't even know about SerializedLambda. Thanks for bringing it to my attention!

I will incorporate @benjiman's code into my Matcher-implementation, and make a pull request.

bruceeddy commented 7 years ago

Thanks for that @runeflobakk . I'll review and merge.

swerfel commented 5 years ago

Suggestion: Add some example text to main README.md, so people can see that great feature faster.

runeflobakk commented 5 years ago

Thank you @swerfel for the kind words! I have made a pull-request with some documentation. Feel free to suggest any changes you would like to have in the readme, if any: https://github.com/unruly/java-8-matchers/pull/17