From 33ede7fbc264c0abcdf1123f29a7095167262d84 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 15 Oct 2022 18:11:09 +0200 Subject: [PATCH 1/5] [java] Improve TestClassWithoutTestCases - Added new property "testClassPatterns" to property detect empty test classes - Added support for different JUnit5 annotations (like ParameterizedTest) - Added support for TestNG - Fixes #929 - Fixes #2636 --- docs/pages/release_notes.md | 10 ++ .../pmd/lang/java/rule/AbstractJUnitRule.java | 73 ++++++++-- .../TestClassWithoutTestCasesRule.java | 44 +++++- .../resources/category/java/errorprone.xml | 8 +- .../xml/TestClassWithoutTestCases.xml | 132 ++++++++++++++++-- 5 files changed, 243 insertions(+), 24 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 2ac4d95984b..203298d13e0 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -25,6 +25,13 @@ This is a {{ site.pmd.release_type }} release. The rule is part of the quickstart.xml ruleset. +#### Modified Rules + +* The Java rule {% rule java/errorprone/TestClassWithoutTestCases %} has a new property `testClassPattern`. This is + used to detect empty test classes by name. Previously this rule could only detect empty JUnit3 test cases + properly. To switch back to the old behavior, this property can be set to an empty value which disables the + test class detection by pattern. + ### Fixed Issues * apex * [#4149](https://github.com/pmd/pmd/issues/4149): \[apex] New rule: ApexUnitTestClassShouldHaveRunAs @@ -32,6 +39,9 @@ The rule is part of the quickstart.xml ruleset. * [#4144](https://github.com/pmd/pmd/pull/4144) \[doc] Update docs to reflect supported languages * java-documentation * [#4141](https://github.com/pmd/pmd/issues/4141): \[java] UncommentedEmptyConstructor FP when constructor annotated with @Autowired +* java-errorprone + * [#929](https://github.com/pmd/pmd/issues/929): \[java] Inconsistent results with TestClassWithoutTestCases + * [#2636](https://github.com/pmd/pmd/issues/2636): \[java] TestClassWithoutTestCases false positive with JUnit5 ParameterizedTest ### API Changes diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java index 10b5bfeb6de..4bd020f007a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java @@ -4,7 +4,11 @@ package net.sourceforge.pmd.lang.java.rule; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.annotation.InternalApi; @@ -30,6 +34,14 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { protected static final String JUNIT3_CLASS_NAME = "junit.framework.TestCase"; protected static final String JUNIT4_CLASS_NAME = "org.junit.Test"; protected static final String JUNIT5_CLASS_NAME = "org.junit.jupiter.api.Test"; + private static final Set JUNIT5_TEST_ANNOTATIONS = new HashSet<>(Arrays.asList( + JUNIT5_CLASS_NAME, + "org.junit.jupiter.api.RepeatedTest", + "org.junit.jupiter.api.TestFactory", + "org.junit.jupiter.api.TestTemplate", + "org.junit.jupiter.params.ParameterizedTest")); + + private static final String TESTNG_ANNOTATION = "org.testng.annotations.Test"; protected boolean isJUnit3Class; protected boolean isJUnit4Class; @@ -92,12 +104,12 @@ protected void analyzeJUnitClass(ASTClassOrInterfaceDeclaration node) { if (isJUnit4Class && isJUnit5Class) { isJUnit4Class &= hasImports(node, JUNIT4_CLASS_NAME); - isJUnit5Class &= hasImports(node, JUNIT5_CLASS_NAME); + isJUnit5Class &= hasImports(node, JUNIT5_TEST_ANNOTATIONS); } } public static boolean isTestClass(ASTClassOrInterfaceBody node) { - return !isAbstractClass(node) + return !isAbstractClass(node) && node.getParent() instanceof ASTClassOrInterfaceDeclaration && (isTestClassJUnit3(node) || isTestClassJUnit4(node) || isTestClassJUnit5(node)); } @@ -131,7 +143,7 @@ public static boolean isTestClassJUnit5(ASTClassOrInterfaceBody node) { Node parent = node.getParent(); if (parent instanceof TypeNode) { TypeNode type = (TypeNode) parent; - return isJUnit5Class(type) && hasImports(type, JUNIT5_CLASS_NAME); + return isJUnit5Class(type) && hasImports(type, JUNIT5_TEST_ANNOTATIONS); } return false; } @@ -145,7 +157,8 @@ public static boolean isTestMethod(ASTMethodDeclaration method) { ASTClassOrInterfaceBody type = method.getFirstParentOfType(ASTClassOrInterfaceBody.class); return isTestClassJUnit3(type) && method.isPublic() && method.isVoid() && method.getName().startsWith("test") || isTestClassJUnit4(type) && method.isPublic() && doesNodeContainJUnitAnnotation(method.getParent(), JUNIT4_CLASS_NAME) - || isTestClassJUnit5(type) && doesNodeContainJUnitAnnotation(method.getParent(), JUNIT5_CLASS_NAME); + || isTestClassJUnit5(type) && doesNodeContainJUnitAnnotation(method.getParent(), JUNIT5_TEST_ANNOTATIONS) + || doesNodeContainJUnitAnnotation(method.getParent(), TESTNG_ANNOTATION); } public boolean isJUnitMethod(ASTMethodDeclaration method, Object data) { @@ -170,7 +183,7 @@ private boolean isJUnit4Method(ASTMethodDeclaration method) { } private boolean isJUnit5Method(ASTMethodDeclaration method) { - return isJUnit5Class && doesNodeContainJUnitAnnotation(method.getParent(), JUNIT5_CLASS_NAME); + return isJUnit5Class && doesNodeContainJUnitAnnotation(method.getParent(), JUNIT5_TEST_ANNOTATIONS); } private boolean isJUnit3Method(ASTMethodDeclaration method) { @@ -186,34 +199,72 @@ private static boolean isJUnit4Class(JavaNode node) { } private static boolean isJUnit5Class(JavaNode node) { - return doesNodeContainJUnitAnnotation(node, JUNIT5_CLASS_NAME); + return doesNodeContainJUnitAnnotation(node, JUNIT5_TEST_ANNOTATIONS); } private static boolean doesNodeContainJUnitAnnotation(JavaNode node, String annotationTypeClassName) { + return doesNodeContainJUnitAnnotation(node, Collections.singleton(annotationTypeClassName)); + } + + private static boolean doesNodeContainJUnitAnnotation(JavaNode node, Set annotationTypeClassNames) { List annotations = node.findDescendantsOfType(ASTAnnotation.class); for (ASTAnnotation annotation : annotations) { Node annotationTypeNode = annotation.getChild(0); TypeNode annotationType = (TypeNode) annotationTypeNode; if (annotationType.getType() == null) { ASTName name = annotationTypeNode.getFirstChildOfType(ASTName.class); - if (name != null && (name.hasImageEqualTo("Test") || name.hasImageEqualTo(annotationTypeClassName))) { - return true; + if (name != null) { + for (String annotationTypeName : annotationTypeClassNames) { + if (areNamesEqual(name, annotationTypeName)) { + return true; + } + } + } + } else { + for (String annotationTypeClassName : annotationTypeClassNames) { + if (TypeTestUtil.isA(annotationTypeClassName, annotationType)) { + return true; + } } - } else if (TypeTestUtil.isA(annotationTypeClassName, annotationType)) { - return true; } } return false; } private static boolean hasImports(JavaNode node, String className) { + return hasImports(node, Collections.singleton(className)); + } + + private static boolean hasImports(JavaNode node, Set classNames) { List imports = node.getRoot().findDescendantsOfType(ASTImportDeclaration.class); for (ASTImportDeclaration importDeclaration : imports) { ASTName name = importDeclaration.getFirstChildOfType(ASTName.class); - if (name != null && name.hasImageEqualTo(className)) { + // imports are always fully qualified + if (name != null && classNames.contains(name.getImage())) { return true; } } return false; } + + /** + * Compares the name against the given fully qualified name. If a direct comparison doesn't + * work, try to compare as a simple name. + * + *

