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

Make AssertJ soft assertions count as assertions

Browse Source
This commit is contained in:
ledoyen
2018-11-12 10:47:18 +01:00
parent a5483b36e4
commit cc8181edc9
5 changed files with 129 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
pmd-java/pom.xml
View File

@ -176,5 +176,10 @@
<artifactId>ant-testutil</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>

52
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java
View File

@ -20,6 +20,7 @@ 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.java.rule.AbstractJUnitRule;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
@ -39,10 +40,12 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
public Object visit(ASTMethodDeclaration method, Object data) {
if (isJUnitMethod(method, data)) {
if (!isExpectAnnotated(method.jjtGetParent())) {
Map<String, VariableNameDeclaration> variables = getVariables(method);
Scope classScope = method.getScope().getParent();
Map<String, List<NameOccurrence>> expectables = getRuleAnnotatedExpectedExceptions(classScope);
if (!containsExpectOrAssert(method.getBlock(), expectables)) {
if (!containsExpectOrAssert(method.getBlock(), expectables, variables)) {
addViolation(data, method);
}
}
@ -50,24 +53,35 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
return data;
}
private boolean containsExpectOrAssert(Node n, Map<String, List<NameOccurrence>> expectables) {
private boolean containsExpectOrAssert(Node n,
Map<String, List<NameOccurrence>> expectables,
Map<String, VariableNameDeclaration> variables) {
if (n instanceof ASTStatementExpression) {
if (isExpectStatement((ASTStatementExpression) n, expectables)
|| isAssertOrFailStatement((ASTStatementExpression) n)
|| isVerifyStatement((ASTStatementExpression) n)) {
|| isVerifyStatement((ASTStatementExpression) n)
|| isSoftAssertionStatement((ASTStatementExpression) n, variables)) {
return true;
}
} else {
for (int i = 0; i < n.jjtGetNumChildren(); i++) {
Node c = n.jjtGetChild(i);
if (containsExpectOrAssert(c, expectables)) {
if (containsExpectOrAssert(c, expectables, variables)) {
return true;
}
}
}
return false;
}
private Map<String, VariableNameDeclaration> getVariables(ASTMethodDeclaration method) {
Map<String, VariableNameDeclaration> variables = new HashMap<>();
for (VariableNameDeclaration vnd : method.getScope().getDeclarations(VariableNameDeclaration.class).keySet()) {
variables.put(vnd.getName(), vnd);
}
return variables;
}
/**
* Gets a list of NameDeclarations for all the fields that have type
* ExpectedException and have a Rule annotation.
@ -189,4 +203,30 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
}
return false;
}
private boolean isSoftAssertionStatement(ASTStatementExpression expression,
Map<String, VariableNameDeclaration> variables) {
if (expression != null) {
ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class);
if (pe != null) {
Node name = pe.getFirstDescendantOfType(ASTName.class);
if (name != null) {
String img = name.getImage();
if (img.indexOf(".") == -1) {
return false;
}
String[] tokens = img.split("\\.");
String methodName = tokens[1];
boolean methodIsAssertAll = "assertAll".equals(methodName);
String varName = tokens[0];
boolean variableTypeIsSoftAssertion = variables.containsKey(varName)
&& TypeHelper.isA(variables.get(varName), "org.assertj.core.api.AbstractSoftAssertions");
return methodIsAssertAll && variableTypeIsSoftAssertion;
}
}
}
return false;
}
}

47
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java
View File

@ -54,23 +54,29 @@ public final class TypeHelper {
private static Class<?> loadClassWithNodeClassloader(final TypeNode n, final String clazzName) {
if (n.getType() != null) {
try {
ClassLoader classLoader = n.getType().getClassLoader();
if (classLoader == null) {
// Using the system classloader then
classLoader = ClassLoader.getSystemClassLoader();
}
// If the requested type is in the classpath, using the same classloader should work
return ClassUtils.getClass(classLoader, clazzName);
} catch (final ClassNotFoundException ignored) {
// The requested type is not on the auxclasspath. This might happen, if the type node
// is probed for a specific type (e.g. is is a JUnit5 Test Annotation class).
// Failing to resolve clazzName does not necessarily indicate an incomplete auxclasspath.
} catch (final LinkageError expected) {
// We found the class but it's invalid / incomplete. This may be an incomplete auxclasspath
// if it was a NoClassDefFoundError. TODO : Report it?
return loadClass(n.getType().getClassLoader(), clazzName);
}
return null;
}
private static Class<?> loadClass(final ClassLoader nullableClassLoader, final String clazzName) {
try {
ClassLoader classLoader = nullableClassLoader;
if (classLoader == null) {
// Using the system classloader then
classLoader = ClassLoader.getSystemClassLoader();
}
// If the requested type is in the classpath, using the same classloader should work
return ClassUtils.getClass(classLoader, clazzName);
} catch (ClassNotFoundException ignored) {
// The requested type is not on the auxclasspath. This might happen, if the type node
// is probed for a specific type (e.g. is is a JUnit5 Test Annotation class).
// Failing to resolve clazzName does not necessarily indicate an incomplete auxclasspath.
} catch (final LinkageError expected) {
// We found the class but it's invalid / incomplete. This may be an incomplete auxclasspath
// if it was a NoClassDefFoundError. TODO : Report it?
}
return null;
@ -133,4 +139,13 @@ public final class TypeHelper {
return clazz.isAssignableFrom(type);
}
public static boolean isA(TypedNameDeclaration vnd, String className) {
Class<?> type = vnd.getType();
if (type != null) {
Class<?> expected = loadClass(type.getClassLoader(), className);
return expected.isAssignableFrom(type);
}
return false;
}
}

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

@ -429,6 +429,48 @@ class Style {
public void moveOutOfBoundsFrom() {
doSomething();
}
}]]></code>
</test-code>
<test-code>
<description>#1435 Treat AssertJ soft assertions as assert expressions</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.Test;
class FooTest {
@Test
void testFoo() {
var softly = new SoftAssertions();
softly.assertThat("doesn't matter").isEqualTo("doesn't matter");
softly.assertAll();
}
@Test
void testBar() {
SoftAssertions softly = new SoftAssertions();
softly.assertThat("doesn't matter").isEqualTo("doesn't matter");
softly.assertAll();
}
}]]></code>
</test-code>
<test-code>
<description>#1435 Treat AssertJ soft assertion rule for JUnit 4 as assert expressions</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.assertj.core.api.JUnitSoftAssertions;
import org.junit.Test;
public class FooTest {
@Rule
public final JUnitSoftAssertions softly = new JUnitSoftAssertions();
@Test
void testFoo() {
softly.assertThat("doesn't matter").isEqualTo("doesn't matter");
}
}]]></code>
</test-code>
</test-data>

5
pom.xml
View File

@ -936,6 +936,11 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code
<artifactId>system-rules</artifactId>
<version>1.8.0</version>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.11.0</version>
</dependency>
<!-- TEST DEPENDENCIES -->