pmd: fix #968 Issues with JUnit4 @Test annotation with expected exception

Thanks to Yiannis Paschalidis
This commit is contained in:
Andreas Dangel
2013-03-17 09:56:02 +01:00
parent 040d465137
commit 41c44e5d8d
8 changed files with 90 additions and 21 deletions

View File

@ -1,5 +1,6 @@
????? ??, 2013 - 5.0.3:
Fixed bug 968: Issues with JUnit4 @Test annotation with expected exception (Thanks to Yiannis Paschalidis)
Fixed bug 976: UselessStringValueOf wrong when appending character arrays
Fixed bug 977: MisplacedNullCheck makes false positives
Fixed bug 984: Cyclomatic complexity should treat constructors like methods

View File

@ -2,7 +2,7 @@
package net.sourceforge.pmd.lang.java.ast;
public class ASTNormalAnnotation extends AbstractJavaNode {
public class ASTNormalAnnotation extends AbstractJavaTypeNode {
public ASTNormalAnnotation(int id) {
super(id);
}

View File

@ -2,7 +2,7 @@
package net.sourceforge.pmd.lang.java.ast;
public class ASTSingleMemberAnnotation extends AbstractJavaNode {
public class ASTSingleMemberAnnotation extends AbstractJavaTypeNode {
public ASTSingleMemberAnnotation(int id) {
super(id);
}

View File

@ -3,20 +3,19 @@ package net.sourceforge.pmd.lang.java.rule.junit;
import java.util.List;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTExtendsList;
import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters;
import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper;
@SuppressWarnings("PMD.AvoidCatchingThrowable")
// Don't think we can otherwise here...
public abstract class AbstractJUnitRule extends AbstractJavaRule {
public static final Class<?> JUNIT4_CLASS;
@ -30,14 +29,14 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule {
Class<?> c;
try {
c = Class.forName("org.junit.Test");
} catch (Throwable t) {
} catch (ClassNotFoundException t) {
c = null;
}
JUNIT4_CLASS = c;
try {
c = Class.forName("junit.framework.TestCase");
} catch (Throwable t) {
} catch (ClassNotFoundException t) {
c = null;
}
JUNIT3_CLASS = c;
@ -111,18 +110,19 @@ public abstract class AbstractJUnitRule extends AbstractJavaRule {
}
private boolean doesNodeContainJUnitAnnotation(Node node) {
List<ASTMarkerAnnotation> lstAnnotations = node.findDescendantsOfType(ASTMarkerAnnotation.class);
for (ASTMarkerAnnotation annotation : lstAnnotations) {
if (annotation.getType() == null) {
ASTName name = (ASTName) annotation.jjtGetChild(0);
if ("Test".equals(name.getImage())) {
return true;
}
} else if (annotation.getType().equals(JUNIT4_CLASS)) {
return true;
}
}
return false;
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())) {
return true;
}
} else if (annotationType.getType().equals(JUNIT4_CLASS)) {
return true;
}
}
return false;
}
}

View File

@ -3,10 +3,14 @@
*/
package net.sourceforge.pmd.lang.java.rule.junit;
import java.util.List;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTNormalAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
@ -24,7 +28,8 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
@Override
public Object visit(ASTMethodDeclaration method, Object data) {
if (isJUnitMethod(method, data)) {
if (!containsAssert(method.getBlock(), false)) {
if (!containsAssert(method.getBlock(), false)
&& !containsExpect(method.jjtGetParent())) {
addViolation(data, method);
}
}
@ -50,6 +55,26 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
return false;
}
/**
* Tells if the node contains a Test annotation with an expected exception.
*/
private boolean containsExpect(Node methodParent) {
List<ASTNormalAnnotation> annotations = methodParent.findDescendantsOfType(ASTNormalAnnotation.class);
for (ASTNormalAnnotation annotation : annotations) {
ASTName name = annotation.getFirstChildOfType(ASTName.class);
if (name != null && ("Test".equals(name.getImage())
|| (name.getType() != null && name.getType().equals(JUNIT4_CLASS)))) {
List<ASTMemberValuePair> memberValues = annotation.findDescendantsOfType(ASTMemberValuePair.class);
for (ASTMemberValuePair pair : memberValues) {
if ("expected".equals(pair.getImage())) {
return true;
}
}
}
}
return false;
}
/**
* Tells if the expression is an assert statement or not.
*/

View File

@ -35,8 +35,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTInclusiveOrExpression;
import net.sourceforge.pmd.lang.java.ast.ASTInstanceOfExpression;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTMultiplicativeExpression;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTNormalAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTPostfixExpression;
@ -49,6 +51,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression;
import net.sourceforge.pmd.lang.java.ast.ASTShiftExpression;
import net.sourceforge.pmd.lang.java.ast.ASTSingleMemberAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration;
@ -542,6 +545,27 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
return data;
}
@Override
public Object visit(ASTNormalAnnotation node, Object data) {
super.visit(node, data);
rollupTypeUnary(node);
return data;
}
@Override
public Object visit(ASTMarkerAnnotation node, Object data) {
super.visit(node, data);
rollupTypeUnary(node);
return data;
}
@Override
public Object visit(ASTSingleMemberAnnotation node, Object data) {
super.visit(node, data);
rollupTypeUnary(node);
return data;
}
// Roll up the type based on type of the first child node.
private void rollupTypeUnary(TypeNode typeNode) {
if (typeNode instanceof Node) {

View File

@ -333,6 +333,7 @@
<li>Roman - Fixed bug 3546093: Type resolution very slow for big project.</li>
<li>Florian Bauer - Add C# support for CPD.</li>
<li>Matthew Short - Support in CPD for IgnoreAnnotations and SuppressWarnings("CPD-START").</li>
<li>Yiannis Paschalidis - Fixed bug #968 Issues with JUnit4 @Test annotation with expected exception</li>
</ul>
</subsection>
<subsection name="Organizations">

View File

@ -253,4 +253,22 @@ public class AssertTest {
}
]]></code>
</test-code>
<test-code>
<description>#968 Issues with JUnit4 @Test annotation with expected exception</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.Test;
public class Foo_Test
{
/** Tests that doSomething throws an exception if arg is negative */
@Test(expected = IllegalArgumentException.class)
public void testDoSomethingFail()
{
new Foo().doSomething(-1);
// Note - no assert is needed as we expect an exception (which is basically an assert).
}
}
]]></code>
</test-code>
</test-data>