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?