Fix Apex metrics framework failing on triggers, refs pmd#768

oowekyala · Nov 30, 2017 · b1c191b · b1c191b
1 parent 345cc50
commit b1c191b
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 21 deletions.
diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexQualifiedName.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexQualifiedName.java
@@ -14,6 +14,7 @@
 
 import apex.jorje.semantic.symbol.type.TypeInfo;
 
+
 /**
  * Qualified name of an apex class or method.
  *
@@ -106,9 +107,9 @@ public int hashCode() {
     @Override
     public boolean equals(Object obj) {
         return obj instanceof ApexQualifiedName
-            && Objects.deepEquals(classes, ((ApexQualifiedName) obj).classes)
-            && Objects.equals(operation, ((ApexQualifiedName) obj).operation)
-            && Objects.equals(nameSpace, ((ApexQualifiedName) obj).nameSpace);
+               && Objects.deepEquals(classes, ((ApexQualifiedName) obj).classes)
+               && Objects.equals(operation, ((ApexQualifiedName) obj).operation)
+               && Objects.equals(nameSpace, ((ApexQualifiedName) obj).nameSpace);
 
     }
 
@@ -170,8 +171,18 @@ private static String getOperationString(ASTMethod node) {
 
 
     static ApexQualifiedName ofMethod(ASTMethod node) {
-        ApexQualifiedName parent = node.getFirstParentOfType(ASTUserClassOrInterface.class).getQualifiedName();
+        ASTUserClassOrInterface<?> parent = node.getFirstParentOfType(ASTUserClassOrInterface.class);
+        if (parent == null) {
+            ASTUserTrigger trigger = node.getFirstParentOfType(ASTUserTrigger.class);
+            String ns = trigger.getNode().getDefiningType().getNamespace().toString();
+
+            return new ApexQualifiedName(ns, new String[]{"trigger"}, trigger.getImage()); // uses a reserved word as a class name to prevent clashes
+            // TODO needs attention from an Apex developer
 
-        return new ApexQualifiedName(parent.nameSpace, parent.classes, getOperationString(node));
+        } else {
+            ApexQualifiedName baseName = parent.getQualifiedName();
+
+            return new ApexQualifiedName(baseName.nameSpace, baseName.classes, getOperationString(node));
+        }
     }
 }
diff --git a/...pex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CyclomaticComplexityRule.java b/...pex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CyclomaticComplexityRule.java
@@ -9,28 +9,38 @@
 
 import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
 import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
+import net.sourceforge.pmd.lang.apex.ast.ASTUserTrigger;
 import net.sourceforge.pmd.lang.apex.metrics.ApexMetrics;
 import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey;
 import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
 import net.sourceforge.pmd.lang.metrics.ResultOption;
 import net.sourceforge.pmd.properties.IntegerProperty;
 
+
 /**
- * Cyclomatic complexity rule using metrics. Uses Wmc to report classes (the Java rule will be updated as well in an
- * upcoming PR)
- *
+ * Cyclomatic complexity rule using metrics. Uses Wmc to report classes.
+ * 
  * @author Clément Fournier
  */
 public class CyclomaticComplexityRule extends AbstractApexRule {
 
-    private static final IntegerProperty CLASS_LEVEL_DESCRIPTOR = new IntegerProperty(
-        "classReportLevel", "Total class complexity reporting threshold", 1, 200, 40, 1.0f);
+    private static final IntegerProperty CLASS_LEVEL_DESCRIPTOR
+        = IntegerProperty.named("classReportLevel")
+                         .desc("Total class complexity reporting threshold")
+                         .range(1, 200)
+                         .defaultValue(40)
+                         .uiOrder(1.0f).build();
 
-    private static final IntegerProperty METHOD_LEVEL_DESCRIPTOR = new IntegerProperty(
-        "methodReportLevel", "Cyclomatic complexity reporting threshold", 1, 30, 10, 1.0f);
+    private static final IntegerProperty METHOD_LEVEL_DESCRIPTOR 
+        = IntegerProperty.named("methodReportLevel")
+                         .desc("Cyclomatic complexity reporting threshold")
+                         .range(1, 30)
+                         .defaultValue(10)
+                         .uiOrder(2.0f).build();
 
-    Stack<String> classNames = new Stack<>();
+    private Stack<String> classNames = new Stack<>();
+    private boolean inTrigger;
 
 
     public CyclomaticComplexityRule() {
@@ -39,6 +49,15 @@ public CyclomaticComplexityRule() {
     }
 
 
+    @Override
+    public Object visit(ASTUserTrigger node, Object data) {
+        inTrigger = true;
+        super.visit(node, data);
+        inTrigger = false;
+        return data;
+    }
+
+
     @Override
     public Object visit(ASTUserClass node, Object data) {
 
@@ -69,10 +88,14 @@ public final Object visit(ASTMethod node, Object data) {
 
         int cyclo = (int) ApexMetrics.get(ApexOperationMetricKey.CYCLO, node);
         if (cyclo >= getProperty(METHOD_LEVEL_DESCRIPTOR)) {
-            addViolation(data, node, new String[] {node.getImage().equals(classNames.peek()) ? "constructor" : "method",
-                                                   node.getQualifiedName().getOperation(),
-                                                   "",
-                                                   "" + cyclo, });
+            String opType = inTrigger ? "trigger"
+                                      : node.getImage().equals(classNames.peek()) ? "constructor"
+                                                                                  : "method";
+
+            addViolation(data, node, new String[]{opType,
+                                                  node.getQualifiedName().getOperation(),
+                                                  "",
+                                                  "" + cyclo, });
         }
 
         return data;

diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexQualifiedNameTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexQualifiedNameTest.java
@@ -15,6 +15,7 @@
 
 import apex.jorje.semantic.ast.compilation.Compilation;
 
+
 /**
  * @author Clément Fournier
  */
@@ -69,9 +70,9 @@ public void testMethodWithArguments() {
     @Test
     public void testOverLoads() {
         ApexNode<Compilation> root = ApexParserTestHelpers.parse("public class Foo { "
-                                                                     + "String foo(String h) {} "
-                                                                     + "String foo(int c) {}"
-                                                                     + "String foo(Foo c) {}}");
+                                                                 + "String foo(String h) {} "
+                                                                 + "String foo(int c) {}"
+                                                                 + "String foo(Foo c) {}}");
 
         List<ASTMethod> methods = root.findDescendantsOfType(ASTMethod.class);
 
@@ -83,4 +84,17 @@ public void testOverLoads() {
             }
         }
     }
+
+
+    @Test
+    public void testTrigger() {
+        ApexNode<Compilation> root = ApexParserTestHelpers.parse("trigger myAccountTrigger on Account (before insert, before update) {}");
+
+
+        List<ASTMethod> methods = root.findDescendantsOfType(ASTMethod.class);
+
+        for (ASTMethod m : methods) {
+            assertNotEquals("c__trigger#myAccountTrigger", m.getQualifiedName());
+        }
+    }
 }
diff --git a/...src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml b/...src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml
@@ -251,5 +251,52 @@
         </expected-messages>
         <code-ref id="many-small-methods"/>
     </test-code>
+
+
+    <code-fragment id="trigger">
+        <![CDATA[
+            trigger CaseAssignLevel on CaseAssignLevel__c (after delete, after insert, after undelete, after update, before delete, before insert, before update) {
+            
+            if(Trigger.isBefore) {
+                if(Trigger.isInsert || Trigger.isUpdate) {
+                    CaseAssignLevel_tr.doIsNotTrigger(Trigger.new);
+                }
+            }
+            
+            
+            
+            if(Trigger.isBefore && !CaseAssignLevel_tr.isNotTrigger) {
+                if(Trigger.isInsert || Trigger.isUpdate) {
+                    CaseAssignLevel_tr.doCreateKeyValue(Trigger.new, Trigger.oldMap);	
+                }
+            }
+            
+            
+            else if(Trigger.isAfter && !CaseAssignLevel_tr.isNotTrigger) {
+                if(Trigger.isInsert || Trigger.isUpdate) {
+                    
+                }
+            }
+            
+            }
+        ]]>
+    </code-fragment>
+
+    <test-code>
+        <description>#768 NPE caused by exception in ApexQualifiedName.ofMethod because of trigger</description>
+        <expected-problems>1</expected-problems>
+        <expected-messages>
+            <message>The trigger 'CaseAssignLevel' has a cyclomatic complexity of 12.</message>
+        </expected-messages>
+        <code-ref id="trigger"/>
+    </test-code>
+
+    <test-code>
+        <description>#768 NPE caused by exception in ApexQualifiedName.ofMethod because of trigger</description>
+        <rule-property name="methodReportLevel">13</rule-property>
+        <expected-problems>0</expected-problems>
+        <code-ref id="trigger"/>
+    </test-code>
+
 
-</test-data>
+</test-data>