Note: This method is only used, if the auxclasspath is incomplete.

+ */ + private static boolean areNamesEqual(ASTName node, String fullyQualifiedAnnotationName) { + String simpleName = node.getImage(); + if (simpleName.equals(fullyQualifiedAnnotationName)) { + return true; + } + + String simpleAnnotationName = fullyQualifiedAnnotationName; + int lastDot = fullyQualifiedAnnotationName.lastIndexOf('.'); + if (lastDot != -1) { + simpleAnnotationName = fullyQualifiedAnnotationName.substring(lastDot + 1); + } + // when comparing by simple name, double check whether we have a import to avoid false positives + return simpleName.equals(simpleAnnotationName) && hasImports(node, fullyQualifiedAnnotationName); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java index acce9a621fd..796540b4e1a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java @@ -5,23 +5,35 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import java.util.List; +import java.util.regex.Pattern; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; public class TestClassWithoutTestCasesRule extends AbstractJavaRule { + private static final PropertyDescriptor TEST_CLASS_PATTERN = PropertyFactory.regexProperty("testClassPattern") + .defaultValue("^(.*\\.)?Test.*$|^(.*\\.)?.*Tests?$|^(.*\\.)?.*TestCase$") + .desc("Test class name pattern to identify test classes by their fully qualified name. " + + "A empty pattern disables test class detection by name. Since PMD 6.51.0.") + .build(); + public TestClassWithoutTestCasesRule() { addRuleChainVisit(ASTClassOrInterfaceBody.class); + definePropertyDescriptor(TEST_CLASS_PATTERN); } @Override public Object visit(ASTClassOrInterfaceBody node, Object data) { - if (AbstractJUnitRule.isTestClass(node)) { + if (AbstractJUnitRule.isTestClass(node) || isTestClassByPattern(node)) { List declarations = node.findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class); int testMethods = 0; for (ASTClassOrInterfaceBodyDeclaration decl : declarations) { @@ -36,6 +48,36 @@ public Object visit(ASTClassOrInterfaceBody node, Object data) { return data; } + private boolean isTestClassByPattern(ASTClassOrInterfaceBody node) { + Pattern testClassPattern = getProperty(TEST_CLASS_PATTERN); + if (testClassPattern.pattern().isEmpty()) { + // detection by pattern is disabled + return false; + } + + ASTClassOrInterfaceDeclaration classDecl = null; + if (node.getParent() instanceof ASTClassOrInterfaceDeclaration) { + classDecl = (ASTClassOrInterfaceDeclaration) node.getParent(); + } + + // classDecl can be null in case of anonymous classes or enum constants + if (classDecl == null || classDecl.isAbstract() || classDecl.isInterface()) { + return false; + } + + StringBuilder fullyQualifiedName = new StringBuilder(); + ASTPackageDeclaration packageDeclaration = classDecl.getRoot().getPackageDeclaration(); + if (packageDeclaration != null) { + fullyQualifiedName.append(packageDeclaration.getName()).append('.'); + } + List parentClasses = classDecl.getParentsOfType(ASTClassOrInterfaceDeclaration.class); + for (int i = parentClasses.size() - 1; i >= 0; i--) { + fullyQualifiedName.append(parentClasses.get(i).getSimpleName()).append('.'); + } + fullyQualifiedName.append(classDecl.getSimpleName()); + return testClassPattern.matcher(fullyQualifiedName).find(); + } + private boolean isTestMethod(ASTClassOrInterfaceBodyDeclaration decl) { JavaNode node = decl.getDeclarationNode(); if (node instanceof ASTMethodDeclaration) { diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 77c809b50d6..f8340365efc 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -3336,8 +3336,12 @@ public void foo() { class="net.sourceforge.pmd.lang.java.rule.errorprone.TestClassWithoutTestCasesRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#testclasswithouttestcases"> -Test classes end with the suffix Test. Having a non-test class with that name is not a good practice, -since most people will assume it is a test case. Test classes have test methods named testXXX. +Test classes end with the suffix "Test" or "TestCase". Having a non-test class with that name is not a good practice, +since most people will assume it is a test case. Test classes have test methods named "testXXX" (JUnit3) or use +annotations (e.g. `@Test`). + +The suffix can be configured using the property `testClassPattern`. To disable the detection of possible test classes +by name, set this property to an empty string. 3 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml index 28b7f3d2dd6..e9a6207eae4 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml @@ -5,7 +5,7 @@ xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> - failure case + JUnit3: failure case 1 - test method should be public + JUnit3: test method should be public 1 - inner class should get checked + JUnit3: inner class should get checked 1 2 - test method in inner class not valid + JUnit3: test method in inner class not valid + 1 + 2 - abstract classes are ok + JUnit3: abstract classes are ok 0 - failure case does not extend TestCase + failure case does not extend TestCase and testClassPattern is not used + 0 - #1453 False positive when the test class extends an other. + failure case does not extend TestCase and default testClassPattern + 1 + 1 + + + + + JUnit4: #1453 False positive when the test class extends an other. 0 - #428 [java] PMD requires public modifier on JUnit 5 test + JUnit5: #428 [java] PMD requires public modifier on JUnit 5 test 0 - false positive with anonymous class inside test class + JUnit4: false positive with anonymous class inside test class 0 - [java] TestClassWithoutTestCases reports wrong classes in a file #3624 + JUnit4: [java] TestClassWithoutTestCases reports wrong classes in a file #3624 0 + + + JUnit5: [java] TestClassWithoutTestCases false positive with JUnit5 parameterized test #2636 + 0 + 0); + } +} +]]> + + + + JUnit5: [java] TestClassWithoutTestCases false positive with JUnit5 normal test #2636 + 0 + + + + + empty package-private test class identified by default testClassPattern + 1 + 1 + + + + + empty test class identified by special testClassPattern + my\.pkg\..*Case + 1 + 2 + + + + + empty test class identified by special testClassPattern and nested classes + my\.pkg\..*Case$ + 2 + 2,3 + + + + + empty test class identified by special testClassPattern - other package + my\.pkg\..*Case + 0 + + + + + TestNG based test class without import + 0 + + + + + TestNG based test class with import + 0 + + From 0363c61b77e6d64cac7d006cfce87687a10724ef Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 Oct 2022 14:59:24 +0200 Subject: [PATCH 2/5] [java] TestClassWithoutTestCases - support JUnit5 nested tests --- .../TestClassWithoutTestCasesRule.java | 14 ++++++- .../xml/TestClassWithoutTestCases.xml | 37 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java index 796540b4e1a..c1d16f22250 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java @@ -36,12 +36,15 @@ public Object visit(ASTClassOrInterfaceBody node, Object data) { if (AbstractJUnitRule.isTestClass(node) || isTestClassByPattern(node)) { List declarations = node.findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class); int testMethods = 0; + int nestedTestClasses = 0; for (ASTClassOrInterfaceBodyDeclaration decl : declarations) { if (isTestMethod(decl)) { testMethods++; + } else if (isNestedClass(decl)) { + nestedTestClasses++; } } - if (testMethods == 0) { + if (testMethods == 0 && nestedTestClasses == 0) { addViolation(data, node); } } @@ -85,4 +88,13 @@ private boolean isTestMethod(ASTClassOrInterfaceBodyDeclaration decl) { } return false; } + + private boolean isNestedClass(ASTClassOrInterfaceBodyDeclaration decl) { + JavaNode node = decl.getDeclarationNode(); + if (node instanceof ASTClassOrInterfaceDeclaration) { + ASTClassOrInterfaceDeclaration classDecl = (ASTClassOrInterfaceDeclaration) node; + return classDecl.isAnnotationPresent("org.junit.jupiter.api.Nested"); + } + return false; + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml index e9a6207eae4..337343080ee 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml @@ -273,6 +273,43 @@ public class SampleTest { @Test public void runAssertions() {} } +]]> + + + + JUnit5 @Nested tests with tests + 0 + + + + + JUnit5 @Nested tests without tests + 1 + 6 + From 5860f64648db65755a34729cc500e34e48506a9f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 Oct 2022 15:33:28 +0200 Subject: [PATCH 3/5] [java] TestClassWithoutTestCases - improve nested class support --- .../pmd/lang/java/rule/AbstractJUnitRule.java | 5 +++ .../TestClassWithoutTestCasesRule.java | 9 +++--- .../xml/TestClassWithoutTestCases.xml | 32 ++++++++++++++++--- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java index 4bd020f007a..97e22c45596 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJUnitRule.java @@ -34,6 +34,7 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { protected static final String JUNIT3_CLASS_NAME = "junit.framework.TestCase"; protected static final String JUNIT4_CLASS_NAME = "org.junit.Test"; protected static final String JUNIT5_CLASS_NAME = "org.junit.jupiter.api.Test"; + private static final String JUNIT5_NESTED = "org.junit.jupiter.api.Nested"; private static final Set JUNIT5_TEST_ANNOTATIONS = new HashSet<>(Arrays.asList( JUNIT5_CLASS_NAME, "org.junit.jupiter.api.RepeatedTest", @@ -161,6 +162,10 @@ public static boolean isTestMethod(ASTMethodDeclaration method) { || doesNodeContainJUnitAnnotation(method.getParent(), TESTNG_ANNOTATION); } + public static boolean isJUnit5NestedClass(ASTClassOrInterfaceBody innerClassDecl) { + return doesNodeContainJUnitAnnotation((JavaNode) innerClassDecl.getNthParent(2), JUNIT5_NESTED); + } + public boolean isJUnitMethod(ASTMethodDeclaration method, Object data) { if (method.isAbstract() || method.isNative() || method.isStatic()) { return false; // skip various inapplicable method variations diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java index c1d16f22250..bbb869a3a19 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java @@ -33,7 +33,7 @@ public TestClassWithoutTestCasesRule() { @Override public Object visit(ASTClassOrInterfaceBody node, Object data) { - if (AbstractJUnitRule.isTestClass(node) || isTestClassByPattern(node)) { + if (AbstractJUnitRule.isTestClass(node) || AbstractJUnitRule.isJUnit5NestedClass(node) || isTestClassByPattern(node)) { List declarations = node.findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class); int testMethods = 0; int nestedTestClasses = 0; @@ -90,10 +90,9 @@ private boolean isTestMethod(ASTClassOrInterfaceBodyDeclaration decl) { } private boolean isNestedClass(ASTClassOrInterfaceBodyDeclaration decl) { - JavaNode node = decl.getDeclarationNode(); - if (node instanceof ASTClassOrInterfaceDeclaration) { - ASTClassOrInterfaceDeclaration classDecl = (ASTClassOrInterfaceDeclaration) node; - return classDecl.isAnnotationPresent("org.junit.jupiter.api.Nested"); + JavaNode node = decl.getParent(); + if (node instanceof ASTClassOrInterfaceBody) { + return AbstractJUnitRule.isJUnit5NestedClass((ASTClassOrInterfaceBody) node); } return false; } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml index 337343080ee..473214a122b 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml @@ -230,7 +230,7 @@ class MyEmptyCase { } - empty test class identified by special testClassPattern and nested classes + empty test class identified by special testClassPattern with nested classes my\.pkg\..*Case$ 2 2,3 @@ -284,7 +284,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertTrue; -class DemoNestedTest { +class DemoNestedTests { @Nested class SomeTest { @Test @@ -298,17 +298,39 @@ class DemoNestedTest { JUnit5 @Nested tests without tests - 1 - 6 + 2 + 6,11 + + + + Ignore other nested classes that are not test classes + 1 + 6 + From dd26af7f9317c65d0875c9f788bb7bb365db22ba Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 Oct 2022 15:47:07 +0200 Subject: [PATCH 4/5] [java] TestClassWithoutTestCases - improve message --- .../rule/errorprone/TestClassWithoutTestCasesRule.java | 10 +++++++++- .../src/main/resources/category/java/errorprone.xml | 8 ++++---- .../rule/errorprone/xml/TestClassWithoutTestCases.xml | 8 ++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java index bbb869a3a19..0f79e7b0ba3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java @@ -45,12 +45,20 @@ public Object visit(ASTClassOrInterfaceBody node, Object data) { } } if (testMethods == 0 && nestedTestClasses == 0) { - addViolation(data, node); + addViolation(data, node, getSimpleClassName(node)); } } return data; } + private String getSimpleClassName(ASTClassOrInterfaceBody node) { + JavaNode parent = node.getParent(); + if (parent instanceof ASTClassOrInterfaceDeclaration) { + return ((ASTClassOrInterfaceDeclaration) parent).getSimpleName(); + } + return ""; + } + private boolean isTestClassByPattern(ASTClassOrInterfaceBody node) { Pattern testClassPattern = getProperty(TEST_CLASS_PATTERN); if (testClassPattern.pattern().isEmpty()) { diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index f8340365efc..079f2929290 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -3332,13 +3332,13 @@ public void foo() { -Test classes end with the suffix "Test" or "TestCase". Having a non-test class with that name is not a good practice, -since most people will assume it is a test case. Test classes have test methods named "testXXX" (JUnit3) or use -annotations (e.g. `@Test`). +Test classes typically end with the suffix "Test", "Tests" or "TestCase". Having a non-test class with that name +is not a good practice, since most people will assume it is a test case. Test classes have test methods +named "testXXX" (JUnit3) or use annotations (e.g. `@Test`). The suffix can be configured using the property `testClassPattern`. To disable the detection of possible test classes by name, set this property to an empty string. diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml index 473214a122b..839c617ddf0 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml @@ -7,6 +7,10 @@ JUnit3: failure case 1 + 2 + + The class 'FooTest' might be a test class, but it contains no test cases. + my\.pkg\..*Case$ 2 2,3 + + The class 'MyEmptyCase' might be a test class, but it contains no test cases. + The class 'MyNestedCase' might be a test class, but it contains no test cases. + Date: Fri, 21 Oct 2022 09:29:06 +0200 Subject: [PATCH 5/5] [java] TestClassWithoutTestCases - fix default pattern To not match inner classes or a potential test class if the inner class is not named test. --- .../TestClassWithoutTestCasesRule.java | 2 +- .../xml/TestClassWithoutTestCases.xml | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java index 0f79e7b0ba3..85e94fef10e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/TestClassWithoutTestCasesRule.java @@ -21,7 +21,7 @@ public class TestClassWithoutTestCasesRule extends AbstractJavaRule { private static final PropertyDescriptor TEST_CLASS_PATTERN = PropertyFactory.regexProperty("testClassPattern") - .defaultValue("^(.*\\.)?Test.*$|^(.*\\.)?.*Tests?$|^(.*\\.)?.*TestCase$") + .defaultValue("^(?:.*\\.)?Test[^\\.]*$|^(?:.*\\.)?.*Tests?$|^(?:.*\\.)?.*TestCase$") .desc("Test class name pattern to identify test classes by their fully qualified name. " + "A empty pattern disables test class detection by name. Since PMD 6.51.0.") .build(); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml index 839c617ddf0..1ede8353eb9 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/TestClassWithoutTestCases.xml @@ -340,6 +340,25 @@ class SampleTest { private static class Other {} } +]]> + + + + Default pattern shouldn't match inner classes that are not named test + 2 + 2,9 +