Allow suppressing UnusedException on the CatchTree's VariableTree.

Quite upset I didn't think of this myself, to be honest. Fixes #1156 RELNOTES: Allow suppressing UnusedException on the CatchTree's VariableTree. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=228749006
google · Jan 15, 2019 · 1056431 · 1056431
1 parent aed0552
commit 1056431
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 8 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java
@@ -19,15 +19,15 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.Iterables.getLast;
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.BugPattern.StandardTags.STYLE;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Streams;
 import com.google.errorprone.BugPattern;
-import com.google.errorprone.BugPattern.ProvidesFix;
-import com.google.errorprone.BugPattern.StandardTags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.CatchTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
@@ -65,17 +65,21 @@
         "This catch block catches an exception and re-throws another, but swallows the caught"
             + " exception rather than setting it as a cause. This can make debugging harder.",
     severity = WARNING,
-    tags = StandardTags.STYLE,
+    tags = STYLE,
     linkType = CUSTOM,
     link = "https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions",
-    providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
+    providesFix = REQUIRES_HUMAN_ATTENTION,
+    documentSuppression = false)
 public final class UnusedException extends BugChecker implements CatchTreeMatcher {
 
   private static final ImmutableSet<Modifier> VISIBILITY_MODIFIERS =
       ImmutableSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC);
 
   @Override
   public Description matchCatch(CatchTree tree, VisitorState state) {
+    if (isSuppressed(tree.getParameter())) {
+      return Description.NO_MATCH;
+    }
     VarSymbol exceptionSymbol = ASTHelpers.getSymbol(tree.getParameter());
     AtomicBoolean symbolUsed = new AtomicBoolean(false);
     ((JCTree) tree)

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedExceptionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedExceptionTest.java
@@ -33,6 +33,9 @@ public final class UnusedExceptionTest {
   private final CompilationTestHelper compilationHelper =
       CompilationTestHelper.newInstance(UnusedException.class, getClass());
 
+  private final BugCheckerRefactoringTestHelper refactoringHelper =
+      BugCheckerRefactoringTestHelper.newInstance(new UnusedException(), getClass());
+
   @Test
   public void positiveCase() {
     compilationHelper
@@ -52,7 +55,7 @@ public void positiveCase() {
 
   @Test
   public void refactoring() {
-    BugCheckerRefactoringTestHelper.newInstance(new UnusedException(), getClass())
+    refactoringHelper
         .addInputLines(
             "in/Test.java",
             "class Test {",
@@ -188,9 +191,25 @@ public void suppressible() {
         .doTest();
   }
 
+  @Test
+  public void suppressibleViaCatchBlock() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "class Test {",
+            "  void test() {",
+            "    try {",
+            "    } catch (@SuppressWarnings(\"UnusedException\") Exception e) {",
+            "      throw new RuntimeException(\"foo\");",
+            "    }",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void anonymousClass() {
-    BugCheckerRefactoringTestHelper.newInstance(new UnusedException(), getClass())
+    refactoringHelper
         .addInputLines(
             "in/Test.java",
             "class Test {",
@@ -216,7 +235,7 @@ public void anonymousClass() {
 
   @Test
   public void replacementNotVisible() {
-    BugCheckerRefactoringTestHelper.newInstance(new UnusedException(), getClass())
+    refactoringHelper
         .addInputLines(
             "in/MyException.java",
             "class MyException extends RuntimeException {",

diff --git a/docs/bugpattern/UnusedException.md b/docs/bugpattern/UnusedException.md
@@ -25,5 +25,13 @@ Prefer wrapping the original exception instead,
 ```
 
 Suppress false positives with `@SuppressWarnings("UnusedException")` on the
-enclosing element. Consider also adding a comment to explain why the exception
+ignored exception. Consider also adding a comment to explain why the exception
 should not be propagated.
+
+```java
+  try {
+    ...
+  } catch (@SuppressWarnings("UnusedException") IOException e) {
+    throw new IllegalStateException();
+  }
+```