From 277ce76eb64b2ba7ca8809eb2c846e9975f93b9d Mon Sep 17 00:00:00 2001
From: David Renz <renz-david@web.de>
Date: Mon, 30 May 2016 11:55:31 +0200
Subject: [PATCH] Added new apex rule (AvoidDmlStatementsInLoops)

---
 .../AvoidDmlStatementsInLoopsRule.java        | 85 +++++++++++++++++++
 .../resources/rulesets/apex/performance.xml   | 22 +++++
 .../main/resources/rulesets/apex/ruleset.xml  | 11 +++
 .../performance/PerformanceRulesTest.java     |  1 +
 .../xml/AvoidDmlStatementsInLoops.xml         | 84 ++++++++++++++++++
 5 files changed, 203 insertions(+)
 create mode 100644 pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java
 create mode 100644 pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDmlStatementsInLoops.xml

diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java
new file mode 100644
index 0000000000..373757216a
--- /dev/null
+++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java
@@ -0,0 +1,85 @@
+/**
+ * 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.ast.AbstractNode;
+import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.apex.ast.ASTDmlDeleteStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTDmlInsertStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTDmlMergeStatement;
+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.ASTDoLoopStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement;
+import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
+
+public class AvoidDmlStatementsInLoopsRule extends AbstractApexRule {
+
+	public AvoidDmlStatementsInLoopsRule() {
+		setProperty(CODECLIMATE_CATEGORIES, new String[]{ "Performance" });
+		// Note: Often more complicated as just moving the SOQL a few lines. Involves Maps...
+		setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 150);
+		setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
+	}
+
+	@Override
+	public Object visit(ASTDmlDeleteStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+	@Override
+	public Object visit(ASTDmlInsertStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+	@Override
+	public Object visit(ASTDmlMergeStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+	@Override
+	public Object visit(ASTDmlUndeleteStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+	@Override
+	public Object visit(ASTDmlUpdateStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+	@Override
+	public Object visit(ASTDmlUpsertStatement node, Object data) {
+		if (insideLoop(node)) {
+			addViolation(data, node);
+		}
+		return data;
+	}
+
+	private boolean insideLoop(AbstractNode 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 85d70a6ab0..3699dfe0dd 100644
--- a/pmd-apex/src/main/resources/rulesets/apex/performance.xml
+++ b/pmd-apex/src/main/resources/rulesets/apex/performance.xml
@@ -25,6 +25,28 @@ New objects created within loops should be checked to see if they can created ou
 		}
 	}
 }
+]]>
+    </example>
+  </rule>
+  
+  <rule name="AvoidDmlStatementsInLoops"
+        since="5.5.0"
+        message="Avoid DML statements inside loops"
+        class="net.sourceforge.pmd.lang.apex.rule.performance.AvoidDmlStatementsInLoopsRule"
+          externalInfoUrl="${pmd.website.baseurl}/rules/apex/performance.html#AvoidDmlStatementsInLoops">
+    <description>Avoid DML statements inside loops to avoid hitting the DML governor limit. Instead, try to batch up the data into a list and invoke your DML once on that list of data outside the loop.</description>
+      <priority>3</priority>
+    <example>
+<![CDATA[
+public class Something {
+	public void foo() {  
+		for (Integer i = 0; i < 151; i++) {
+		    Account account;
+		    ...
+		    insert account;
+		}
+	}
+}
 ]]>
     </example>
   </rule>
diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
index 4f360cf949..6f817c148a 100644
--- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
+++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
@@ -136,6 +136,17 @@
        	</properties>
 	</rule>
 	
+	<rule ref="rulesets/apex/performance.xml/AvoidDmlStatementsInLoops" message="Avoid DML Statements inside loops">
+		<priority>3</priority>
+        
+        <properties>	
+	       	<!-- relevant for Code Climate output only -->
+	       	<property name="cc_categories" value="Performance"/>
+	       	<property name="cc_remediation_points_multiplier" value="150"/>
+	       	<property name="cc_block_highlighting" value="false"/>
+       	</properties>
+	</rule>
+	
 	<rule ref="rulesets/apex/style.xml/AvoidLogicInTrigger" message="Avoid logic in triggers">
 		<priority>3</priority>
         
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 cada0231e0..e1f12c5038 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
@@ -12,5 +12,6 @@ public class PerformanceRulesTest extends SimpleAggregatorTst {
 	@Override
 	public void setUp() {
 		addRule(RULESET, "AvoidSoqlInLoops");
+		addRule(RULESET, "AvoidDmlStatementsInLoops");
 	}
 }
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDmlStatementsInLoops.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDmlStatementsInLoops.xml
new file mode 100644
index 0000000000..00921ba3d5
--- /dev/null
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDmlStatementsInLoops.xml
@@ -0,0 +1,84 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<test-data>
+
+    <test-code>
+        <description>Problematic Dml Statement in for each</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+	public void foo() {
+		for(Integer i : new List<Integer>{1,2}) {
+			Account account = new Account();
+			insert account;
+		}
+	}
+}
+		]]></code>
+	</test-code>
+    
+	<test-code>
+        <description>Problematic Dml Statement in for loop</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+	public void foo() {
+		for(;;) {
+			Account account = new Account();
+			insert account;
+		}
+	}
+}
+		]]></code>
+    </test-code>
+    
+    <test-code>
+        <description>Problematic Dml Statement in While loop</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+	public void foo() {
+		while(true) {
+			Account account = new Account();
+			insert account;
+		}
+	}
+}
+		]]></code>
+    </test-code>
+    
+    <test-code>
+        <description>Problematic Dml Statement in do loop</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+	public void foo() {
+		do {
+			Account account = new Account();
+			insert account;
+		} while(true);
+	}
+}
+		]]></code>
+    </test-code>
+    
+    <test-code>
+        <description>Best Practice: Batch up data into a list and invoke your DML 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>();
+		
+		while(true) {
+			Account account = new Account();
+			accounts.add(account);
+		}
+		
+		insert accounts;
+	}
+}
+		]]></code>
+    </test-code>
+    
+</test-data>