diff --git a/docs/pages/pmd/rules/apex.md b/docs/pages/pmd/rules/apex.md index ab1867350a..5390a956a8 100644 --- a/docs/pages/pmd/rules/apex.md +++ b/docs/pages/pmd/rules/apex.md @@ -12,6 +12,7 @@ folder: pmd/rules {% include callout.html content="Rules which enforce generally accepted best practices." %} * [ApexUnitTestClassShouldHaveAsserts](pmd_rules_apex_bestpractices.html#apexunittestclassshouldhaveasserts): Apex unit tests should include at least one assertion. This makes the tests more robust, and usi... +* [ApexUnitTestMethodShouldHaveIsTestAnnotation](pmd_rules_apex_bestpractices.html#apexunittestmethodshouldhaveistestannotation): Apex test methods should have @isTest annotation. As testMethod keyword is deprecated,... * [ApexUnitTestShouldNotUseSeeAllDataTrue](pmd_rules_apex_bestpractices.html#apexunittestshouldnotuseseealldatatrue): Apex unit tests should not use @isTest(seeAllData=true) because it opens up the existing database... * [AvoidGlobalModifier](pmd_rules_apex_bestpractices.html#avoidglobalmodifier): Global classes should be avoided (especially in managed packages) as they can never be deleted or... * [AvoidLogicInTrigger](pmd_rules_apex_bestpractices.html#avoidlogicintrigger): As triggers do not allow methods like regular classes they are less flexible and suited to apply ... diff --git a/docs/pages/pmd/rules/apex/bestpractices.md b/docs/pages/pmd/rules/apex/bestpractices.md index b4800dc6c9..4978698a37 100644 --- a/docs/pages/pmd/rules/apex/bestpractices.md +++ b/docs/pages/pmd/rules/apex/bestpractices.md @@ -5,10 +5,49 @@ permalink: pmd_rules_apex_bestpractices.html folder: pmd/rules/apex sidebaractiveurl: /pmd_rules_apex.html editmepath: ../pmd-apex/src/main/resources/category/apex/bestpractices.xml -keywords: Best Practices, ApexUnitTestClassShouldHaveAsserts, ApexUnitTestShouldNotUseSeeAllDataTrue, AvoidGlobalModifier, AvoidLogicInTrigger +keywords: Best Practices, ApexUnitTestClassShouldHaveAsserts, ApexUnitTestShouldNotUseSeeAllDataTrue, AvoidGlobalModifier, AvoidLogicInTrigger, ApexUnitTestAssertStatement, ApexUnitTestMethodShouldHaveIsTestAnnotation language: Apex --- +## ApexUnitTestAssertStatement + +**Since:** PMD 6.13.0 + +**Priority:** Medium (3) + +The second parameter of System.assert/third parameter of System.assertEquals/System.assertNotEquals is a message. +Having a second/third parameter provides more information and makes it easier to debug the test failure and improves the readability of test output. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.apex.rule.bestpractices.ApexAssertionsShouldIncludeMessageRule](https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestAssertStatementRule.java) + +**Example(s):** + +``` java +@isTest +public class Foo { + @isTest + static void methodATest() { + System.assertNotEquals('123', o.StageName); // not good + System.assertEquals('123', o.StageName, 'Opportunity stageName is wrong.'); // good + System.assert(o.isClosed); // not good + System.assert(o.isClosed, 'Opportunity is not closed.'); // good + } +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description|Multivalued| +|----|-------------|-----------|-----------| +|cc\_categories|Style|Code Climate Categories|yes. Delimiter is '\|'.| +|cc\_remediation\_points\_multiplier|1|Code Climate Remediation Points multiplier|no| +|cc\_block\_highlighting|false|Code Climate Block Highlighting|no| + +**Use this rule by referencing it:** +``` xml + +``` + ## ApexUnitTestClassShouldHaveAsserts **Since:** PMD 5.5.1 @@ -47,6 +86,47 @@ public class Foo { ``` +## ApexUnitTestMethodShouldHaveIsTestAnnotation + +**Since:** PMD 6.13.0 + +**Priority:** Medium (3) + +Apex test methods should have @isTest annotation. +As testMethod keyword is deprecated, Salesforce advices to use @isTest annotation for test class/methods. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.apex.rule.bestpractices.ApexUnitTestMethodShouldHaveIsTestAnnotationRule](https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java) + +**Example(s):** + +``` java +@isTest +private class ATest { + @isTest + static void methodATest() { + } + static void methodBTest() { + } + @isTest static void methodCTest() { + } + private void fetchData() { + } +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description|Multivalued| +|----|-------------|-----------|-----------| +|cc\_categories|Style|Code Climate Categories|yes. Delimiter is '\|'.| +|cc\_remediation\_points\_multiplier|1|Code Climate Remediation Points multiplier|no| +|cc\_block\_highlighting|false|Code Climate Block Highlighting|no| + +**Use this rule by referencing it:** +``` xml + +``` + ## ApexUnitTestShouldNotUseSeeAllDataTrue **Since:** PMD 5.5.1 diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8aedaffbaf..6c6969e8a3 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -47,6 +47,12 @@ The designer will still be shipped with PMD's binaries. Do-While-Loops and While-Loops that can be simplified since they use simply `true` or `false` as their loop condition. +* The new Apex rule {% rule "apex/bestpractices/ApexAssertionsShouldIncludeMessage" %} (`apex-bestpractices`) + searches for assertions in unit tests and checks, whether they use a message argument. + +* The new Apex rule {% rule "apex/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotation" %} (`apex-bestpractices`) + searches for methods in test classes, which are missing the `@IsTest` annotation. + ### Fixed Issues * doc @@ -68,6 +74,7 @@ The designer will still be shipped with PMD's binaries. ### External Contributions +* [#1694](https://github.com/pmd/pmd/pull/1694): \[apex] New rules for test method and assert statements - [triandicAnt](https://github.com/triandicAnt) * [#1697](https://github.com/pmd/pmd/pull/1697): \[doc] Update CPD documentation - [Matías Fraga](https://github.com/matifraga) * [#1704](https://github.com/pmd/pmd/pull/1704): \[java] Added AvoidUncheckedExceptionsInSignatures Rule - [Bhanu Prakash Pamidi](https://github.com/pamidi99) * [#1706](https://github.com/pmd/pmd/pull/1706): \[java] Add DetachedTestCase rule - [David Burström](https://github.com/davidburstromspotify) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexAssertionsShouldIncludeMessageRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexAssertionsShouldIncludeMessageRule.java new file mode 100644 index 0000000000..10c8424826 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexAssertionsShouldIncludeMessageRule.java @@ -0,0 +1,33 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.bestpractices; + +import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexUnitTestRule; + +public class ApexAssertionsShouldIncludeMessageRule extends AbstractApexUnitTestRule { + + private static final String ASSERT = "System.assert"; + private static final String ASSERT_EQUALS = "System.assertEquals"; + private static final String ASSERT_NOT_EQUALS = "System.assertNotEquals"; + + @Override + public Object visit(ASTMethodCallExpression node, Object data) { + String methodName = node.getFullMethodName(); + + if (ASSERT.equalsIgnoreCase(methodName) && node.jjtGetNumChildren() == 2) { + addViolationWithMessage(data, node, + "''{0}'' should have 2 parameters.", + new Object[] { ASSERT }); + } else if ((ASSERT_EQUALS.equalsIgnoreCase(methodName) + || ASSERT_NOT_EQUALS.equalsIgnoreCase(methodName)) + && node.jjtGetNumChildren() == 3) { + addViolationWithMessage(data, node, + "''{0}'' should have 3 parameters.", + new Object[] { methodName }); + } + return data; + } +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java new file mode 100644 index 0000000000..66185f9a3d --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java @@ -0,0 +1,48 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.bestpractices; + +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import net.sourceforge.pmd.lang.apex.ast.ASTMethod; +import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexUnitTestRule; + +public class ApexUnitTestMethodShouldHaveIsTestAnnotationRule extends AbstractApexUnitTestRule { + private static final Set ASSERT_METHODS = new HashSet<>(); + + static { + ASSERT_METHODS.add("system.assert"); + ASSERT_METHODS.add("system.assertequals"); + ASSERT_METHODS.add("system.assertnotequals"); + } + + @Override + public Object visit(final ASTMethod node, final Object data) { + // test methods should have @isTest annotation. + if (isTestMethodOrClass(node)) { + return data; + } + return checkForAssertStatements(node, data); + } + + private Object checkForAssertStatements(final ASTMethod testMethod, final Object data) { + List methodCallList = testMethod.findDescendantsOfType(ASTMethodCallExpression.class); + String assertMethodName; + for (ASTMethodCallExpression assertMethodCall : methodCallList) { + assertMethodName = assertMethodCall.getFullMethodName().toLowerCase(Locale.ROOT); + if (ASSERT_METHODS.contains(assertMethodName)) { + addViolationWithMessage(data, testMethod, + "''{0}'' method should have @IsTest annotation.", + new Object[] { testMethod.getImage() }); + return data; + } + } + return data; + } +} diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 1ac782d8b1..404502d662 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -9,6 +9,33 @@ Rules which enforce generally accepted best practices. + + +The second parameter of System.assert/third parameter of System.assertEquals/System.assertNotEquals is a message. +Having a second/third parameter provides more information and makes it easier to debug the test failure and +improves the readability of test output. + + 3 + + + + + + + +Apex test methods should have @isTest annotation. +As testMethod keyword is deprecated, Salesforce advices to use @isTest annotation for test class/methods. + + 3 + + + + + + + + + Problematic apex unit test assert statements - assert should have 2/3 params + 2 + 5,7 + + + + [apex] assert should have 2 param and assertEquals should have 3 param. + 0 + + + + [apex] Avoid false positive in non test classes. + 0 + + + diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml new file mode 100644 index 0000000000..4c804775d9 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml @@ -0,0 +1,73 @@ + + + + + Problematic apex unit test methods - test methods should have @isTest annotation. + 2 + 6,18 + + + + Test methods should have @isTest annotation. + 0 + + + + Avoid false positive in non test class methods. + 0 + + + \ No newline at end of file diff --git a/pmd-core/src/main/resources/rulesets/releases/6130.xml b/pmd-core/src/main/resources/rulesets/releases/6130.xml index 55c31de0d4..7faa4caac6 100644 --- a/pmd-core/src/main/resources/rulesets/releases/6130.xml +++ b/pmd-core/src/main/resources/rulesets/releases/6130.xml @@ -8,6 +8,9 @@ This ruleset contains links to rules that are new in PMD v6.13.0 + + +