diff --git a/.all-contributorsrc b/.all-contributorsrc index 8a891884e5..72a0c129d0 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -7517,6 +7517,7 @@ "avatar_url": "https://avatars.githubusercontent.com/u/18402464?v=4", "profile": "https://github.com/mitchspano", "contributions": [ + "code", "bug" ] }, diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 496f8a26cb..c6576be241 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,10 +14,18 @@ This is a {{ site.pmd.release_type }} release. ### 🚀 New and noteworthy +### 🌟 New and changed rules + +#### New Rules +* The new Apex rule {% rule apex/bestpractices/QueueableWithoutFinalizer %} detects when the Queueable interface + is used but a Finalizer is not attached. Without attaching a Finalizer, there is no way of designing error + recovery actions should the Queueable action fail. + ### 🐛 Fixed Issues * ant * [#1860](https://github.com/pmd/pmd/issues/1860): \[ant] Reflective access warnings on java > 9 and java < 17 * apex + * [#5302](https://github.com/pmd/pmd/issues/5302): \[apex] New Rule: Queueable Should Attach Finalizer * [#5333](https://github.com/pmd/pmd/issues/5333): \[apex] Token recognition errors for string containing unicode escape sequence * html * [#5322](https://github.com/pmd/pmd/issues/5322): \[html] CPD throws exception on when HTML file is missing closing tag @@ -42,6 +50,7 @@ This is a {{ site.pmd.release_type }} release. ### ✨ External Contributions * [#5284](https://github.com/pmd/pmd/pull/5284): \[apex] Use case-insensitive input stream to avoid choking on Unicode escape sequences - [Willem A. Hajenius](https://github.com/wahajenius) (@wahajenius) +* [#5303](https://github.com/pmd/pmd/pull/5303): \[apex] New Rule: Queueable Should Attach Finalizer - [Mitch Spano](https://github.com/mitchspano) (@mitchspano) {% endtocmaker %} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerRule.java new file mode 100644 index 0000000000..9acb39e07d --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerRule.java @@ -0,0 +1,92 @@ +/* + * 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 org.checkerframework.checker.nullness.qual.NonNull; + +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; +import net.sourceforge.pmd.lang.rule.RuleTargetSelector; + +/** + * 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 QueueableWithoutFinalizerRule 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"; + + @Override + protected @NonNull RuleTargetSelector buildTargetSelector() { + return RuleTargetSelector.forTypes(ASTUserClass.class); + } + + /** + * 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. + */ + @Override + public Object visit(ASTUserClass theClass, Object data) { + if (!implementsTheQueueableInterface(theClass)) { + return data; + } + for (ASTMethod theMethod : theClass.descendants(ASTMethod.class).toList()) { + if (isTheExecuteMethodOfTheQueueableInterface(theMethod) + && !callsTheSystemAttachFinalizerMethod(theMethod)) { + asCtx(data).addViolation(theMethod); + } + } + return data; + } + + /** Determines if the class implements the Queueable interface. */ + private boolean implementsTheQueueableInterface(ASTUserClass theClass) { + for (String interfaceName : theClass.getInterfaceNames()) { + if (QUEUEABLE.equalsIgnoreCase(interfaceName)) { + 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 (!EXECUTE.equalsIgnoreCase(theMethod.getCanonicalName())) { + return false; + } + List parameters = theMethod.descendants(ASTParameter.class).toList(); + return parameters.size() == 1 && QUEUEABLE_CONTEXT.equalsIgnoreCase(parameters.get(0).getType()); + } + + /** + * 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 (SYSTEM_ATTACH_FINALIZER.equalsIgnoreCase(methodCallExpression.getFullMethodName())) { + return true; + } + } + return false; + } +} diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index f2d723cb8c..836894f3b5 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -285,4 +285,56 @@ Detects when a local variable is declared and/or assigned but not used. + + +Detects when the Queueable interface is used but a Finalizer is not attached. +It is best practice to call the `System.attachFinalizer(Finalizer f)` method within the `execute` method of a class which implements the `Queueable` interface. +Without attaching a Finalizer, there is no way of designing error recovery actions should the Queueable action fail. + + 5 + + usersToUpdate; + + public UserUpdater(List usersToUpdate) { + this.usersToUpdate = usersToUpdate; + } + + public void execute(QueueableContext context) { // no Finalizer is attached + update usersToUpdate; + } +} + +// Proper code, attaches a finalizer. +public class UserUpdater implements Queueable, Finalizer { + public List usersToUpdate; + + public UserUpdater(List 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 + } + } +} +]]> + + + diff --git a/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml b/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml index 83774146c1..53c63c7546 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml @@ -209,6 +209,7 @@ 3 + diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerTest.java new file mode 100644 index 0000000000..7daa72c169 --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/QueueableWithoutFinalizerTest.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 QueueableWithoutFinalizerTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/QueueableWithoutFinalizer.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/QueueableWithoutFinalizer.xml new file mode 100644 index 0000000000..41145ef966 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/QueueableWithoutFinalizer.xml @@ -0,0 +1,51 @@ + + + + + [apex] Queueable Without Finalizer - positive test case #5302 + 1 + 8 + usersToUpdate; + + public UserUpdater(List usersToUpdate) { + this.usersToUpdate = usersToUpdate; + } + + public void execute(QueueableContext context) { + update usersToUpdate; + } +} + ]]> + + + [apex] Queueable Without Finalizer - negative test case #5302 + 0 + usersToUpdate; + + public UserUpdater(List 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 + } + } +} + ]]> + + \ No newline at end of file