From 0fc9ac185a3212e5262c0c16455901a7e38ec4f9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sun, 11 Jun 2017 10:14:23 +0200 Subject: [PATCH 1/3] [java] PMD requires public modifier on JUnit 5 test (fixes #428) --- .../java/rule/junit/AbstractJUnitRule.java | 82 +++++++++++++++---- .../xml/JUnitTestsShouldIncludeAssert.xml | 1 + .../junit/xml/TestClassWithoutTestCases.xml | 14 ++++ .../rule/migrating/xml/JUnitUseExpected.xml | 1 + src/site/markdown/overview/changelog.md | 2 + 5 files changed, 82 insertions(+), 18 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java index 6eacf5fb1a..253d7db564 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java @@ -23,17 +23,31 @@ import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; public abstract class AbstractJUnitRule extends AbstractJavaRule { - public static final Class JUNIT4_CLASS; - public static final Class JUNIT3_CLASS; + private static final String JUNIT3_CLASS_NAME = "junit.framework.TestCase"; + public static final Class JUNIT4_CLASS; + private static final String JUNIT4_CLASS_NAME = "org.junit.Test"; + public static final Class JUNIT5_CLASS; + private static final String JUNIT5_CLASS_NAME = "org.junit.jupiter.api.Test"; private boolean isJUnit3Class; private boolean isJUnit4Class; + private boolean isJUnit5Class; static { Class c; + try { - c = Class.forName("org.junit.Test"); + c = Class.forName(JUNIT3_CLASS_NAME); + } catch (ClassNotFoundException t) { + c = null; + } catch (NoClassDefFoundError t) { + c = null; + } + JUNIT3_CLASS = c; + + try { + c = Class.forName(JUNIT4_CLASS_NAME); } catch (ClassNotFoundException t) { c = null; } catch (NoClassDefFoundError t) { @@ -42,13 +56,13 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { JUNIT4_CLASS = c; try { - c = Class.forName("junit.framework.TestCase"); + c = Class.forName(JUNIT5_CLASS_NAME); } catch (ClassNotFoundException t) { c = null; } catch (NoClassDefFoundError t) { c = null; } - JUNIT3_CLASS = c; + JUNIT5_CLASS = c; } @Override @@ -56,13 +70,19 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { isJUnit3Class = false; isJUnit4Class = false; + isJUnit5Class = false; isJUnit3Class = isJUnit3Class(node); if (!isJUnit3Class) { isJUnit4Class = isJUnit4Class(node); + isJUnit5Class = isJUnit5Class(node); + } + if (isJUnit4Class && isJUnit5Class) { + isJUnit4Class &= hasImports(node, JUNIT4_CLASS_NAME); + isJUnit5Class &= hasImports(node, JUNIT5_CLASS_NAME); } - if (!isTestNgClass(node) && (isJUnit3Class || isJUnit4Class)) { + if (!isTestNgClass(node) && (isJUnit3Class || isJUnit4Class || isJUnit5Class)) { return super.visit(node, data); } return data; @@ -79,22 +99,31 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { } public boolean isJUnitMethod(ASTMethodDeclaration method, Object data) { - - if (!method.isPublic() || method.isAbstract() || method.isNative() || method.isStatic()) { + if (method.isAbstract() || method.isNative() || method.isStatic()) { return false; // skip various inapplicable method variations } + if (!isJUnit5Class && !method.isPublic()) { + // junit5 class doesn't require test methods to be public anymore + return false; + } + boolean result = false; if (isJUnit3Class) { result = isJUnit3Method(method); } result |= isJUnit4Method(method); + result |= isJUnit5Method(method); return result; } private boolean isJUnit4Method(ASTMethodDeclaration method) { - return doesNodeContainJUnitAnnotation(method.jjtGetParent()); + return doesNodeContainJUnitAnnotation(method.jjtGetParent(), JUNIT4_CLASS, JUNIT4_CLASS_NAME); + } + + private boolean isJUnit5Method(ASTMethodDeclaration method) { + return doesNodeContainJUnitAnnotation(method.jjtGetParent(), JUNIT5_CLASS, JUNIT5_CLASS_NAME); } private boolean isJUnit3Method(ASTMethodDeclaration method) { @@ -106,14 +135,14 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { } private boolean isJUnit3Class(ASTCompilationUnit node) { + ASTClassOrInterfaceDeclaration cid = node.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class); + if (cid == null) { + return false; + } + if (node.getType() != null && TypeHelper.isA(node, JUNIT3_CLASS)) { return true; - } else if (node.getType() == null) { - ASTClassOrInterfaceDeclaration cid = node.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class); - if (cid == null) { - return false; - } ASTExtendsList extendsList = cid.getFirstChildOfType(ASTExtendsList.class); if (extendsList == null) { return false; @@ -123,25 +152,42 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { } String className = cid.getImage(); return className.endsWith("Test"); + } else if (hasImports(node, JUNIT3_CLASS_NAME)) { + return cid.getImage().endsWith("Test"); } return false; } private boolean isJUnit4Class(ASTCompilationUnit node) { - return doesNodeContainJUnitAnnotation(node); + return doesNodeContainJUnitAnnotation(node, JUNIT4_CLASS, JUNIT4_CLASS_NAME); } - private boolean doesNodeContainJUnitAnnotation(Node node) { + private boolean isJUnit5Class(ASTCompilationUnit node) { + return doesNodeContainJUnitAnnotation(node, JUNIT5_CLASS, JUNIT5_CLASS_NAME); + } + + private boolean doesNodeContainJUnitAnnotation(Node node, Class annotationTypeClass, String annotationTypeClassName) { List annotations = node.findDescendantsOfType(ASTAnnotation.class); for (ASTAnnotation annotation : annotations) { Node annotationTypeNode = annotation.jjtGetChild(0); TypeNode annotationType = (TypeNode) annotationTypeNode; if (annotationType.getType() == null) { ASTName name = annotationTypeNode.getFirstChildOfType(ASTName.class); - if (name != null && "Test".equals(name.getImage())) { + if (name != null && (name.hasImageEqualTo("Test") || name.hasImageEqualTo(annotationTypeClassName))) { return true; } - } else if (annotationType.getType().equals(JUNIT4_CLASS)) { + } else if (annotationType.getType().equals(annotationTypeClass)) { + return true; + } + } + return false; + } + + private boolean hasImports(ASTCompilationUnit cu, String className) { + List imports = cu.findDescendantsOfType(ASTImportDeclaration.class); + for (ASTImportDeclaration importDeclaration : imports) { + ASTName name = importDeclaration.getFirstChildOfType(ASTName.class); + if (name != null && name.hasImageEqualTo(className)) { return true; } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml index 8f4f956c1a..61f5047edc 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml @@ -332,6 +332,7 @@ public class BaseConsoleTest extends UART { #330 NPE accessing local field / method with this.XXX 1 + + + #428 [java] PMD requires public modifier on JUnit 5 test + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/migrating/xml/JUnitUseExpected.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/migrating/xml/JUnitUseExpected.xml index f62b265827..b02335e603 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/migrating/xml/JUnitUseExpected.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/migrating/xml/JUnitUseExpected.xml @@ -7,6 +7,7 @@ JUnit 4 - Contains assert 1 Date: Mon, 12 Jun 2017 19:11:15 -0300 Subject: [PATCH 2/3] Prefer multicatch for brevity --- .../pmd/lang/java/rule/junit/AbstractJUnitRule.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java index 253d7db564..a804ae06df 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/AbstractJUnitRule.java @@ -39,27 +39,21 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule { try { c = Class.forName(JUNIT3_CLASS_NAME); - } catch (ClassNotFoundException t) { - c = null; - } catch (NoClassDefFoundError t) { + } catch (ClassNotFoundException | NoClassDefFoundError t) { c = null; } JUNIT3_CLASS = c; try { c = Class.forName(JUNIT4_CLASS_NAME); - } catch (ClassNotFoundException t) { - c = null; - } catch (NoClassDefFoundError t) { + } catch (ClassNotFoundException | NoClassDefFoundError t) { c = null; } JUNIT4_CLASS = c; try { c = Class.forName(JUNIT5_CLASS_NAME); - } catch (ClassNotFoundException t) { - c = null; - } catch (NoClassDefFoundError t) { + } catch (ClassNotFoundException | NoClassDefFoundError t) { c = null; } JUNIT5_CLASS = c; From 53687afeb81d7c2875bdd38f05eda99d84b7bdae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 12 Jun 2017 19:20:07 -0300 Subject: [PATCH 3/3] Update changelog, include changed rules info --- src/site/markdown/overview/changelog.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index a05288d10c..c2d0fae802 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -9,12 +9,18 @@ This is a minor release. ### Table Of Contents * [New and noteworthy](#New_and_noteworthy) + * [Modified Rules](#Modified_Rules) * [Fixed Issues](#Fixed_Issues) * [API Changes](#API_Changes) * [External Contributions](#External_Contributions) ### New and noteworthy +#### Modified Rules + +* The ruleset java-junit now properly detects JUnit5, and rules are being adapted to the changes on it's API. + This support is, however, still incomplete. Let us know of any uses we are still missing on the [issue tracker](https://github.com/pmd/pmd/issues) + ### Fixed Issues * General