From 05933c646e50e2eca39bdb4946e8a57201bd08d6 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Thu, 16 Jul 2020 20:13:53 +0200
Subject: [PATCH] [apex] AvoidSoqlInLoops false positive for SOQL with in
 For-Loop

Fixes #2598
---
 docs/pages/release_notes.md                       |  2 ++
 .../rule/performance/AvoidSoqlInLoopsRule.java    | 15 ++++++++-------
 .../rule/performance/xml/AvoidSoqlInLoops.xml     | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index 4528deb794..17e7378c07 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
 *   core
     *   [#2594](https://github.com/pmd/pmd/issues/2594): \[core] Update exec-maven-plugin and align it in all project
 *   java-design
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]) {
         }
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>[apex] AvoidSoqlInLoops false positive for SOQL with in For-Loop #2598</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    public void bar() {
+        for(NAMESPACE__CustomObject__c customObject : DatabaseUtility.query([Select Id FROM NAMESPACE__CustomObject__c where id IN :setIds])) // <-- violation is reported here
+        {
+            // …
+        }
+    }
 }
         ]]></code>
     </test-code>