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

Merge pull request #3198 from

Browse Source 
jonathanwiesel:more-apex-operations-with-limits-in-loops

[apex] OperationWithLimitsInLoopRule: Support more limit consuming
static method invocations #3198
This commit is contained in:
Andreas Dangel 2021-04-10 19:24:25 +02:00
commit 92e4631fc2
3 changed files with 156 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/pages/release_notes.md
View File

@ -23,6 +23,8 @@ This is a {{ site.pmd.release_type }} release.
### Fixed Issues
* apex-performance
* [#3198](https://github.com/pmd/pmd/pull/3198): \[apex] OperationWithLimitsInLoopRule: Support more limit consuming static method invocations
* java-bestpractices
* [#3190](https://github.com/pmd/pmd/issues/3190): \[java] Use StandardCharsets instead of Charset.forName
* java-errorprone
@ -33,6 +35,7 @@ This is a {{ site.pmd.release_type }} release.
### External Contributions
* [#3193](https://github.com/pmd/pmd/pull/3193): \[java] New rule: UseStandardCharsets - [Andrea Aime](https://github.com/aaime)
* [#3198](https://github.com/pmd/pmd/pull/3198): \[apex] OperationWithLimitsInLoopRule: Support more limit consuming static method invocations - [Jonathan Wiesel](https://github.com/jonathanwiesel)
{% endtocmaker %}

57
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopRule.java
View File

@ -11,6 +11,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTDmlUndeleteStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpdateStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpsertStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTRunAsBlockStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -19,6 +20,14 @@ import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
* Warn users when code that could trigger governor limits is executing within a looping construct.
*/
public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule {
private static final String APPROVAL_CLASS_NAME = "Approval";
private static final String MESSAGING_CLASS_NAME = "Messaging";
private static final String SYSTEM_CLASS_NAME = "System";
private static final String[] MESSAGING_LIMIT_METHODS = new String[] { "renderEmailTemplate", "renderStoredEmailTemplate", "sendEmail" };
private static final String[] SYSTEM_LIMIT_METHODS = new String[] { "enqueueJob", "schedule", "scheduleBatch" };
public OperationWithLimitsInLoopRule() {
setProperty(CODECLIMATE_CATEGORIES, "Performance");
// Note: Often more complicated as just moving a few lines.
@ -33,12 +42,13 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
addRuleChainVisit(ASTDmlUndeleteStatement.class);
addRuleChainVisit(ASTDmlUpdateStatement.class);
addRuleChainVisit(ASTDmlUpsertStatement.class);
// Database methods
addRuleChainVisit(ASTMethodCallExpression.class);
// SOQL
addRuleChainVisit(ASTSoqlExpression.class);
// SOSL
addRuleChainVisit(ASTSoslExpression.class);
// Other limit consuming methods
addRuleChainVisit(ASTRunAsBlockStatement.class);
addRuleChainVisit(ASTMethodCallExpression.class);
}
// Begin DML Statements
@ -73,17 +83,6 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
}
// End DML Statements
// Begin Database method invocations
@Override
public Object visit(ASTMethodCallExpression node, Object data) {
if (Helper.isAnyDatabaseMethodCall(node)) {
return checkForViolation(node, data);
} else {
return data;
}
}
// End Database method invocations
// Begin SOQL method invocations
@Override
public Object visit(ASTSoqlExpression node, Object data) {
@ -97,4 +96,36 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
return checkForViolation(node, data);
}
// End SOSL method invocations
// Begin general method invocations
@Override
public Object visit(ASTRunAsBlockStatement node, Object data) {
return checkForViolation(node, data);
}
@Override
public Object visit(ASTMethodCallExpression node, Object data) {
if (Helper.isAnyDatabaseMethodCall(node)
|| Helper.isMethodName(node, APPROVAL_CLASS_NAME, Helper.ANY_METHOD)
|| checkLimitClassMethods(node, MESSAGING_CLASS_NAME, MESSAGING_LIMIT_METHODS)
|| checkLimitClassMethods(node, SYSTEM_CLASS_NAME, SYSTEM_LIMIT_METHODS)) {
return checkForViolation(node, data);
} else {
return data;
}
}
private boolean checkLimitClassMethods(ASTMethodCallExpression node, String className, String[] methodNames) {
for (String method : methodNames) {
if (Helper.isMethodName(node, className, method)) {
return true;
}
}
return false;
}
// End general method invocations
}

109
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithLimitsInLoop.xml
View File

@ -410,4 +410,113 @@ public class Foo {
]]></code>
</test-code>
<!-- End Sosl method invocations -->
<!-- Begin approval method invocations -->
<test-code>
<description>Problematic approval actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
Account a = new Account();
Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest();
req.setObjectId(a.id);
do {
Approval.process(req);
Approval.unlock(a);
Approval.lock(a);
} while(true);
}
}
]]></code>
</test-code>
<test-code>
<description>Best Practice: Batch up data into a list and invoke your Approval actions once on that list of data.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
List<Account> accounts = new List<Account>();
List<Approval.ProcessSubmitRequest> reqs = new List<Approval.ProcessSubmitRequest>();
while(true) {
Account account = new Account();
accounts.add(account);
Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest();
req.setObjectId(account.id);
reqs.add(req);
}
Approval.process(reqs, true);
Approval.unlock(accounts);
Approval.lock(accounts);
}
}
]]></code>
</test-code>
<!-- End approval method invocations -->
<!-- Begin messaging method invocations -->
<test-code>
<description>Problematic messaging actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
Contact cont = new Contact();
Account acc = new Account();
do {
Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();
List<Messaging.RenderEmailTemplateBodyResult> renderedRes = Messaging.renderEmailTemplate(cont.Id, acc.Id, new List<String>());
Messaging.SingleEmailMessage renderedMail = Messaging.renderStoredEmailTemplate(null, cont.Id, acc.Id);
Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email});
} while(true);
}
}
]]></code>
</test-code>
<!-- End messaging method invocations -->
<!-- Begin system method invocations -->
<test-code>
<description>Problematic system actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
do {
System.enqueueJob(new MyQueueable());
System.schedule('x', '0 0 0 1 1 ?', new MySchedule());
System.scheduleBatch(new MyBatch(), 'x', 1);
System.debug(LoggingLevel.INFO, 'X');
System.assertEquals(1, 1);
} while(true);
}
}
]]></code>
</test-code>
<test-code>
<description>Problematic system.runAs action in loop</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
do {
System.runAs(new User()) {
System.debug(LoggingLevel.INFO, 'X');
System.assertEquals(1, 1);
}
} while(true);
}
}
]]></code>
</test-code>
<!-- End system method invocations -->
</test-data>