diff --git a/docs/pages/pmd/rules/apex/performance.md b/docs/pages/pmd/rules/apex/performance.md index 6cf6388106..b3f3c6b5bb 100644 --- a/docs/pages/pmd/rules/apex/performance.md +++ b/docs/pages/pmd/rules/apex/performance.md @@ -5,7 +5,7 @@ permalink: pmd_rules_apex_performance.html folder: pmd/rules/apex sidebaractiveurl: /pmd_rules_apex.html editmepath: ../pmd-apex/src/main/resources/rulesets/apex/performance.xml -keywords: Performance, AvoidSoqlInLoops, AvoidDmlStatementsInLoops +keywords: Performance, AvoidSoqlInLoops, AvoidSoslInLoops, AvoidDmlStatementsInLoops --- ## AvoidDmlStatementsInLoops @@ -79,3 +79,37 @@ public class Something { ``` +## AvoidSoslInLoops + +**Since:** PMD 6.0.0 + +**Priority:** Medium (3) + +Sosl calls within loops can cause governor limit exceptions. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.apex.rule.performance.AvoidSoslInLoopsRule](https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java) + +**Example(s):** + +``` java +public class Something { + public static void main( String as[] ) { + for (Integer i = 0; i < 10; i++) { + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|cc_categories|[Style]|Code Climate Categories| +|cc_remediation_points_multiplier|1|Code Climate Remediation Points multiplier| +|cc_block_highlighting|false|Code Climate Block Highlighting| + +**Use this rule by referencing it:** +``` xml + +``` diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java new file mode 100644 index 0000000000..3d16b1871e --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java @@ -0,0 +1,55 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.performance; + +import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; +import net.sourceforge.pmd.lang.ast.Node; + +public class AvoidSoslInLoopsRule extends AbstractApexRule { + + public AvoidSoslInLoopsRule() { + setProperty(CODECLIMATE_CATEGORIES, "Performance"); + // Note: Often more complicated as just moving the SOSL a few lines. + // Involves Maps... + setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 150); + setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + } + + @Override + public Object visit(ASTSoslExpression node, Object data) { + if (insideLoop(node) && parentNotReturn(node) && parentNotForEach(node)) { + addViolation(data, node); + } + return data; + } + + private boolean parentNotReturn(ASTSoslExpression node) { + return !(node.jjtGetParent() instanceof ASTReturnStatement); + } + + private boolean parentNotForEach(ASTSoslExpression node) { + return !(node.jjtGetParent() instanceof ASTForEachStatement); + } + + private boolean insideLoop(ASTSoslExpression node) { + Node n = node.jjtGetParent(); + + while (n != null) { + if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement + || n instanceof ASTForLoopStatement || n instanceof ASTForEachStatement) { + return true; + } + n = n.jjtGetParent(); + } + + return false; + } +} diff --git a/pmd-apex/src/main/resources/rulesets/apex/performance.xml b/pmd-apex/src/main/resources/rulesets/apex/performance.xml index 89e47f739c..9d82b0a879 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/performance.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/performance.xml @@ -30,6 +30,28 @@ public class Something { + + +Sosl calls within loops can cause governor limit exceptions. + + 3 + +> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} +]]> + + + + + 3 + + + + + + + 3 diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/PerformanceRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/PerformanceRulesTest.java index c89531d091..3f9f88eb58 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/PerformanceRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/PerformanceRulesTest.java @@ -13,6 +13,7 @@ public class PerformanceRulesTest extends SimpleAggregatorTst { @Override public void setUp() { addRule(RULESET, "AvoidSoqlInLoops"); + addRule(RULESET, "AvoidSoslInLoops"); addRule(RULESET, "AvoidDmlStatementsInLoops"); } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoslInLoops.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoslInLoops.xml new file mode 100644 index 0000000000..19aa716baa --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoslInLoops.xml @@ -0,0 +1,107 @@ + + + + + + Problematic Sosl in for each + 1 + {1,2}) { + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in for loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in While loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in do loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + }while(true) ; + } +} + ]]> + + + + Multiple problematic Sosl expressions + 2 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name)]; + }while(true) ; + } +} + ]]> + + + + Return Sosl is even ok in loop + 0 + test1() { + for(;;) { + return [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + #29 SOSL For Loops should not throw an Avoid Sosl queries inside loops issue + 0 + a : [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity]) { + + } + } +} + ]]> + + +