Add JUnit 5 support to JUnit-related bug patterns

check_api/src/main/java/com/google/errorprone/matchers/JUnitMatchers.java

@@ -47,11 +47,16 @@
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.ExpressionStatementTree;
+import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.tree.TryTree;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Type.ClassType;
+import java.util.Optional;
 import javax.lang.model.element.Modifier;
 
 /**
@@ -61,18 +66,25 @@
  * @author eaftan@google.com (Eddie Aftandillian)
  */
 public final class JUnitMatchers {
+  public static final String JUNIT3_TEST_CASE_CLASS = "junit.framework.TestCase";
   public static final String JUNIT4_TEST_ANNOTATION = "org.junit.Test";
+  public static final String JUNIT5_TEST_ANNOTATION = "org.junit.jupiter.api.Test";
   public static final String JUNIT4_THEORY_ANNOTATION = "org.junit.experimental.theories.Theory";
   public static final String JUNIT_BEFORE_ANNOTATION = "org.junit.Before";
   public static final String JUNIT_AFTER_ANNOTATION = "org.junit.After";
   public static final String JUNIT_BEFORE_CLASS_ANNOTATION = "org.junit.BeforeClass";
   public static final String JUNIT_AFTER_CLASS_ANNOTATION = "org.junit.AfterClass";
-  public static final String JUNIT4_RUN_WITH_ANNOTATION = "org.junit.runner.RunWith";
-  public static final String JUNIT4_ASSERT_CLASS = "org.junit.Assert";
-  public static final String JUNIT3_TEST_CASE_CLASS = "junit.framework.TestCase";
+  public static final String JUNIT5_BEFORE_EACH_ANNOTATION = "org.junit.jupiter.api.BeforeEach";
+  public static final String JUNIT5_AFTER_EACH_ANNOTATION = "org.junit.jupiter.api.AfterEach";
+  public static final String JUNIT5_BEFORE_ALL_ANNOTATION = "org.junit.jupiter.api.BeforeAll";
+  public static final String JUNIT5_AFTER_ALL_ANNOTATION = "org.junit.jupiter.api.AfterAll";
   public static final String JUNIT4_IGNORE_ANNOTATION = "org.junit.Ignore";
-  public static final String JUNIT4_RUNNER_CLASS = "org.junit.runners.JUnit4";
+  public static final String JUNIT5_DISABLED_ANNOTATION = "org.junit.jupiter.api.Disabled";
   public static final String JUNIT3_ASSERT_CLASS = "junit.framework.Assert";
+  public static final String JUNIT4_ASSERT_CLASS = "org.junit.Assert";
+  public static final String JUNIT5_ASSERT_CLASS = "org.junit.jupiter.api.Assertions";
+  public static final String JUNIT4_RUN_WITH_ANNOTATION = "org.junit.runner.RunWith";
+  public static final String JUNIT4_RUNNER_CLASS = "org.junit.runners.JUnit4";
 
   /**
    * Checks if a method, or any overridden method, is annotated with any annotation from the
@@ -126,6 +138,47 @@ private static boolean hasJUnitAttr(MethodSymbol methodSym) {
   public static final Matcher<ClassTree> hasJUnit4TestCases =
       hasMethod(hasAnnotationOnAnyOverriddenMethod(JUNIT4_TEST_ANNOTATION));
 
+  /** Match a class which has one or more methods with a JUnit 5 @Test annotation. */
+  public static final Matcher<ClassTree> hasJUnit5TestCases =
+      hasMethod(hasAnnotation(JUNIT5_TEST_ANNOTATION));
+
+  /** Match a method annotated with JUnit 5 @BeforeEach. */
+  public static final Matcher<MethodTree> hasJUnit5BeforeEach =
+      hasAnnotation(JUNIT5_BEFORE_EACH_ANNOTATION);
+
+  /** Match a method annotated with JUnit 5 @AfterEach. */
+  public static final Matcher<MethodTree> hasJUnit5AfterEach =
+      hasAnnotation(JUNIT5_AFTER_EACH_ANNOTATION);
+
+  /** Match a method annotated with JUnit 5 @BeforeAll. */
+  public static final Matcher<MethodTree> hasJUnit5BeforeAll =
+      hasAnnotation(JUNIT5_BEFORE_ALL_ANNOTATION);
+
+  /** Match a method annotated with JUnit 5 @AfterAll. */
+  public static final Matcher<MethodTree> hasJUnit5AfterAll =
+      hasAnnotation(JUNIT5_AFTER_ALL_ANNOTATION);
+
+  /** Match a method annotated with any JUnit 5 before annotation (@BeforeEach or @BeforeAll). */
+  public static final Matcher<MethodTree> hasJUnit5BeforeAnnotations =
+      anyOf(hasJUnit5BeforeEach, hasJUnit5BeforeAll);
+
+  /** Match a method annotated with any JUnit 5 after annotation (@AfterEach or @AfterAll). */
+  public static final Matcher<MethodTree> hasJUnit5AfterAnnotations =
+      anyOf(hasJUnit5AfterEach, hasJUnit5AfterAll);
+
+  /**
+   * Returns {@code true} if the enclosing class of the given state is a JUnit 5 test class.
+   */
+  public static boolean isJUnit5TestClass(VisitorState state) {
+    for (com.sun.source.tree.Tree ancestor : state.getPath()) {
+      if (ancestor instanceof ClassTree classTree
+          && hasJUnit5TestCases.matches(classTree, state)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * Match a class which appears to be a JUnit 3 test class.
    *
@@ -238,12 +291,13 @@ private static boolean hasJUnitAttr(MethodSymbol methodSym) {
           hasAnnotationOnAnyOverriddenMethod(JUNIT4_TEST_ANNOTATION),
           not(hasAnnotationOnAnyOverriddenMethod(JUNIT4_IGNORE_ANNOTATION)));
 
-  /** Matches a JUnit 3 or 4 test case. */
+  /** Matches a JUnit 3, 4, or 5 test case. */
   public static final Matcher<MethodTree> TEST_CASE =
       anyOf(
           isJunit3TestCase,
           hasAnnotation(JUNIT4_TEST_ANNOTATION),
-          hasAnnotation(JUNIT4_THEORY_ANNOTATION));
+          hasAnnotation(JUNIT4_THEORY_ANNOTATION),
+          hasAnnotation(JUNIT5_TEST_ANNOTATION));
 
   /**
    * A list of test runners that this matcher should look for in the @RunWith annotation. Subclasses
@@ -328,5 +382,56 @@ public static Matcher<ExpressionTree> isJUnit4TestRunnerOfType(Iterable<String>
   public static final Matcher<ClassTree> isAmbiguousJUnitVersion =
       allOf(isTestCaseDescendant, anyOf(hasJUnit4TestRunner, hasJUnit4TestCases));
 
+  /**
+   * Returns {@code true} if the given method invocation is a call to a JUnit 5 assertion method,
+   * determined by the symbol owner being {@code org.junit.jupiter.api.Assertions}.
+   *
+   * <p>This is more robust than import scanning: it works with fully-qualified calls, imported
+   * calls, and star imports, and answers per-call rather than per-file.
+   */
+  public static boolean isJUnit5AssertionCall(ExpressionTree tree) {
+    Symbol sym = getSymbol(tree);
+    return sym != null
+        && sym.owner.getQualifiedName().toString().equals(JUNIT5_ASSERT_CLASS);
+  }
+
+  /**
+   * Returns the assertion class name appropriate for the enclosing test class: {@link
+   * #JUNIT5_ASSERT_CLASS} for JUnit 5 tests, {@link #JUNIT4_ASSERT_CLASS} for JUnit 4 and
+   * earlier.
+   */
+  public static String getAssertionClassName(VisitorState state) {
+    return isJUnit5TestClass(state) ? JUNIT5_ASSERT_CLASS : JUNIT4_ASSERT_CLASS;
+  }
+
+  /**
+   * Returns the assertion class name appropriate for the given assertion call: {@link
+   * #JUNIT5_ASSERT_CLASS} if the call is to a JUnit 5 assertion, {@link #JUNIT4_ASSERT_CLASS}
+   * otherwise.
+   */
+  public static String getAssertionClassName(ExpressionTree tree) {
+    return isJUnit5AssertionCall(tree) ? JUNIT5_ASSERT_CLASS : JUNIT4_ASSERT_CLASS;
+  }
+
+  /**
+   * Scans the try block of a {@link TryTree} for a {@code fail()} call statement.
+   *
+   * @return the {@code fail()} invocation, or empty if not found
+   */
+  public static Optional<MethodInvocationTree> findFailCallInTry(TryTree tryTree) {
+    for (StatementTree statement : tryTree.getBlock().getStatements()) {
+      if (statement instanceof ExpressionStatementTree est
+          && est.getExpression() instanceof MethodInvocationTree mit) {
+        Symbol sym = getSymbol(mit);
+        if (sym != null
+            && sym.getSimpleName().contentEquals("fail")
+            && sym.isStatic()) {
+          return Optional.of(mit);
+        }
+      }
+    }
+    return Optional.empty();
+  }
+
   private JUnitMatchers() {}
 }

check_api/src/main/java/com/google/errorprone/matchers/Matchers.java

@@ -1509,7 +1509,11 @@ public static Matcher<ExpressionTree> instanceHashCodeInvocation() {
 
   private static final Matcher<ExpressionTree> ASSERT_EQUALS =
       staticMethod()
-          .onClassAny("org.junit.Assert", "junit.framework.Assert", "junit.framework.TestCase")
+          .onClassAny(
+              "org.junit.jupiter.api.Assertions",
+              "org.junit.Assert",
+              "junit.framework.Assert",
+              "junit.framework.TestCase")
           .named("assertEquals");
 
   /**
@@ -1522,7 +1526,11 @@ public static Matcher<ExpressionTree> assertEqualsInvocation() {
 
   private static final Matcher<ExpressionTree> ASSERT_NOT_EQUALS =
       staticMethod()
-          .onClassAny("org.junit.Assert", "junit.framework.Assert", "junit.framework.TestCase")
+          .onClassAny(
+              "org.junit.jupiter.api.Assertions",
+              "org.junit.Assert",
+              "junit.framework.Assert",
+              "junit.framework.TestCase")
           .named("assertNotEquals");
 
   /**

check_api/src/main/java/com/google/errorprone/matchers/UnusedReturnValueMatcher.java

@@ -187,6 +187,7 @@ private static boolean exceptionTesting(ExpressionTree tree, VisitorState state)
           instanceMethod()
               .onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
               .named("fail"),
+          staticMethod().onClass("org.junit.jupiter.api.Assertions").named("fail"),
           staticMethod().onClass("org.junit.Assert").named("fail"),
           staticMethod().onClass("junit.framework.Assert").named("fail"),
           staticMethod().onClass("junit.framework.TestCase").named("fail"));

check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

@@ -1482,7 +1482,7 @@ public static Type getUpperBound(Type type, Types types) {
 
   /**
    * Returns true if the leaf node in the {@link TreePath} from {@code state} sits somewhere
-   * underneath a class or method that is marked as JUnit 3 or 4 test code.
+   * underneath a class or method that is marked as JUnit test code.
    */
   public static boolean isJUnitTestCode(VisitorState state) {
     for (Tree ancestor : state.getPath()) {
@@ -1492,7 +1492,8 @@ public static boolean isJUnitTestCode(VisitorState state) {
       }
       if (ancestor instanceof ClassTree classTree
           && (JUnitMatchers.isTestCaseDescendant.matches(classTree, state)
-              || hasAnnotation(getSymbol(ancestor), JUNIT4_RUN_WITH_ANNOTATION, state))) {
+              || hasAnnotation(getSymbol(ancestor), JUNIT4_RUN_WITH_ANNOTATION, state)
+              || JUnitMatchers.hasJUnit5TestCases.matches(classTree, state))) {
         return true;
       }
     }

core/pom.xml

@@ -127,6 +127,13 @@
       <version>${junit.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <!-- Eclipse Public License 1.0 -->
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-api</artifactId>
+      <version>${junit5.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <!-- Apache 2.0 -->
       <groupId>com.google.testparameterinjector</groupId>

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

@@ -16,8 +16,10 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static com.google.errorprone.matchers.JUnitMatchers.hasJUnit5TestCases;
 import static com.google.errorprone.matchers.JUnitMatchers.isJUnit4TestClass;
 import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.enclosingClass;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 import static com.google.errorprone.matchers.Matchers.hasAnnotationOnAnyOverriddenMethod;
@@ -27,6 +29,7 @@
 import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.JUnitMatchers;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.util.ASTHelpers;
@@ -49,6 +52,7 @@
 abstract class AbstractJUnit4InitMethodNotRun extends BugChecker implements MethodTreeMatcher {
 
   private static final String JUNIT_TEST = "org.junit.Test";
+  private static final String JUNIT5_TEST = JUnitMatchers.JUNIT5_TEST_ANNOTATION;
 
   /**
    * Returns a matcher that selects which methods this matcher applies to (e.g. public void setUp()
@@ -63,17 +67,17 @@ abstract class AbstractJUnit4InitMethodNotRun extends BugChecker implements Meth
    * <p>If another annotation is on the method that has the same name, the import will be replaced
    * with the appropriate one (e.g.: com.example.Before becomes org.junit.Before)
    */
-  protected abstract String correctAnnotation();
+  protected abstract String correctAnnotation(VisitorState state);
 
   /**
    * Returns a collection of 'before-and-after' pairs of annotations that should be replaced on
    * these methods.
    *
    * <p>If this method matcher finds a method annotated with {@link
-   * AnnotationReplacements#badAnnotation}, instead of applying {@link #correctAnnotation()},
+   * AnnotationReplacements#badAnnotation}, instead of applying {@link #correctAnnotation},
    * instead replace it with {@link AnnotationReplacements#goodAnnotation}
    */
-  protected abstract List<AnnotationReplacements> annotationReplacements();
+  protected abstract List<AnnotationReplacements> annotationReplacements(VisitorState state);
 
   /**
    * Matches if all of the following conditions are true: 1) The method matches {@link
@@ -88,7 +92,8 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
         allOf(
                 methodMatcher(),
                 not(hasAnnotationOnAnyOverriddenMethod(JUNIT_TEST)),
-                enclosingClass(isJUnit4TestClass))
+                not(hasAnnotationOnAnyOverriddenMethod(JUNIT5_TEST)),
+                enclosingClass(anyOf(isJUnit4TestClass, hasJUnit5TestCases)))
             .matches(methodTree, state);
     if (!matches) {
       return Description.NO_MATCH;
@@ -97,7 +102,7 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
     // For each annotationReplacement, replace the first annotation that matches. If any of them
     // matches, don't try and do the rest of the work.
     Description description;
-    for (AnnotationReplacements replacement : annotationReplacements()) {
+    for (AnnotationReplacements replacement : annotationReplacements(state)) {
       description =
           tryToReplaceAnnotation(
               methodTree, state, replacement.badAnnotation, replacement.goodAnnotation);
@@ -108,7 +113,7 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
 
     // Search for another @Before annotation on the method and replace the import
     // if we find one
-    String correctAnnotation = correctAnnotation();
+    String correctAnnotation = correctAnnotation(state);
     String unqualifiedClassName = getUnqualifiedClassName(correctAnnotation);
     for (AnnotationTree annotationNode : methodTree.getModifiers().getAnnotations()) {
       Symbol annoSymbol = ASTHelpers.getSymbol(annotationNode);

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

@@ -19,6 +19,7 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getStartPosition;
 import static java.util.stream.Collectors.joining;
@@ -50,7 +51,9 @@ public class AssertThrowsBlockToExpression extends BugChecker
     implements MethodInvocationTreeMatcher {
 
   private static final Matcher<ExpressionTree> MATCHER =
-      staticMethod().onClass("org.junit.Assert").named("assertThrows");
+      anyOf(
+          staticMethod().onClass("org.junit.jupiter.api.Assertions").named("assertThrows"),
+          staticMethod().onClass("org.junit.Assert").named("assertThrows"));
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {

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

@@ -78,7 +78,9 @@
 public class AssertThrowsMinimizer extends BugChecker implements MethodTreeMatcher {
 
   private static final Matcher<ExpressionTree> MATCHER =
-      anyOf(staticMethod().onClass("org.junit.Assert").named("assertThrows"));
+      anyOf(
+          staticMethod().onClass("org.junit.jupiter.api.Assertions").named("assertThrows"),
+          staticMethod().onClass("org.junit.Assert").named("assertThrows"));
 
   private final ConstantExpressions constantExpressions;
   private final boolean useVarType;

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

@@ -18,6 +18,7 @@
 
 import static com.google.common.collect.Iterables.getLast;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getStartPosition;
 
@@ -55,7 +56,9 @@ public class AssertThrowsMultipleStatements extends BugChecker
   }
 
   private static final Matcher<ExpressionTree> MATCHER =
-      staticMethod().onClass("org.junit.Assert").named("assertThrows");
+      anyOf(
+          staticMethod().onClass("org.junit.jupiter.api.Assertions").named("assertThrows"),
+          staticMethod().onClass("org.junit.Assert").named("assertThrows"));
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {

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

@@ -29,6 +29,7 @@
 import com.google.errorprone.fixes.Fix;
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.fixes.SuggestedFixes.VariableNamer;
+import com.google.errorprone.matchers.JUnitMatchers;
 import com.google.errorprone.util.ErrorProneComment;
 import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.CatchTree;
@@ -109,7 +110,11 @@ public static Optional<Fix> tryFailToAssertThrows(
       return Optional.empty();
     }
     List<? extends StatementTree> catchStatements = catchTree.getBlock().getStatements();
-    fix.addStaticImport("org.junit.Assert.assertThrows");
+    String assertThrowsClass =
+        JUnitMatchers.findFailCallInTry(tryTree)
+            .map(JUnitMatchers::getAssertionClassName)
+            .orElse(JUnitMatchers.JUNIT4_ASSERT_CLASS);
+    fix.addStaticImport(assertThrowsClass + ".assertThrows");
     List<? extends Tree> resources = tryTree.getResources();
     if (!resources.isEmpty()) {
       fixPrefix.append(