allow ITE to be handled in another function

palantir · Nov 8, 2019 · 99c8b96 · 99c8b96
1 parent 0146d26
commit 99c8b96
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 3 deletions.
diff --git a/...ror-prone/src/main/java/com/palantir/baseline/errorprone/InvocationHandlerDelegation.java b/...ror-prone/src/main/java/com/palantir/baseline/errorprone/InvocationHandlerDelegation.java
@@ -21,6 +21,7 @@
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.matchers.ChildMultiMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
@@ -44,7 +45,10 @@
         severity = BugPattern.SeverityLevel.WARNING,
         summary = "InvocationHandlers which delegate to another object must catch and unwrap "
                 + "InvocationTargetException, otherwise an UndeclaredThrowableException will be thrown "
-                + "each time the delegate throws an exception")
+                + "each time the delegate throws an exception.\n"
+                + "This check is intended to be advisory. It's fine to "
+                + "@SuppressWarnings(\"InvocationHandlerDelegation\") in certain cases, "
+                + "but is usually not recommended.")
 public final class InvocationHandlerDelegation extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
 
     private static final Matcher<MethodTree> INVOCATION_HANDLER = Matchers.anyOf(
@@ -75,8 +79,14 @@ public final class InvocationHandlerDelegation extends BugChecker implements Bug
             .namedAnyOf("getCause", "getTargetException")
             .withParameters();
 
-    private static final Matcher<Tree> CONTAINS_UNWRAP_ITE = Matchers.contains(
-            ExpressionTree.class, UNWRAP_ITE);
+    private static final Matcher<ExpressionTree> PASS_ITE = Matchers.methodInvocation(
+            Matchers.anyMethod(),
+            ChildMultiMatcher.MatchType.AT_LEAST_ONE,
+            Matchers.isSubtypeOf(InvocationTargetException.class));
+
+    private static final Matcher<Tree> CONTAINS_UNWRAP_ITE = Matchers.anyOf(
+            Matchers.contains(ExpressionTree.class, UNWRAP_ITE),
+            Matchers.contains(ExpressionTree.class, PASS_ITE));
 
     private static final Matcher<MethodTree> HANDLES_ITE =
             Matchers.anyOf(

diff --git a/...prone/src/test/java/com/palantir/baseline/errorprone/InvocationHandlerDelegationTest.java b/...prone/src/test/java/com/palantir/baseline/errorprone/InvocationHandlerDelegationTest.java
@@ -152,6 +152,29 @@ void testCorrectInvocationHandler_lambda() {
                 .doTest();
     }
 
+    @Test
+    void testCorrectInvocationHandler_delegatesException() {
+        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 (InvocationTargetException e) {",
+                "        throw handleInvocationTargetException(e);",
+                "    }",
+                "  }",
+                "  private Throwable handleInvocationTargetException(InvocationTargetException e) throws Throwable {",
+                "    throw e.getCause();",
+                "  }",
+                "}")
+                .doTest();
+    }
+
     private CompilationTestHelper helper() {
         return CompilationTestHelper.newInstance(InvocationHandlerDelegation.class, getClass());
     }