Skip to content

Commit

Permalink
Handle instanceof checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@carterkozak
carterkozak committed Nov 8, 2019
1 parent 99c8b96 commit 508121d
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 1 deletion.
31 changes: 30 additions & 1 deletion ...ror-prone/src/main/java/com/palantir/baseline/errorprone/InvocationHandlerDelegation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.matchers.method.MethodMatchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ConditionalExpressionTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IfTree;
import com.sun.source.tree.InstanceOfTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
Expand Down Expand Up @@ -73,6 +77,14 @@ public final class InvocationHandlerDelegation extends BugChecker implements Bug
private static final Matcher<Tree> CONTAINS_METHOD_INVOKE = Matchers.contains(
ExpressionTree.class, METHOD_INVOKE);

private static final Matcher<ExpressionTree> UNWRAP_THROWABLE = MethodMatchers.instanceMethod()
.onDescendantOf(Throwable.class.getName())
.named("getCause")
.withParameters();

private static final Matcher<Tree> CONTAINS_UNWRAP_THROWABLE =
Matchers.contains(ExpressionTree.class, UNWRAP_THROWABLE);

private static final Matcher<ExpressionTree> UNWRAP_ITE = MethodMatchers.instanceMethod()
.onDescendantOf(InvocationTargetException.class.getName())
// getTargetException is deprecated, but does work correctly.
Expand All @@ -84,9 +96,26 @@ public final class InvocationHandlerDelegation extends BugChecker implements Bug
ChildMultiMatcher.MatchType.AT_LEAST_ONE,
Matchers.isSubtypeOf(InvocationTargetException.class));

private static final Matcher<Tree> CONTAINS_INSTANCEOF_ITE = Matchers.contains(
InstanceOfTree.class,
(instanceOfTree, state) -> ASTHelpers.isSameType(
ASTHelpers.getType(instanceOfTree.getType()),
state.getTypeFromString(InvocationTargetException.class.getName()),
state));

private static final Matcher<Tree> CONTAINS_UNWRAP_ITE = Matchers.anyOf(
Matchers.contains(ExpressionTree.class, UNWRAP_ITE),
Matchers.contains(ExpressionTree.class, PASS_ITE));
Matchers.contains(ExpressionTree.class, PASS_ITE),
Matchers.contains(
IfTree.class,
(Matcher<IfTree>) (ifExpression, state) ->
CONTAINS_INSTANCEOF_ITE.matches(ifExpression.getCondition(), state)
&& CONTAINS_UNWRAP_THROWABLE.matches(ifExpression.getThenStatement(), state)),
Matchers.contains(
ConditionalExpressionTree.class,
(Matcher<ConditionalExpressionTree>) (ifExpression, state) ->
CONTAINS_INSTANCEOF_ITE.matches(ifExpression.getCondition(), state)
&& CONTAINS_UNWRAP_THROWABLE.matches(ifExpression.getTrueExpression(), state)));

private static final Matcher<MethodTree> HANDLES_ITE =
Matchers.anyOf(
Expand Down
71 changes: 71 additions & 0 deletions ...prone/src/test/java/com/palantir/baseline/errorprone/InvocationHandlerDelegationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,77 @@ void testCorrectInvocationHandler_delegatesException() {
.doTest();
}

@Test
void testInvocationHandler_instanceOf_if() {
helper().addSourceLines(
"Test.java",
"import java.lang.reflect.InvocationHandler;",
"import java.lang.reflect.Method;",
"import java.lang.reflect.InvocationTargetException;",
"final class Test implements InvocationHandler {",
" @Override",
" public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {",
" try {",
" return method.invoke(this, args);",
" } catch (Throwable t) {",
" if (t instanceof InvocationTargetException) {",
" throw t.getCause();",
" }",
" throw t;",
" }",
" }",
"}")
.doTest();
}

@Test
void testInvocationHandler_instanceOf_ternary() {
helper().addSourceLines(
"Test.java",
"import java.lang.reflect.InvocationHandler;",
"import java.lang.reflect.Method;",
"import java.lang.reflect.InvocationTargetException;",
"final class Test implements InvocationHandler {",
" @Override",
" public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {",
" try {",
" return method.invoke(this, args);",
" } catch (Throwable t) {",
" throw (t instanceof InvocationTargetException) ? t.getCause() : t;",
" }",
" }",
"}")
.doTest();
}

@Test
void testInvocationHandler_delegatedThrowable() {
// This implementation is functionally correct, but risks breaking when code is refactored.
helper().addSourceLines(
"Test.java",
"import java.lang.reflect.InvocationHandler;",
"import java.lang.reflect.Method;",
"import java.lang.reflect.InvocationTargetException;",
"final class Test implements InvocationHandler {",
" @Override",
" public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {",
" try {",
" // BUG: Diagnostic contains: unwrap InvocationTargetException",
" return method.invoke(this, args);",
" } catch (Throwable t) {",
" return handle(t);",
" }",
" }",
" private static Object handle(Throwable t) throws Throwable {",
" if (t instanceof InvocationTargetException) {",
" throw t.getCause();",
" }",
" throw t;",
" }",
"}")
.doTest();
}

private CompilationTestHelper helper() {
return CompilationTestHelper.newInstance(InvocationHandlerDelegation.class, getClass());
}
Expand Down

0 comments on commit 508121d

Please sign in to comment.