SOQL Query in loop: improve/split dmlinloop

package.json

@@ -1,6 +1,6 @@
 {
   "name": "lightning-flow-scanner-core",
-  "version": "2.25.0",
+  "version": "2.26.0",
   "main": "out/**",
   "types": "out/index.d.ts",
   "scripts": {

readme.md

@@ -49,7 +49,7 @@ ___
 
 ### DML Statement In A Loop
 
-To prevent exceeding Apex governor limits, it is advisable to consolidate all your record-related operations, including creation, updates, or deletions, at the conclusion of the flow.
+To prevent exceeding Apex governor limits, it is advisable to consolidate all your database operations, including record creation, updates, or deletions, at the conclusion of the flow.
 
 **Configuration ID: `DMLStatementInLoop`**
 _([View source code](https://github.com/Force-Config-Control/lightning-flow-scanner-core/tree/master/src/main/rules/DMLStatementInLoop.ts))_
@@ -121,6 +121,15 @@ _([View source code](https://github.com/Force-Config-Control/lightning-flow-scan
 
 ___
 
+### SOQL Query In A Loop
+
+To prevent exceeding Apex governor limits, it is advisable to consolidate all your SOQL queries at the conclusion of the flow.
+
+**Configuration ID: `SOQLQueryInLoop`** 
+_([View source code](https://github.com/Force-Config-Control/lightning-flow-scanner-core/tree/master/src/main/rules/SOQLQueryInLoop.ts))_
+
+___
+
 ### Unconnected Element
 
 To maintain the efficiency and manageability of your Flow, it's best to avoid including unconnected elements that are not in use.

src/main/libs/Compiler.ts

@@ -8,6 +8,22 @@ export class Compiler {
         this.visitedElements = new Set<string>();
     }
 
+    isInLoop = (flow: Flow, element: FlowNode, startOfLoop: FlowNode): boolean => {
+        const connectors = element.connectors || [];
+        for (const connector of connectors) {
+          if (connector.reference) {
+            const referencedElement = (flow.elements as FlowNode[]).find(el => el.name === connector.reference);
+            if (referencedElement === startOfLoop) {
+              return true;
+            }
+            if (this.isInLoop(flow, referencedElement, startOfLoop)) {
+              return true;
+            }
+          }
+        }
+        return false;
+      };
+
     traverseFlow(flow: Flow, startElementName: string, visitCallback: (element: FlowNode) => void) {
         // Iterative Deepening Depth-First Search (IDDFS)
         let depth = 0;

src/main/rules/DMLStatementInLoop.ts

@@ -5,82 +5,48 @@ import { FlowType } from '../models/FlowType';
 import { RuleResult } from '../models/RuleResult';
 import { RuleCommon } from '../models/RuleCommon';
 import { ResultDetails } from '../models/ResultDetails';
+import { Compiler } from '../libs/Compiler';
 
 export class DMLStatementInLoop extends RuleCommon implements IRuleDefinition {
 
   constructor() {
     super({
       name: 'DMLStatementInLoop',
       label: 'DML Statement In A Loop',
-      description: "To prevent exceeding Apex governor limits, it is advisable to consolidate all your record-related operations, including creation, updates, or deletions, at the conclusion of the flow.",
+      description: "To prevent exceeding Apex governor limits, it is advisable to consolidate all your database operations, including record creation, updates, or deletions, at the conclusion of the flow.",
       type: 'pattern',
       supportedTypes: [...FlowType.backEndTypes, ...FlowType.visualTypes],
       docRefs: [{ 'label': 'Flow Best Practices', 'path': 'https://help.salesforce.com/s/articleView?id=sf.flow_prep_bestpractices.htm&type=5' }],
       isConfigurable: false
-    },
-    );
+    });
   }
 
   public execute(flow: Flow): RuleResult {
     if (flow.type[0] === 'Survey') {
       return new RuleResult(this, []);
     }
-    const dmlStatementTypes = ['recordLookups', 'recordDeletes', 'recordUpdates', 'recordCreates'];
-    const flowElements: FlowNode[] = flow.elements.filter(node => node.metaType === 'node') as FlowNode[];
+
+    const dmlStatementTypes = ['recordDeletes', 'recordUpdates', 'recordCreates'];
     const loopElements: FlowNode[] = flow.elements.filter(node => node.subtype === 'loops') as FlowNode[];
-    const dmlInLoopIndexes: number[] = [];
+    const dmlStatementsInLoops: FlowNode[] = [];
+    const compiler = new Compiler();
 
+    // Check if a DML statement is inside a loop
     for (const loopElement of loopElements) {
-      const startOfLoop = flowElements.findIndex(element => element.name === this.findStartOfLoopReference(loopElement));
-      let reachedEndOfLoop = false;
-      let indexesToProcess: number[] = [startOfLoop];
-      const processedLoopElementIndexes: number[] = [];
-      do {
-        indexesToProcess = indexesToProcess.filter(index => !processedLoopElementIndexes.includes(index));
-        if (indexesToProcess.length <= 0 || (indexesToProcess.length == 1 && indexesToProcess[0] == -1)) {
-          break;
-        }
-        for (const [index, element] of flowElements.entries()) {
-          if (indexesToProcess.includes(index)) {
-            const connectors = [];
-            for (const connector of element.connectors) {
-              if (connector.reference) {
-                connectors.push(connector);
-              }
-            }
-            if (dmlStatementTypes.includes(element.subtype)) {
-              dmlInLoopIndexes.push(index);
-            }
-            if (connectors.length > 0) {
-              const elementsByReferences = flowElements.filter(anElement => connectors.map(c => c.reference).includes(anElement.name));
-              for (const nextElement of elementsByReferences) {
-                const nextIndex = flowElements.findIndex(anElement => nextElement.name === anElement.name);
-                if ('loops' === nextElement.subtype) {
-                  reachedEndOfLoop = true;
-                } else if (!processedLoopElementIndexes.includes(nextIndex)) {
-                  indexesToProcess.push(nextIndex);
-                }
-              }
-            }
-            processedLoopElementIndexes.push(index);
-          }
+      const startOfLoop = loopElement;
+
+      compiler.traverseFlow(flow, loopElement.name, (element) => {
+        if (dmlStatementTypes.includes(element.subtype) && compiler.isInLoop(flow, element, startOfLoop)) {
+          dmlStatementsInLoops.push(element);
         }
-      } while (reachedEndOfLoop === false);
-    }
-    const dmlStatementsInLoops: FlowNode[] = [];
-    for (const [index, element] of flowElements.entries()) {
-      if (dmlInLoopIndexes.includes(index)) {
-        dmlStatementsInLoops.push(element);
-      }
+      });
     }
+
     let results = [];
     for (const det of dmlStatementsInLoops) {
       results.push(new ResultDetails(det));
     }
-    return new RuleResult(this, results);
-  }
 
-  private findStartOfLoopReference(loopElement: FlowNode) {
-    return loopElement.connectors.find(el => el.type === 'nextValueConnector').reference;
+    return new RuleResult(this, results);
   }
 }

src/main/rules/SOQLQueryInLoop.ts

@@ -0,0 +1,53 @@
+import { IRuleDefinition } from '../interfaces/IRuleDefinition';
+import { Flow } from '../models/Flow';
+import { FlowNode } from '../models/FlowNode';
+import { FlowType } from '../models/FlowType';
+import { RuleResult } from '../models/RuleResult';
+import { RuleCommon } from '../models/RuleCommon';
+import { ResultDetails } from '../models/ResultDetails';
+import { Compiler } from '../libs/Compiler';
+
+export class SOQLQueryInLoop extends RuleCommon implements IRuleDefinition {
+
+    constructor() {
+      super({
+        name: 'SOQLQueryInLoop',
+        label: 'SOQL Query In A Loop',
+        description: "To prevent exceeding Apex governor limits, it is advisable to consolidate all your SOQL queries at the conclusion of the flow.",
+        type: 'pattern',
+        supportedTypes: [...FlowType.backEndTypes, ...FlowType.visualTypes],
+        docRefs: [{ 'label': 'Flow Best Practices', 'path': 'https://help.salesforce.com/s/articleView?id=sf.flow_prep_bestpractices.htm&type=5' }],
+        isConfigurable: false
+      });
+    }
+
+    public execute(flow: Flow): RuleResult {
+      if (flow.type[0] === 'Survey') {
+        return new RuleResult(this, []);
+      }
+
+      const dmlStatementTypes = ['recordLookups'];
+      const loopElements: FlowNode[] = flow.elements.filter(node => node.subtype === 'loops') as FlowNode[];
+      const dmlStatementsInLoops: FlowNode[] = [];
+      const compiler = new Compiler();
+
+      // Check if a DML statement is inside a loop
+      for (const loopElement of loopElements) {
+        const startOfLoop = loopElement;
+
+        compiler.traverseFlow(flow, loopElement.name, (element) => {
+          if (dmlStatementTypes.includes(element.subtype) && compiler.isInLoop(flow, element, startOfLoop)) {
+            dmlStatementsInLoops.push(element);
+          }
+        });
+      }
+
+      let results = [];
+      for (const det of dmlStatementsInLoops) {
+        results.push(new ResultDetails(det));
+      }
+
+      return new RuleResult(this, results);
+    }
+  }
+

src/main/store/DefaultRuleStore.ts

@@ -7,6 +7,7 @@ import { FlowName } from '../rules/FlowName';
 import { HardcodedId } from '../rules/HardcodedId';
 import { MissingFaultPath } from '../rules/MissingFaultPath';
 import { MissingNullHandler } from '../rules/MissingNullHandler';
+import { SOQLQueryInLoop } from '../rules/SOQLQueryInLoop';
 import { UnconnectedElement } from '../rules/UnconnectedElement';
 import { UnusedVariable } from '../rules/UnusedVariable';
 
@@ -20,6 +21,7 @@ export const DefaultRuleStore: {} = {
   HardcodedId,
   MissingFaultPath,
   MissingNullHandler,
+  SOQLQueryInLoop,
   UnconnectedElement,
   UnusedVariable
 };

tests/demo_dmlStatementInALoop.test.ts → tests/DMLStatementInALoop_Demo.test.ts

@@ -5,7 +5,7 @@ import { Flow } from '../src/main/models/Flow';
 import { ScanResult } from '../src/main/models/ScanResult';
 import dmlstatementsinaloop from './testfiles/DMLStatementInALoop_Demo.json';
 
-describe('In the DML_statements_in_a_loop flow', () => {
+describe('In the DMLStatementInALoop_Demo flow', () => {
   let flow: Flow;
 
   before('arrange', () => {

tests/DMLStatementInALoop_Demo_Fixed.test.ts

@@ -0,0 +1,25 @@
+import { assert, expect } from 'chai';
+import 'mocha';
+import { scan } from '../src';
+import { Flow } from '../src/main/models/Flow';
+import { ScanResult } from '../src/main/models/ScanResult';
+import dmlstatementsinaloop from './testfiles/DMLStatementInALoop_Demo_Fixed.json';
+
+describe('In the DMLStatementInALoop_Demo_Fixed flow', () => {
+  let flow: Flow;
+
+  before('arrange', () => {
+    // ARRANGE
+    flow = new Flow({
+      path: './testfiles/DML_statements_in_a_loop_fixed.flow',
+      xmldata: dmlstatementsinaloop,
+    });
+  });
+
+  it('there should be no result for the rule DMLStatementInLoop', ()                                                                            => {
+
+    const results: ScanResult[] = scan([flow]);
+    const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs);
+    expect(occurringResults.length).to.equal(0);
+  });
+});

tests/SOQLQueryInALoop_Demo.test.ts

@@ -0,0 +1,26 @@
+import { assert, expect } from 'chai';
+import 'mocha';
+import { scan } from '../src';
+import { Flow } from '../src/main/models/Flow';
+import { ScanResult } from '../src/main/models/ScanResult';
+import flowfile from './testfiles/SOQLQueryInALoop_Demo.json';
+
+describe('In the SOQLQueryInALoop_Demo flow', () => {
+  let flow: Flow;
+
+  before('arrange', () => {
+    // ARRANGE
+    flow = new Flow({
+      path: './testfiles/SOQL_Query_In_Loop_Demo.flow',
+      xmldata: flowfile,
+    });
+  });
+
+  it('there should be one result for the rule SOQLQueryInLoop', ()                                                                            => {
+
+    const results: ScanResult[] = scan([flow]);
+    const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs);
+    expect(occurringResults.length).to.equal(1);
+    expect(occurringResults[0].ruleName).to.equal("SOQLQueryInLoop");
+  });
+});

tests/SOQLQueryInALoop_Demo_Fixed.test.ts

@@ -0,0 +1,25 @@
+import { assert, expect } from 'chai';
+import 'mocha';
+import { scan } from '../src';
+import { Flow } from '../src/main/models/Flow';
+import { ScanResult } from '../src/main/models/ScanResult';
+import flowfile from './testfiles/SOQLQueryInALoop_Demo_Fixed.json';
+
+describe('In the SOQLQueryInALoop_Demo flow', () => {
+  let flow: Flow;
+
+  before('arrange', () => {
+    // ARRANGE
+    flow = new Flow({
+      path: './testfiles/SOQL_Query_In_Loop_Demo.flow',
+      xmldata: flowfile,
+    });
+  });
+
+  it('there should be no result for the rule SOQLQueryInLoop', ()                                                                            => {
+
+    const results: ScanResult[] = scan([flow]);
+    const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs);
+    expect(occurringResults.length).to.equal(0);
+  });
+});

tests/getRules_findsAll.test.ts

@@ -3,8 +3,8 @@ import { expect } from 'chai';
 import 'mocha';
 
 describe('GetRuleDefinitions function', () => {
-  it('should return 11 rules', () => {
+  it('should return 12 rules', () => {
     const result = getRules();
-    expect(result.length).to.equal(11);
+    expect(result.length).to.equal(12);
   });
 });