Merge branch 'master' of https://github.com/DavidRenz/pmd into pr-95

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;
+	}
+}

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>

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>
         

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");
 	}
 }

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>