How do I suggest that the user wrap this parameter in a method call?

62 views Asked by At

A common problem we find in code review is people writing this:

    assertThat(thing, nullValue());

instead of this:

    assertThat(thing, is(nullValue()));

In order to catch it sooner, I thought I'd try writing a custom error-prone check. This is a poorly documented area though so I've been doing so by digging inside GitHub for working examples.

I have so far:


@AutoService(BugChecker.class)
@BugPattern(
    name = "AssertThatThingNullValue",
    summary = "`assertThat(thing, nullValue())` doesn't sound like English, wrap `nullValue` in `is`"
    severity = WARNING)
public class AssertThatThingNullValue extends BugChecker implements MethodInvocationTreeMatcher
{
    private static final Matcher<ExpressionTree> ASSERT_THAT = staticMethod()
        .onClassAny("org.hamcrest.MatcherAssert", "org.junit.Assert")
        .named("assertThat");

    private static final Matcher<ExpressionTree> NULL_VALUE = staticMethod()
        .onClass("org.hamcrest.Matchers")
        .named("nullValue");

    private static final Matcher<ExpressionTree> NULL_VALUE_INVOCATION =
        methodInvocation(NULL_VALUE);

    private static final Matcher<ExpressionTree> ASSSERT_THAT_THING_NULL_VALUE =
        methodInvocation(ASSERT_THAT, MatchType.LAST, NULL_VALUE_INVOCATION);

    @Override
    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state)
    {
        if (ASSSERT_THAT_THING_NULL_VALUE.matches(tree))
        {
            buildDescription(tree)
                .addFix(SuggestedFixes.somethingGoesHere(...))
                .build();
        }

        return Description.NO_MATCH;
    }
}

My problem is I can't figure out how to build the suggested fix from the available methods in SuggestedFixes. I'm wondering whether this API just isn't fleshed out well or whether I'm just going down the wrong track entirely and should be writing the check in a better way?

0

There are 0 answers