hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-1694'

Browse Source
This commit is contained in:
Andreas Dangel
2019-03-25 20:07:15 +01:00
parent 3a866ddb9f 53d123703b
commit 17ab453db9
11 changed files with 377 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
docs/pages/pmd/rules/apex.md
View File

@ -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 ...

82
docs/pages/pmd/rules/apex/bestpractices.md
View File

@ -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
---
<!-- DO NOT EDIT THIS FILE. This file is generated from file ../pmd-apex/src/main/resources/category/apex/bestpractices.xml. -->
## 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
<rule ref="category/apex/bestpractices.xml/ApexUnitTestAssertStatement" />
```
## ApexUnitTestClassShouldHaveAsserts
**Since:** PMD 5.5.1
@ -47,6 +86,47 @@ public class Foo {
<rule ref="category/apex/bestpractices.xml/ApexUnitTestClassShouldHaveAsserts" />
```
## 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
<rule ref="category/apex/bestpractices.xml/ApexUnitTestMethodShouldHaveIsTestAnnotation" />
```
## ApexUnitTestShouldNotUseSeeAllDataTrue
**Since:** PMD 5.5.1

7
docs/pages/release_notes.md
View File

@ -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)

33
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexAssertionsShouldIncludeMessageRule.java Normal file
View File

@ -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;
}
}

48
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java Normal file
View File

@ -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<String> 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<ASTMethodCallExpression> 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;
}
}

59
pmd-apex/src/main/resources/category/apex/bestpractices.xml
View File

@ -9,6 +9,33 @@
Rules which enforce generally accepted best practices.
</description>
<rule name="ApexAssertionsShouldIncludeMessage"
since="6.13.0"
message="Apex test assert statement should make use of the message parameter."
class="net.sourceforge.pmd.lang.apex.rule.bestpractices.ApexAssertionsShouldIncludeMessageRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_bestpractices.html#apexassertionsshouldincludemessage">
<description>
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.
</description>
<priority>3</priority>
<example>
<![CDATA[
@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
}
}
]]>
</example>
</rule>
<rule name="ApexUnitTestClassShouldHaveAsserts"
since="5.5.1"
message="Apex unit tests should System.assert() or assertEquals() or assertNotEquals()"
@ -34,6 +61,38 @@ public class Foo {
</example>
</rule>
<rule name="ApexUnitTestMethodShouldHaveIsTestAnnotation"
since="6.13.0"
message="Apex test methods should have @isTest annotation."
class="net.sourceforge.pmd.lang.apex.rule.bestpractices.ApexUnitTestMethodShouldHaveIsTestAnnotationRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_bestpractices.html#apexunittestmethodshouldhaveistestannotation">
<description>
Apex test methods should have @isTest annotation.
As testMethod keyword is deprecated, Salesforce advices to use @isTest annotation for test class/methods.
</description>
<priority>3</priority>
<example>
<![CDATA[
@isTest
private class ATest {
@isTest
static void methodATest() {
}
static void methodBTest() {
}
@isTest static void methodCTest() {
System.assert(1==2);
}
@isTest static void methodCTest() {
System.debug('I am a debug statement');
}
private void fetchData() {
}
}
]]>
</example>
</rule>
<rule name="ApexUnitTestShouldNotUseSeeAllDataTrue"
since="5.5.1"
message="Apex unit tests should not use @isTest(seeAllData = true)"

11
pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexAssertionsShouldIncludeMessageTest.java Normal file
View File

@ -0,0 +1,11 @@
/**
* 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.testframework.PmdRuleTst;
public class ApexAssertionsShouldIncludeMessageTest extends PmdRuleTst {
// no additional unit tests
}

11
pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationTest.java Normal file
View File

@ -0,0 +1,11 @@
/**
* 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.testframework.PmdRuleTst;
public class ApexUnitTestMethodShouldHaveIsTestAnnotationTest extends PmdRuleTst {
// no additional unit tests
}

50
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexAssertionsShouldIncludeMessage.xml Normal file
View File

@ -0,0 +1,50 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>Problematic apex unit test assert statements - assert should have 2/3 params</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>5,7</expected-linenumbers>
<code><![CDATA[
@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
}
}
]]></code>
</test-code>
<test-code>
<description>[apex] assert should have 2 param and assertEquals should have 3 param.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
@isTest
public class Foo {
@isTest
static void methodATest() {
System.assertEquals('Test opportunity', o.Name, 'Opportunity Name' + o.Name + 'is wrong.'); // good
System.assert(o.isClosed, 'Opportunity is not closed.'); // good
}
}
]]></code>
</test-code>
<test-code>
<description>[apex] Avoid false positive in non test classes.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
static void methodA() {
System.assertEquals('Test opportunity', o.Name);
System.assert(o.isClosed);
}
}
]]></code>
</test-code>
</test-data>

73
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml Normal file
View File

@ -0,0 +1,73 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>Problematic apex unit test methods - test methods should have @isTest annotation.</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>6,18</expected-linenumbers>
<code><![CDATA[
@isTest
private class ATest {
@isTest
static void methodATest() {
}
static void methodBTest() {
System.assert(1==2);
}
@isTest static void methodCTest() {
}
@isTest
static void methodDTest() {
System.assert(3==3);
}
static void methodETest() {
System.debug('I am a debug statement.');
}
static void methodFTest() {
System.assertEquals(1,2);
}
private void fetchData() {
}
}
]]></code>
</test-code>
<test-code>
<description>Test methods should have @isTest annotation.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
@isTest
private class ATest {
@setUp
static void setUp(){
}
@isTest
static void methodATestOne() {
System.assertEquals(1,2);
}
@isTest
static void methodBTest() {
System.assertEquals(1,2);
}
@isTest static void methodC() {
System.assertEquals(1,2);
}
private void fetchData() {
}
}
]]></code>
</test-code>
<test-code>
<description>Avoid false positive in non test class methods.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
private class A {
private void fetchData() {
System.assertEquals(1,1);
}
}
]]></code>
</test-code>
</test-data>

3
pmd-core/src/main/resources/rulesets/releases/6130.xml
View File

@ -8,6 +8,9 @@
This ruleset contains links to rules that are new in PMD v6.13.0
</description>
<rule ref="category/apex/bestpractices.xml/ApexAssertionsShouldIncludeMessage"/>
<rule ref="category/apex/bestpractices.xml/ApexUnitTestMethodShouldHaveIsTestAnnotation"/>
<rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean"/>
<rule ref="category/java/design.xml/AvoidUncheckedExceptionsInSignatures"/>
<rule ref="category/java/errorprone.xml/DetachedTestCase"/>