phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'pr-308' into pmd/5.5.x

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-03-26 23:58:19 -03:00
parent 74b6c71025 c32f12e09f
commit 0c3a72fa9e
3 changed files with 207 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

261
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/junit/JUnitTestsShouldIncludeAssertRule.java
View File

@ -1,98 +1,163 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
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;
public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (node.isInterface()) {
return data;
}
return super.visit(node, data);
}
@Override
public Object visit(ASTMethodDeclaration method, Object data) {
if (isJUnitMethod(method, data)) {
if (!containsAssert(method.getBlock(), false) && !containsExpect(method.jjtGetParent())) {
addViolation(data, method);
}
}
return data;
}
private boolean containsAssert(Node n, boolean assertFound) {
if (!assertFound) {
if (n instanceof ASTStatementExpression) {
if (isAssertOrFailStatement((ASTStatementExpression) n)) {
return true;
}
}
if (!assertFound) {
for (int i = 0; i < n.jjtGetNumChildren() && !assertFound; i++) {
Node c = n.jjtGetChild(i);
if (containsAssert(c, assertFound)) {
return true;
}
}
}
}
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.
*/
private boolean isAssertOrFailStatement(ASTStatementExpression expression) {
if (expression != null && expression.jjtGetNumChildren() > 0
&& expression.jjtGetChild(0) instanceof ASTPrimaryExpression) {
ASTPrimaryExpression pe = (ASTPrimaryExpression) expression.jjtGetChild(0);
if (pe.jjtGetNumChildren() > 0 && pe.jjtGetChild(0) instanceof ASTPrimaryPrefix) {
ASTPrimaryPrefix pp = (ASTPrimaryPrefix) pe.jjtGetChild(0);
if (pp.jjtGetNumChildren() > 0 && pp.jjtGetChild(0) instanceof ASTName) {
String img = ((ASTName) pp.jjtGetChild(0)).getImage();
if (img != null
&& (img.startsWith("assert") || img.startsWith("fail") || img.startsWith("Assert.assert") || img
.startsWith("Assert.fail"))) {
return true;
}
}
}
}
return false;
}
}
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.junit;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
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.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
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.ASTReferenceType;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.lang.symboltable.Scope;
public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (node.isInterface()) {
return data;
}
return super.visit(node, data);
}
@Override
public Object visit(ASTMethodDeclaration method, Object data) {
if (isJUnitMethod(method, data)) {
if (!isExpectAnnotated(method.jjtGetParent())) {
Scope classScope = method.getScope().getParent();
Map<String, List<NameOccurrence>> expectables = getRuleAnnotatedExpectedExceptions(classScope);
if (!containsExpectOrAssert(method.getBlock(), expectables)) {
addViolation(data, method);
}
}
}
return data;
}
private boolean containsExpectOrAssert(Node n, Map<String, List<NameOccurrence>> expectables) {
if (n instanceof ASTStatementExpression) {
if (isExpectStatement((ASTStatementExpression) n, expectables)
|| isAssertOrFailStatement((ASTStatementExpression) n)) {
return true;
}
} else {
for (int i = 0; i < n.jjtGetNumChildren(); i++) {
Node c = n.jjtGetChild(i);
if (containsExpectOrAssert(c, expectables)) {
return true;
}
}
}
return false;
}
/**
* Gets a list of NameDeclarations for all the fields that have type
* ExpectedException and have a Rule annotation.
*
* @param classScope
* The class scope to search for
* @return See description
*/
private Map<String, List<NameOccurrence>> getRuleAnnotatedExpectedExceptions(Scope classScope) {
Map<String, List<NameOccurrence>> result = new HashMap<>();
Map<NameDeclaration, List<NameOccurrence>> decls = classScope.getDeclarations();
for (NameDeclaration decl : decls.keySet()) {
Node parent = decl.getNode().jjtGetParent().jjtGetParent().jjtGetParent();
if (parent.hasDescendantOfType(ASTAnnotation.class)
&& parent.getFirstChildOfType(ASTFieldDeclaration.class) != null) {
String annot = parent.getFirstDescendantOfType(ASTMarkerAnnotation.class).jjtGetChild(0).getImage();
if (!"Rule".equals(annot) && !"org.junit.Rule".equals(annot)) {
continue;
}
Node type = parent.getFirstDescendantOfType(ASTReferenceType.class);
if (!"ExpectedException".equals(type.jjtGetChild(0).getImage())) {
continue;
}
result.put(decl.getName(), decls.get(decl));
}
}
return result;
}
/**
* Tells if the node contains a Test annotation with an expected exception.
*/
private boolean isExpectAnnotated(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.
*/
private boolean isAssertOrFailStatement(ASTStatementExpression expression) {
if (expression != null) {
ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class);
if (pe != null) {
String img = pe.jjtGetChild(0).jjtGetChild(0).getImage();
if (img != null && (img.startsWith("assert") || img.startsWith("fail")
|| img.startsWith("Assert.assert") || img.startsWith("Assert.fail"))) {
return true;
}
}
}
return false;
}
private boolean isExpectStatement(ASTStatementExpression expression,
Map<String, List<NameOccurrence>> expectables) {
if (expression != null) {
ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class);
if (pe != null) {
String img = pe.jjtGetChild(0).jjtGetChild(0).getImage();
if (img.indexOf(".") == -1) {
return false;
}
String varname = img.split("\\.")[0];
if (!expectables.containsKey(varname)) {
return false;
}
for (NameOccurrence occ : expectables.get(varname)) {
if (occ.getLocation() == pe.jjtGetChild(0).jjtGetChild(0) && img.startsWith(varname + ".expect")) {
return true;
}
}
}
}
return false;
}
}

