diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index c091bc5abf..454b332b24 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -25,6 +25,8 @@ This is a {{ site.pmd.release_type }} release. * [#2610](https://github.com/pmd/pmd/pull/2610): \[apex] Support top-level enums in rules * apex-bestpractices * [#2626](https://github.com/pmd/pmd/issues/2626): \[apex] UnusedLocalVariable - false positive on case insensitivity allowed in Apex +* apex-performance + * [#2598](https://github.com/pmd/pmd/issues/2598): \[apex] AvoidSoqlInLoops false positive for SOQL with in For-Loop * apex-security * [#2620](https://github.com/pmd/pmd/issues/2620): \[visualforce] False positive on VfUnescapeEl with new Message Channel feature * core diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java index f8b6937c03..5a4ad597a0 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java @@ -1,9 +1,10 @@ -/** +/* * 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.ASTBlockStatement; import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; @@ -25,7 +26,7 @@ public class AvoidSoqlInLoopsRule extends AbstractApexRule { @Override public Object visit(ASTSoqlExpression node, Object data) { - if (insideLoop(node) && parentNotReturn(node) && parentNotForEach(node)) { + if (insideLoop(node) && parentNotReturn(node)) { addViolation(data, node); } return data; @@ -35,16 +36,16 @@ public class AvoidSoqlInLoopsRule extends AbstractApexRule { return !(node.getParent() instanceof ASTReturnStatement); } - private boolean parentNotForEach(ASTSoqlExpression node) { - return !(node.getParent() instanceof ASTForEachStatement); - } - private boolean insideLoop(ASTSoqlExpression node) { Node n = node.getParent(); while (n != null) { + if (n instanceof ASTBlockStatement && n.getParent() instanceof ASTForEachStatement) { + // only consider the block of the for-each statement, not the iterator + return true; + } if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement - || n instanceof ASTForLoopStatement || n instanceof ASTForEachStatement) { + || n instanceof ASTForLoopStatement) { return true; } n = n.getParent(); diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoqlInLoops.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoqlInLoops.xml index 14baee6285..a8c8d085f3 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoqlInLoops.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidSoqlInLoops.xml @@ -98,6 +98,21 @@ public class Foo { for(Account a : [SELECT Id FROM Account]) { } } +} + ]]> + + + + [apex] AvoidSoqlInLoops false positive for SOQL with in For-Loop #2598 + 0 +