Detect invalid recursive matchers

[cushon]

Original issue created by mdempsky@google.com on 2013-09-23 at 07:53 PM

---

In CompoundAssignment.matches(), this return statement is erroneous:

return receiverMatcher.matches(compoundAssignmentTree.getVariable(), state)
    && expressionMatcher.matches(compoundAssignmentTree.getExpression(), state);

because the VisitorState passed to the submatchers will have the wrong TreePath (i.e., it will still have a TreePath for the compoundAssignmentTree expression instead of the appropriate subexpression).

It should actually be something like:

return submatch(receiverMatcher, compoundAssignmentTree.getVariable(), state)
    && submatch(expressionMatcher, compoundAssignmentTree.getExpression(), state);

where:

private <T> submatch(Matcher<T> matcher, T tree, VisitorState state) {
  return matcher.matches(tree, state.withPath(new TreePath(state.getPath(), tree)));
}

Looking through Matchers, a lot of other helpers are similarly buggy (methodHasParameters, hasMethod, variableType).

So probably:

1. There should be helpers to make writing the right code easier
2. There should be an error-prone checker to detect this bad checking logic. ;)

[cushon]

Original comment posted by mdempsky@google.com on 2013-10-01 at 01:01 AM

---

I put together https://github.com/cushon/issues-import/commits/mdempsky which has two commits:

1. Adding two helper methods
2. Converting a bunch of error_prone_core to use them.

I think there's still a lot to go (e.g., the classes in the bugpatterns package), but I wanted to at least get these out for review to see if y'all agree with the direction I'm going here.

[cushon]

Original comment posted by eaftan@google.com on 2013-10-01 at 04:41 PM

---

Nice catch! You're totally right that the VisitorState will be incorrect.

Let me look at your branch and post comments there.