41
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/junit/xml/JUnitTestsShouldIncludeAssert.xml
View File

@ -271,4 +271,45 @@ public class Foo_Test
}
]]></code>
</test-code>
<test-code>
<description>#285 Issues with @Rule annotation and ExpectedException</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import org.junit.*;
public class SimpleExpectedExceptionTest {
@Rule
public ExpectedException thrown = ExpectedException.none();
@Test
public void throwsExceptionWithSpecificType() {
throw new NullPointerException(); // No expect! this is a violation
}
@Test
public void throwsIllegalArgumentExceptionIfIconIsNull() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Icon is null, not a file, or doesn't exist.");
new DigitalAssetManager(null, null);
}
}
]]></code>
</test-code>
<test-code>
<description>#285 Follow-up for @org.junit.Rule</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.*;
public class SimpleExpectedExceptionTest {
@org.junit.Rule
public ExpectedException thrown = ExpectedException.none();
@Test
public void throwsExceptionWithSpecificType() {
thrown.expect(NullPointerException.class);
throw new NullPointerException();
}
}
]]></code>
</test-code>
</test-data>

3
src/site/markdown/overview/changelog.md
View File

@ -23,6 +23,8 @@ The PMD team is pleased to announce PMD 5.5.5.
* [#275](https://github.com/pmd/pmd/issues/275): \[java] FinalFieldCouldBeStatic: Constant in @interface incorrectly reported as "could be made static"
* [#282](https://github.com/pmd/pmd/issues/282): \[java] UnnecessaryLocalBeforeReturn false positive when cloning Maps
* [#291](https://github.com/pmd/pmd/issues/291): \[java] Improve quality of AccessorClassGeneration
* java-junit
* [#285](https://github.com/pmd/pmd/issues/285): \[java] JUnitTestsShouldIncludeAssertRule should support @Rule as well as @Test(expected = ...)
* java-optimizations:
* [#222](https://github.com/pmd/pmd/issues/222): \[java] UseStringBufferForStringAppends: False Positive with ternary operator
* java-strings:
@ -37,4 +39,5 @@ The PMD team is pleased to announce PMD 5.5.5.
* [#296](https://github.com/pmd/pmd/pull/296): \[apex] Adding String.IsNotBlank to the whitelist to prevent False positives
* [#303](https://github.com/pmd/pmd/pull/303): \[java] InefficientEmptyStringCheckRule now reports String.trim().isEmpty()
* [#307](https://github.com/pmd/pmd/pull/307): \[java] Fix false positive with UseStringBufferForStringAppendsRule
* [#308](https://github.com/pmd/pmd/pull/308): \[java] JUnitTestsShouldIncludeAssertRule supports @Rule annotated ExpectedExceptions