java.util.Optional matchers

[seregamorph]

This change was waiting for releasing 3.0 with JDK 8 compatibility.

Matchers for java.util.Optional:

• OptionalMatchers.emptyOptional()
• OptionalMatchers.optionalWithValue()
• OptionalMatchers.optionalWithValue(value)
• OptionalMatchers.optionalWithValue(Matcher)

[tumbarumba]

Thanks for this, @seregamorph! I appreciate the time you've put into this PR. The code is clean and readable, and I think it will be a valuable addition to Hamcrest. If you don't mind, can I request some changes?

Firstly, I'm not keen on the use of anonymous inner classes for published matchers in Hamcrest. It would be great if you re-write them as top level public classes that subclass the same TypeSafeDiagnosingMatcher<Optional<T>> parent

Secondly, I'm not definite about this, but I think it would be nicer to put them in a separate sub-package (e.g. optional)

Thirdly (and finally), I think static factory method names could give a bit more context, to make them more readable when they've been staticly imported. e.g. change isEmpty() to emptyOptional(), and change isPresent() to optionalWithValue().

Putting those points together, we'd end up with something like this:

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.optional.OptionalMatchers.emptyOptional;
import static org.hamcrest.optional.OptionalMatchers.optionalWithValue;
import static org.hamcrest.text.MatchesPattern.matchesPattern;
import org.junit.Test;
import java.util.Optional;

public class OptionalMatchersTest {
    @Test
    public void testEmptyOptional() {
        Optional<Object> actual = Optional.empty();
        // assertThat(actual, isEmpty());
        assertThat(actual, is(emptyOptional()));
        assertThat(actual, not(optionalWithValue()));
    }

    @Test
    public void testOptionalWithValue() {
        Optional<String> actual = Optional.of("Hello, world");
        // assertThat(actual, isPresent());
        assertThat(actual, not(emptyOptional()));
        assertThat(actual, is(optionalWithValue()));
        assertThat(actual, optionalWithValue("Hello, world"));
        assertThat(actual, optionalWithValue(matchesPattern("Hell")));
    }
}

What do you think?

[seregamorph]

Thank you for the review and comments.

Done:

• re-write them as top level public classes
• put them in a separate sub-package
• factory method names could give a bit more context, to make them more readable when they've been staticly imported

Also please note: I adjusted a bit the description message from "to be empty"/"to be present"/"to be present and match" to "empty"/"present"/"present and matches" because of the notation you suggested with "is" wrapper.

These tests clarify the failure messages:

AssertionError failure = assertThrows(AssertionError.class, () -> {
    assertThat(Optional.of(1), is(emptyOptional()));
});
assertEquals("\n" +
        "Expected: is empty\n" +
        "     but: is Optional[1]", failure.getMessage());

or without is wrapper:

AssertionError failure = assertThrows(AssertionError.class, () -> {
    assertThat(Optional.of(1), emptyOptional());
});
assertEquals("\n" +
        "Expected: empty\n" +
        "     but: is Optional[1]", failure.getMessage());

[tumbarumba]

Thanks again @seregamorph. Now that I can see the changes in context, I have a bit more feedback:

• I don't think we need a separate class for OptionalWithMatchingValue. That functionality can be merged into OptionalWithValue
• I don't think the OptionalMatchers class is really pulling its weight, we could delete it without loss of clarity and readability in calling code. Rather, move the static factory methods into OptionalEmpty and OptionalWithValue, and add additional factory methods to org.hamcrest.Matchers.
• I think there should be a factory method for an option literal value (e.g. optionalWithValue("Hello, world") as per my example test above). In the implementation, you can just use the IsAnything.anything() and IsEqual.equalTo(T) methods to wrap the literal value and allow consistent handling with an matcher in OptionalWithValue.
• Note that there are javadoc warnings in the build. Run ./gradlew javadoc locally to see those.

[seregamorph]

Done

[tumbarumba]

Looks good, thanks!