Queueable Should Attach Finalizer

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableShouldAttachFinalizerRule.java

@@ -0,0 +1,87 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.apex.rule.bestpractices;
+
+import java.util.List;
+import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
+import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
+import net.sourceforge.pmd.lang.apex.ast.ASTParameter;
+import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
+import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
+
+/**
+ * Scans classes which implement the `Queueable` interface. If the `public void
+ * execute(QueueableContext context)` method does not call the `System.attachFinalizer(Finalizer f)`
+ * method, then a violation will be added to the `execute` method.
+ *
+ * @author mitchspano
+ */
+public class QueueableShouldAttachFinalizerRule extends AbstractApexRule {
+
+  private static final String EXECUTE = "execute";
+  private static final String QUEUEABLE = "queueable";
+  private static final String QUEUEABLE_CONTEXT = "queueablecontext";
+  private static final String SYSTEM_ATTACH_FINALIZER = "system.attachfinalizer";
+
+  /** Scans the top level class and all inner classes. */
+  @Override
+  public Object visit(ASTUserClass topLevelClass, Object data) {
+    scanClassForViolation(topLevelClass, data);
+    for (ASTUserClass innerClass : topLevelClass.descendants(ASTUserClass.class).toList()) {
+      scanClassForViolation(innerClass, data);
+    }
+    return data;
+  }
+
+  /**
+   * If the class implements the `Queueable` interface and the `execute(QueueableContext context)`
+   * does not call the `System.attachFinalizer(Finalizer f)` method, then add a violation.
+   */
+  private void scanClassForViolation(ASTUserClass theClass, Object data) {
+    if (!implementsTheQueueableInterface(theClass)) {
+      return;
+    }
+    for (ASTMethod theMethod : theClass.descendants(ASTMethod.class).toList()) {
+      if (isTheExecuteMethodOfTheQueueableInterface(theMethod)
+          && !callsTheSystemAttachFinalizerMethod(theMethod)) {
+        asCtx(data).addViolation(theMethod);
+      }
+    }
+  }
+
+  /** Determines if the class implements the Queueable interface. */
+  private boolean implementsTheQueueableInterface(ASTUserClass theClass) {
+    for (String interfaceName : theClass.getInterfaceNames()) {
+      if (interfaceName.equalsIgnoreCase(QUEUEABLE)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Determines if the method is the `execute(QueueableContext context)` method. Parameter count is
+   * checked to account for method overloading.
+   */
+  private boolean isTheExecuteMethodOfTheQueueableInterface(ASTMethod theMethod) {
+    if (!theMethod.getCanonicalName().equalsIgnoreCase(EXECUTE)) {
+      return false;
+    }
+    List<ASTParameter> parameters = theMethod.descendants(ASTParameter.class).toList();
+    return parameters.size() == 1
+        && parameters.get(0).getType().equalsIgnoreCase(QUEUEABLE_CONTEXT);
+  }
+
+  /** Determines if the method calls the `System.attachFinalizer(Finalizer f)` method. */
+  private boolean callsTheSystemAttachFinalizerMethod(ASTMethod theMethod) {
+    for (ASTMethodCallExpression methodCallExpression :
+        theMethod.descendants(ASTMethodCallExpression.class).toList()) {
+      if (methodCallExpression.getFullMethodName().equalsIgnoreCase(SYSTEM_ATTACH_FINALIZER)) {
+        return true;
+      }
+    }
+    return false;
+  }
+}

pmd-apex/src/main/resources/category/apex/bestpractices.xml

@@ -285,4 +285,30 @@ Detects when a local variable is declared and/or assigned but not used.
         </example>
     </rule>
 
+    <rule name="QueueableShouldAttachFinalizer"
+          since="7.8.0"
+          language="apex"
+          message="It is best practice to call the `System.attachFinalizer(Finalizer f)` method within the `execute` method of a class which implements the `Queueable` interface."
+          class="net.sourceforge.pmd.lang.apex.rule.bestpractices.QueueableShouldAttachFinalizerRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_bestpractices.html#queueableshouldattachfinalizer">
+        <description>
+Detects when the Queueable interface is used but a Finalizer is not attached.
+        </description>
+        <example>
+<![CDATA[
+    public class UserUpdater implements Queueable {
+        public List<User> usersToUpdate;
+
+        public UserUpdater(List<User> usersToUpdate) {
+            this.usersToUpdate = usersToUpdate;
+        }
+
+        public void execute(QueueableContext context) { // no Finalizer is attached
+            update usersToUpdate;
+        }
+    }
+]]>
+        </example>
+    </rule>
+
 </ruleset>

pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableShouldAttachFinalizerTest.java

@@ -0,0 +1,11 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.apex.rule.bestpractices;
+
+import net.sourceforge.pmd.test.PmdRuleTst;
+
+class QueueableShouldAttachFinalizerTest extends PmdRuleTst {
+  // no additional unit tests
+}

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/QueueableShouldAttachFinalizer.xml

@@ -0,0 +1,51 @@
+<?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>[apex] Queueable Should Attach Finalizer - positive test case #5302</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>8</expected-linenumbers>
+        <code><![CDATA[
+public class UserUpdater implements Queueable {
+    public List<User> usersToUpdate;
+
+    public UserUpdater(List<User> usersToUpdate) {
+        this.usersToUpdate = usersToUpdate;
+    }
+
+    public void execute(QueueableContext context) {
+        update usersToUpdate;
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>[apex] Queueable Should Attach Finalizer - negative test case #5302</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class UserUpdater implements Queueable, Finalizer {
+    public List<User> usersToUpdate;
+
+    public UserUpdater(List<User> usersToUpdate) {
+        this.usersToUpdate = usersToUpdate;
+    }
+
+    public void execute(QueueableContext context) {
+        System.attachFinalizer(this);
+        update usersToUpdate;
+    }
+
+    public void execute(FinalizerContext ctx) {
+        if (ctx.getResult() == ParentJobResult.SUCCESS) {
+            // Handle success
+        } else {
+            // Handle failure
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+</test-data>