diff --git a/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringJUnit5Check.java b/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringJUnit5Check.java index db7cbbfd..59eb1bec 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringJUnit5Check.java +++ b/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 BANNED_IMPORTS; static { Set bannedImports = new LinkedHashSet<>(); @@ -84,9 +86,13 @@ public class SpringJUnit5Check extends AbstractSpringCheck { private final List lifecycleMethods = new ArrayList<>(); + private final List 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 methods, String publicMessageKey, String privateMessageKey) { - for (DetailAST method : methods) { - DetailAST modifiers = method.findFirstToken(TokenTypes.MODIFIERS); + private void checkVisibility(List 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 --git a/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringTestFileNameCheck.java b/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringTestFileNameCheck.java index da188129..b2041870 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringTestFileNameCheck.java +++ b/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 --git a/spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/check/messages.properties b/spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/check/messages.properties index 9982830e..a27f9263 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/check/messages.properties +++ b/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 --git a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/JUnit5BadModifier.txt b/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/JUnit5BadModifier.txt index 376aa13c..7ec1d4de 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/JUnit5BadModifier.txt +++ b/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 --git a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5BadModifier.java b/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5BadModifier.java index e9b91697..9c59c587 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5BadModifier.java +++ b/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 --git a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5Valid.java b/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5Valid.java index cc7b4cd0..bad7023d 100644 --- a/spring-javaformat/spring-javaformat-checkstyle/src/test/resources/source/JUnit5Valid.java +++ b/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() {