Merge branch 'pr-435'

This commit is contained in:
Juan Martín Sotuyo Dodero
2017-06-12 19:21:05 -03:00
5 changed files with 84 additions and 23 deletions

View File

@ -23,32 +23,40 @@ 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");
} catch (ClassNotFoundException t) {
c = Class.forName(JUNIT3_CLASS_NAME);
} catch (ClassNotFoundException | NoClassDefFoundError t) {
c = null;
} catch (NoClassDefFoundError t) {
}
JUNIT3_CLASS = c;
try {
c = Class.forName(JUNIT4_CLASS_NAME);
} catch (ClassNotFoundException | NoClassDefFoundError t) {
c = null;
}
JUNIT4_CLASS = c;
try {
c = Class.forName("junit.framework.TestCase");
} catch (ClassNotFoundException t) {
c = null;
} catch (NoClassDefFoundError t) {
c = Class.forName(JUNIT5_CLASS_NAME);
} catch (ClassNotFoundException | NoClassDefFoundError t) {
c = null;
}
JUNIT3_CLASS = c;
JUNIT5_CLASS = c;
}
@Override
@ -56,13 +64,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 +93,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 +129,14 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule {
}
private boolean isJUnit3Class(ASTCompilationUnit node) {
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;
}
if (node.getType() != null && TypeHelper.isA(node, JUNIT3_CLASS)) {
return true;
} else if (node.getType() == null) {
ASTExtendsList extendsList = cid.getFirstChildOfType(ASTExtendsList.class);
if (extendsList == null) {
return false;
@ -123,25 +146,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<ASTAnnotation> 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<ASTImportDeclaration> imports = cu.findDescendantsOfType(ASTImportDeclaration.class);
for (ASTImportDeclaration importDeclaration : imports) {
ASTName name = importDeclaration.getFirstChildOfType(ASTName.class);
if (name != null && name.hasImageEqualTo(className)) {
return true;
}
}

View File

@ -332,6 +332,7 @@ public class BaseConsoleTest extends UART {
<description>#330 NPE accessing local field / method with this.XXX</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import org.junit.Test;
public class Foo {
@Test
public void testName() {

View File

@ -113,6 +113,20 @@ public class MyTest extends YourTest {
public void falseMethod() {
assertFalse(0 == 1);
}
}
]]></code>
</test-code>
<test-code>
<description>#428 [java] PMD requires public modifier on JUnit 5 test</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.jupiter.api.Test;
class JUnit5Test {
@Test
void myTest() {
}
}
]]></code>
</test-code>

View File

@ -7,6 +7,7 @@ JUnit 4 - Contains assert
<expected-problems>1</expected-problems>
<code><![CDATA[
import java.lang.Thread;
import org.junit.Test;
public class Foo {
@Test
public void foo() throws Throwable {

View File

@ -25,6 +25,9 @@ This is a minor release.
This change may introduce some false positives if using the exception in non-orthodox ways for things other than setting the
root cause of the exception. Contact us if you find any such scenarios.
* 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
@ -38,6 +41,8 @@ This is a minor release.
* [#397](https://github.com/pmd/pmd/issues/397): \[java] ConstructorCallsOverridableMethodRule: false positive for method called from lambda expression
* [#410](https://github.com/pmd/pmd/issues/410): \[java] ImmutableField: False positive with lombok
* [#422](https://github.com/pmd/pmd/issues/422): \[java] PreserveStackTraceRule: false positive when using builder pattern
* java-junit
* [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test
* java-unnecessary
* [#421](https://github.com/pmd/pmd/issues/421): \[java] UnnecessaryFinalModifier final in private method