diff --git a/pmd-java/pom.xml b/pmd-java/pom.xml index e212339c44..9d8f8fe151 100644 --- a/pmd-java/pom.xml +++ b/pmd-java/pom.xml @@ -176,5 +176,10 @@ ant-testutil test + + org.assertj + assertj-core + test + diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java index 071b76840f..4cbfb154b2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java @@ -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 variables = getVariables(method); + Scope classScope = method.getScope().getParent(); Map> 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> expectables) { + private boolean containsExpectOrAssert(Node n, + Map> expectables, + Map 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 getVariables(ASTMethodDeclaration method) { + Map 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 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; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java index c77263615a..8635735a07 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java @@ -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; + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml index b69044eff3..7c31031804 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml @@ -429,6 +429,48 @@ class Style { public void moveOutOfBoundsFrom() { doSomething(); } +}]]> + + + #1435 Treat AssertJ soft assertions as assert expressions + 0 + + + + #1435 Treat AssertJ soft assertion rule for JUnit 4 as assert expressions + 0 + diff --git a/pom.xml b/pom.xml index 52722c32d2..bdaed4be92 100644 --- a/pom.xml +++ b/pom.xml @@ -936,6 +936,11 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code system-rules 1.8.0 + + org.assertj + assertj-core + 3.11.0 +