Remove exemption for tests from EmptyCatch

core/src/main/java/com/google/errorprone/bugpatterns/EmptyCatch.java

@@ -24,7 +24,6 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.CatchTreeMatcher;
 import com.google.errorprone.matchers.Description;
-import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.CatchTree;
 
@@ -47,12 +46,6 @@ public Description matchCatch(CatchTree tree, VisitorState state) {
     if (state.getTokensForNode(block).stream().anyMatch(t -> !t.comments().isEmpty())) {
       return NO_MATCH;
     }
-    if (ASTHelpers.isJUnitTestCode(state)) {
-      return NO_MATCH;
-    }
-    if (ASTHelpers.isTestNgTestCode(state)) {
-      return NO_MATCH;
-    }
     return describeMatch(tree);
   }
 }

core/src/test/java/com/google/errorprone/bugpatterns/EmptyCatchTest.java

@@ -65,6 +65,7 @@ public class SomeTest {
               public void testNG() {
                 try {
                   System.err.println();
+                // BUG: Diagnostic contains:
                 } catch (Exception doNotCare) {
                 }
               }

core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchNegativeCases.java

@@ -16,10 +16,8 @@
 
 package com.google.errorprone.bugpatterns;
 
-import static org.junit.Assert.fail;
 
 import java.io.FileNotFoundException;
-import org.junit.Test;
 
 /**
  * @author [email protected] (Ding Yuan)
@@ -126,13 +124,4 @@ public void catchIsLoggedOnly() {
       System.out.println("Caught an exception: " + t);
     }
   }
-
-  @Test
-  public void expectedException() {
-    try {
-      System.err.println();
-      fail();
-    } catch (Exception expected) {
-    }
-  }
 }

core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchPositiveCases.java

@@ -16,6 +16,10 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+
 /**
  * @author [email protected] (Ding Yuan)
  */
@@ -32,4 +36,14 @@ public void catchIsCompleteEmpty() {
 
     }
   }
+
+  @Test
+  public void expectedException() {
+    try {
+      System.err.println();
+      fail();
+      // BUG: Diagnostic contains:
+    } catch (Exception expected) {
+    }
+  }
 }

docs/bugpattern/EmptyCatch.md

@@ -0,0 +1,30 @@
+The [Google Java Style Guide §6.2][style] states:
+
+> It is very rarely correct to do nothing in response to a caught exception.
+> (Typical responses are to log it, or if it is considered "impossible", rethrow
+> it as an AssertionError.)
+>
+> When it truly is appropriate to take no action whatsoever in a catch block,
+> the reason this is justified is explained in a comment.
+
+When writing tests that expect an exception to be thrown, prefer using
+[`Assert.assertThrows`][assertthrows] instead of writing a try-catch. That is,
+prefer this:
+
+```java
+assertThrows(NoSuchElementException.class, () -> emptyStack.pop());
+```
+
+instead of this:
+
+```java
+try {
+  emptyStack.pop();
+  fail();
+} catch (NoSuchElementException expected) {
+}
+```
+
+[style]: https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions
+
+[assertthrows]: https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertThrows(java.lang.Class,%20org.junit.function.ThrowingRunnable)