From 2639b4fe66471e6c5b3a49f5000383f188aa05ff Mon Sep 17 00:00:00 2001
From: Jan Aertgeerts <jaertgee@its.jnj.com>
Date: Wed, 20 Sep 2017 12:42:22 +0200
Subject: [PATCH] [apex] Add AvoidDirectAccessTriggerMap rule to the
 performance set

---
 docs/pages/pmd/rules/apex/performance.md      | 37 +++++++++++++++++++
 .../resources/rulesets/apex/performance.xml   | 32 ++++++++++++++++
 .../main/resources/rulesets/apex/ruleset.xml  |  9 +++++
 .../performance/PerformanceRulesTest.java     |  1 +
 .../xml/AvoidDirectAccessTriggerMap.xml       | 31 ++++++++++++++++
 5 files changed, 110 insertions(+)
 create mode 100644 pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDirectAccessTriggerMap.xml

diff --git a/docs/pages/pmd/rules/apex/performance.md b/docs/pages/pmd/rules/apex/performance.md
index 6cf6388106..5b87cd8cc2 100644
--- a/docs/pages/pmd/rules/apex/performance.md
+++ b/docs/pages/pmd/rules/apex/performance.md
@@ -79,3 +79,40 @@ public class Something {
 <rule ref="rulesets/apex/performance.xml/AvoidSoqlInLoops" />
 ```
 
+
+## AvoidDirectAccessTriggerMap
+
+**Since:** PMD 6.0.0
+
+**Priority:** Medium (3)
+
+Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Triggers should be bulkified and iterate through the map to handle the actions for each item separately.
+
+```
+//ArrayLoadExpression/TriggerVariableExpression
+```
+
+**Example(s):**
+``` 
+trigger AccountTrigger on Account (before insert, before update) {
+   Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
+   
+   foreach ( Account a : Trigger.new ){   
+        //Good: Iterate through the trigger.new array instead.
+   }
+}
+```
+
+**This rule has the following properties:**
+
+|Name|Default Value|Description|
+|----|-------------|-----------|
+|cc_categories|[Style]|Code Climate Categories|
+|cc_remediation_points_multiplier|1|Code Climate Remediation Points multiplier|
+|cc_block_highlighting|false|Code Climate Block Highlighting|
+
+**Use this rule by referencing it:**
+``` xml
+<rule ref="rulesets/apex/performance.xml/AvoidDirectAccessTriggerMap" />
+```
+
diff --git a/pmd-apex/src/main/resources/rulesets/apex/performance.xml b/pmd-apex/src/main/resources/rulesets/apex/performance.xml
index 89e47f739c..a9bf247bc1 100644
--- a/pmd-apex/src/main/resources/rulesets/apex/performance.xml
+++ b/pmd-apex/src/main/resources/rulesets/apex/performance.xml
@@ -50,6 +50,38 @@ public class Something {
         }
     }
 }
+]]>
+    </example>
+  </rule>
+
+  <rule name="AvoidDirectAccessTriggerMap"
+        since="6.0.0"
+        message="Avoid directly accessing Trigger.old and Trigger.new"
+        class="net.sourceforge.pmd.lang.apex.rule.ApexXPathRule"
+        externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_performance.html#avoiddirectaccesstriggermap">
+    <description>
+Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Triggers should be bulkified and iterate through the map to handle the actions for each item separately.
+    </description>
+    <priority>3</priority>
+        <properties>
+        <property name="xpath">
+            <value>
+<![CDATA[
+//ArrayLoadExpression/TriggerVariableExpression
+]]>
+            </value>
+        </property>
+    </properties>
+
+    <example>
+<![CDATA[
+trigger AccountTrigger on Account (before insert, before update) {
+   Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
+   
+   foreach ( Account a : Trigger.new ){   
+        //Good: Iterate through the trigger.new array instead.
+   }
+}
 ]]>
     </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 768ecd8994..583081b964 100644
--- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
+++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml
@@ -113,6 +113,15 @@
          <property name="cc_block_highlighting" value="false" />
       </properties>
    </rule>
+   <rule ref="rulesets/apex/performance.xml/AvoidDirectAccessTriggerMap" message="Avoid directly accessing Trigger.old and Trigger.new">
+      <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>
       <properties>
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 c89531d091..a64b338a5a 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
@@ -14,5 +14,6 @@ public class PerformanceRulesTest extends SimpleAggregatorTst {
     public void setUp() {
         addRule(RULESET, "AvoidSoqlInLoops");
         addRule(RULESET, "AvoidDmlStatementsInLoops");
+        addRule(RULESET, "AvoidDirectAccessTriggerMap");
     }
 }
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDirectAccessTriggerMap.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDirectAccessTriggerMap.xml
new file mode 100644
index 0000000000..5a9287532a
--- /dev/null
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidDirectAccessTriggerMap.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<test-data
+    xmlns="http://pmd.sourceforge.net/rule-tests"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+
+    <test-code>
+        <description>Directly accessing the Trigger.old map.</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+trigger AccountTrigger on Account (before insert, before update) {
+   Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
+}
+		]]></code>
+	</test-code>
+    
+	<test-code>
+        <description>Looping through map, best practice</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+trigger AccountTrigger on Account (before insert, before update) {
+   foreach ( Account a : Trigger.new ){   
+        //Good: Iterate through the trigger.new array instead.
+        system.debug(a);
+   }
+}
+		]]></code>
+    </test-code>
+    
+</test-data>