Check that JUnit 5 test classes are package-private

Closes gh-281

Author: wilkinsona

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

@@ -63,6 +63,8 @@ public class SpringJUnit5Check extends AbstractSpringCheck {
 		LIFECYCLE_ANNOTATIONS = Collections.unmodifiableList(new ArrayList<>(annotations));
 	}
 
+	private static final Annotation NESTED_ANNOTATION = new Annotation("org.junit.jupiter.api", "Nested");
+
 	private static final Set<String> BANNED_IMPORTS;
 	static {
 		Set<String> bannedImports = new LinkedHashSet<>();
@@ -84,9 +86,13 @@ public class SpringJUnit5Check extends AbstractSpringCheck {
 
 	private final List<DetailAST> lifecycleMethods = new ArrayList<>();
 
+	private final List<DetailAST> nestedTestClasses = new ArrayList<>();
+
+	private DetailAST testClass;
+
 	@Override
 	public int[] getAcceptableTokens() {
-		return new int[] { TokenTypes.METHOD_DEF, TokenTypes.IMPORT };
+		return new int[] { TokenTypes.METHOD_DEF, TokenTypes.IMPORT, TokenTypes.CLASS_DEF };
 	}
 
 	@Override
@@ -101,9 +107,13 @@ public void visitToken(DetailAST ast) {
 		switch (ast.getType()) {
 			case TokenTypes.METHOD_DEF:
 				visitMethodDef(ast);
+				break;
 			case TokenTypes.IMPORT:
 				visitImport(ast);
 				break;
+			case TokenTypes.CLASS_DEF:
+				visitClassDefinition(ast);
+				break;
 		}
 	}
 
@@ -140,6 +150,17 @@ private void visitImport(DetailAST ast) {
 		this.imports.put(ident.getText(), ident);
 	}
 
+	private void visitClassDefinition(DetailAST ast) {
+		if (ast.getParent().getType() == TokenTypes.COMPILATION_UNIT) {
+			this.testClass = ast;
+		}
+		else {
+			if (containsAnnotation(ast, Arrays.asList(NESTED_ANNOTATION))) {
+				this.nestedTestClasses.add(ast);
+			}
+		}
+	}
+
 	@Override
 	public void finishTree(DetailAST rootAST) {
 		if (shouldCheck()) {
@@ -148,7 +169,7 @@ public void finishTree(DetailAST rootAST) {
 	}
 
 	private boolean shouldCheck() {
-		if (this.testMethods.isEmpty() && this.lifecycleMethods.isEmpty()) {
+		if (this.testMethods.isEmpty() && this.lifecycleMethods.isEmpty() && this.nestedTestClasses.isEmpty()) {
 			return false;
 		}
 		for (String unlessImport : this.unlessImports) {
@@ -160,6 +181,10 @@ private boolean shouldCheck() {
 	}
 
 	private void check() {
+		if (this.testClass != null) {
+			checkVisibility(Arrays.asList(this.testClass), "junit5.publicClass", null);
+		}
+		checkVisibility(this.nestedTestClasses, "junit5.publicNestedClass", "junit5.privateNestedClass");
 		for (String bannedImport : BANNED_IMPORTS) {
 			FullIdent ident = this.imports.get(bannedImport);
 			if (ident != null) {
@@ -171,25 +196,25 @@ private void check() {
 				log(testMethod, "junit5.bannedTestAnnotation");
 			}
 		}
-		checkMethodVisibility(this.testMethods, "junit5.testPublicMethod", "junit5.testPrivateMethod");
-		checkMethodVisibility(this.lifecycleMethods, "junit5.lifecyclePublicMethod", "junit5.lifecyclePrivateMethod");
+		checkVisibility(this.testMethods, "junit5.testPublicMethod", "junit5.testPrivateMethod");
+		checkVisibility(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);
+	private void checkVisibility(List<DetailAST> asts, String publicMessageKey, String privateMessageKey) {
+		for (DetailAST ast : asts) {
+			DetailAST modifiers = ast.findFirstToken(TokenTypes.MODIFIERS);
 			if (modifiers.findFirstToken(TokenTypes.LITERAL_PUBLIC) != null) {
-				log(method, publicMessageKey);
+				log(ast, publicMessageKey);
 			}
-			if (modifiers.findFirstToken(TokenTypes.LITERAL_PRIVATE) != null) {
-				log(method, privateMessageKey);
+			if ((privateMessageKey != null) && (modifiers.findFirstToken(TokenTypes.LITERAL_PRIVATE) != null)) {
+				log(ast, privateMessageKey);
 			}
 		}
 	}
 
-	private void log(DetailAST method, String key) {
-		String name = method.findFirstToken(TokenTypes.IDENT).getText();
-		log(method.getLineNo(), method.getColumnNo(), key, name);
+	private void log(DetailAST ast, String key) {
+		String name = ast.findFirstToken(TokenTypes.IDENT).getText();
+		log(ast.getLineNo(), ast.getColumnNo(), key, name);
 	}
 
 	public void setUnlessImports(String unlessImports) {

Diff for: spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringTestFileNameCheck.java

@@ -41,7 +41,7 @@ protected void processFiltered(File file, FileText fileText) throws CheckstyleEx
 			visitCompilationUnit(JavaParser.parseFileText(fileText, Options.WITHOUT_COMMENTS));
 		}
 	}
-	
+
 	private void visitCompilationUnit(DetailAST ast) {
 		DetailAST child = ast.getFirstChild();
 		while (child != null) {
@@ -52,5 +52,5 @@ private void visitCompilationUnit(DetailAST ast) {
 			child = child.getNextSibling();
 		}
 	}
-	
+
 }

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

@@ -16,6 +16,9 @@ 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.lifecyclePrivateMethod=Lifecycle method ''{0}'' should not be private.
 junit5.lifecyclePublicMethod=Lifecycle method ''{0}'' should not be public.
+junit5.publicClass=Test class ''{0}'' should not be public.
+junit5.publicNestedClass=Nested test class ''{0}'' should not be public.
+junit5.privateNestedClass=Nested test class ''{0}'' should not be private.
 junit5.testPrivateMethod=Test method ''{0}'' should not be private.
 junit5.testPublicMethod=Test method ''{0}'' should not be public.
 lambda.missingParen=Lambda argument missing parentheses.

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

@@ -1,7 +1,11 @@
++Test class 'JUnit5BadModifier' should not be public
++Nested test class 'PublicNestedTests' should not be public
++Nested test class 'PrivateNestedTests' should not be private
 +Test method 'doSomethingWorks' should not be public
 +Test method 'doSomethingElseWorks' should not be private
 +Test method 'doSomethingWithTemplateWorks' should not be public
 +Test method 'doSomethingElseWithTemplateWorks' should not be private
++Test method 'nestedPublicTest' should not be public
 +Lifecycle method 'publicBeforeAll' should not be public
 +Lifecycle method 'publicBeforeEach' should not be public
 +Lifecycle method 'publicAfterAll' should not be public

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

@@ -88,4 +88,19 @@ private void doSomethingElseWithTemplateWorks() {
 		// test here
 	}
 
+	@Nested
+	public static class PublicNestedTests {
+
+		@Test
+		public void nestedPublicTest() {
+
+		}
+
+	}
+
+	@Nested
+	private static class PrivateNestedTests {
+
+	}
+
 }

Diff for: spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5Valid.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2019 the original author or authors.
+ * Copyright 2017-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,7 +21,7 @@
  *
  * @author Phillip Webb
  */
-public class JUnit5Valid {
+class JUnit5Valid {
 
 	@Test
 	void doSomethingWorks() {