Check that JUnit 5 lifecycle methods are not public or private

Closes gh-115

Author: wilkinsona

spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringJUnit5Check.java

@@ -43,6 +43,10 @@ public class SpringJUnit5Check extends AbstractSpringCheck {
 	private static final List<String> TEST_ANNOTATIONS = Collections
 			.unmodifiableList(Arrays.asList("Test", JUNIT4_TEST_ANNOTATION, JUNIT5_TEST_ANNOTATION));
 
+	private static final List<String> LIFECYCLE_ANNOTATIONS = Collections.unmodifiableList(Arrays.asList("BeforeAll",
+			"org.junit.jupiter.api.BeforeAll", "BeforeEach", "org.junit.jupiter.api.BeforeEach", "AfterAll",
+			"org.junit.jupiter.api.AfterAll", "AfterEach", "org.junit.jupiter.api.AfterEach"));
+
 	private static final List<String> BANNED_IMPORTS;
 	static {
 		List<String> bannedImports = new ArrayList<>();
@@ -62,6 +66,8 @@ public class SpringJUnit5Check extends AbstractSpringCheck {
 
 	private final Map<String, FullIdent> imports = new LinkedHashMap<>();
 
+	private final List<DetailAST> lifecycleMethods = new ArrayList<>();
+
 	@Override
 	public int[] getAcceptableTokens() {
 		return new int[] { TokenTypes.METHOD_DEF, TokenTypes.IMPORT };
@@ -71,6 +77,7 @@ public int[] getAcceptableTokens() {
 	public void beginTree(DetailAST rootAST) {
 		this.imports.clear();
 		this.testMethods.clear();
+		this.lifecycleMethods.clear();
 	}
 
 	@Override
@@ -88,6 +95,9 @@ private void visitMethodDef(DetailAST ast) {
 		if (AnnotationUtil.containsAnnotation(ast, TEST_ANNOTATIONS)) {
 			this.testMethods.add(ast);
 		}
+		if (AnnotationUtil.containsAnnotation(ast, LIFECYCLE_ANNOTATIONS)) {
+			this.lifecycleMethods.add(ast);
+		}
 	}
 
 	private void visitImport(DetailAST ast) {
@@ -103,7 +113,7 @@ public void finishTree(DetailAST rootAST) {
 	}
 
 	private boolean shouldCheck() {
-		if (this.testMethods.isEmpty()) {
+		if (this.testMethods.isEmpty() && this.lifecycleMethods.isEmpty()) {
 			return false;
 		}
 		for (String unlessImport : this.unlessImports) {
@@ -126,13 +136,18 @@ private void check() {
 				log(testMethod, "junit5.bannedTestAnnotation");
 			}
 		}
-		for (DetailAST testMethod : this.testMethods) {
-			DetailAST modifiers = testMethod.findFirstToken(TokenTypes.MODIFIERS);
+		checkMethodVisibility(this.testMethods, "junit5.testPublicMethod", "junit5.testPrivateMethod");
+		checkMethodVisibility(this.lifecycleMethods, "junit5.lifecyclePublicMethod", "junit5.lifecyclePrivateMethod");
+	}
+
+	private void checkMethodVisibility(List<DetailAST> methods, String publicMessageKey, String privateMessageKey) {
+		for (DetailAST method : methods) {
+			DetailAST modifiers = method.findFirstToken(TokenTypes.MODIFIERS);
 			if (modifiers.findFirstToken(TokenTypes.LITERAL_PUBLIC) != null) {
-				log(testMethod, "junit5.publicMethod");
+				log(method, publicMessageKey);
 			}
 			if (modifiers.findFirstToken(TokenTypes.LITERAL_PRIVATE) != null) {
-				log(testMethod, "junit5.privateMethod");
+				log(method, privateMessageKey);
 			}
 		}
 	}

spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/check/messages.properties

@@ -8,7 +8,9 @@ javadoc.badCase=Javadoc element descriptions should not start with an uppercase
 header.unexpected=Unexpected header.
 nothis.unexpected=Reference to instance variable ''{0}'' should not use \"this.\".
 methodorder.outOfOrder=Method ''{0}'' is out of order, expected {1}.
-junit5.publicMethod="Test method ''{0}'' should not be public."
-junit5.privateMethod="Test method ''{0}'' should not be private."
 junit5.bannedImport="Import ''{0}'' should not be used in a JUnit 5 test."
 junit5.bannedTestAnnotation="JUnit 4 @Test annotation should not be used in a JUnit 5 test."
+junit5.lifecyclePublicMethod="Lifecycle method ''{0}'' should not be public."
+junit5.lifecyclePrivateMethod="Lifecycle method ''{0}'' should not be private."
+junit5.testPublicMethod="Test method ''{0}'' should not be public."
+junit5.testPrivateMethod="Test method ''{0}'' should not be private."

spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/JUnit5BadModifier.txt

@@ -1,2 +1,10 @@
 +Test method 'doSomethingWorks' should not be public
 +Test method 'doSomethingElseWorks' should not be private
++Lifecycle method 'publicBeforeAll' should not be public
++Lifecycle method 'publicBeforeEach' should not be public
++Lifecycle method 'publicAfterAll' should not be public
++Lifecycle method 'publicAfterEach' should not be public
++Lifecycle method 'privateBeforeAll' should not be private
++Lifecycle method 'privateBeforeEach' should not be private
++Lifecycle method 'privateAfterAll' should not be private
++Lifecycle method 'privateAfterEach' should not be private

spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5BadModifier.java

@@ -23,6 +23,46 @@
  */
 public class JUnit5BadModifier {
 
+	@BeforeAll
+	public static void publicBeforeAll() {
+
+	}
+
+	@BeforeEach
+	public void publicBeforeEach() {
+
+	}
+
+	@AfterAll
+	public static void publicAfterAll() {
+
+	}
+
+	@BeforeEach
+	public void publicAfterEach() {
+
+	}
+
+	@BeforeAll
+	private static void privateBeforeAll() {
+
+	}
+
+	@BeforeEach
+	private void privateBeforeEach() {
+
+	}
+
+	@AfterAll
+	private static void privateAfterAll() {
+
+	}
+
+	@BeforeEach
+	private void privateAfterEach() {
+
+	}
+
 	@Test
 	public void doSomethingWorks() {
 		// test here