Apex PMD: Avoid empty block statements

Question

Perhaps this isn't the place for this -- but it looks like a good place to start. I'm using Apex PMD code analysis (within Visual Studio Code) and it's throwing off the following for an interface implementation I am developing:

Avoid empty block statements.

The interface (Obviously, a trigger framework) looks like this:

public interface TriggerHandler {
    void beforeInsert(List<SObject> newItems);
    void beforeUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems);
    void beforeDelete(Map<Id, SObject> oldItems);
    void afterInsert(Map<Id, SObject> newItems);
    void afterUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems);
    void afterDelete(Map<Id, SObject> oldItems);
    void afterUndelete(Map<Id, SObject> oldItems);
    Boolean isDisabled();
}

Let's say I have a very simple implementation where I only want to check an account name before it's inserted:

public class AccountTriggerHandler implements TriggerHandler {
    public static Boolean triggerDisabled = false;
    public Boolean isDisabled() {
        return triggerDisabled;
    }
    public void beforeInsert(List<SObject> newItems) {
        accountNameCheck((List<Account>)newitems);
    }
    public void beforeUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems) {}
    public void beforeDelete(Map<Id, SObject> oldItems) {}
    public void afterInsert(Map<Id, SObject> newItems) {}
    public void afterUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems) {}
    public void afterDelete(Map<Id, SObject> oldItems) {}
    public void afterUndelete(Map<Id, SObject> oldItems) {}
    /**
    * Check the accounts to make sure their name does not contain the text "test".
    * If any do, reject them.
    */
    private void accountNameCheck(List<Account> accountList) {
        // Reject any Accounts which have the word "Test" in the name
        for (Account acc : accountList) {
            if (acc.Name.toLowerCase().containsOnly('test')) {
                acc.Name.addError('You may not use the word "test" in the account name');
            }
        }
    }
}

I don't plan to on any other account trigger actions at this time, so I simply leave the other methods empty. Doing so triggers the Apex PMD notice above. How would I set things up so I can avoid the Apex PMD error?

NOTE: Not sure why, but if I change:

public class AccountTriggerHandler implements TriggerHandler

to

public inherited sharingclass AccountTriggerHandler implements TriggerHandler

the PMD notice/error goes away.

Answer 1

Static code analysis simply gives you suggestions of places to look at improving your code. It's nice to have a clear report here, but it isn't necessary to take care of every item.

This particular case, empty blocks in a trigger framework, I'd probably just ignore.

If you really do feel the need to resolve this, then I'd suggest using an abstract class to provide a default implementation for all of the trigger context methods so that you only need to include the methods in your concrete trigger handler classes that will have a non-empty body.

Given your interface

public interface TriggerHandler {
    void beforeInsert(List<SObject> newItems);
    void beforeUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems);
    void beforeDelete(Map<Id, SObject> oldItems);
    void afterInsert(Map<Id, SObject> newItems);
    void afterUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems);
    void afterDelete(Map<Id, SObject> oldItems);
    void afterUndelete(Map<Id, SObject> oldItems);
    Boolean isDisabled();
}

Implementing my suggestion would look something like this

// The "abstract" keyword allows us to inherit from this class, but not construct an
//   instance of _this class_.
// I.e. DefaultTriggerHandler handler = new DefaultTriggerHandler(); is not allowed
public abstract class DefaultTriggerHandler implements TriggerHandler{

    // A method with the "virtual" keyword can be overridden in child classes, but still
    //   allows us to provide a default implementation.
    // If this were an "abstract" method, we would not be able to provide a default
    //   implementation (just a method signature).
    // Abstract methods are also inherited, the big difference being that a child class
    //   must override _all_ of its parent's abstract methods
    public virtual void beforeInsert(List<SObject> newItems){
        system.debug('Using default implementation of beforeInsert, returning');
        return;
    }

    // As another note, interface implementations must match the signature provided
    //   in the interface itself.
    // The exception to this are the "virtual", "abstract", and "override" modifiers,
    //   which we can add to the signature (as we're doing here)
    public virtual void beforeUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems){
        system.debug('Using default implementation of beforeUpdate, returning');
        return;
    }

    public virtual void beforeDelete(Map<Id, SObject> oldItems){
        system.debug('Using default implementation of beforeDelete, returning');
        return;
    }

    public virtual void afterInsert(Map<Id, SObject> newItems){
        system.debug('Using default implementation of afterInsert, returning');
        return;
    }

    public virtual void afterUpdate(Map<Id, SObject> newItems, Map<Id, SObject> oldItems){
        system.debug('Using default implementation of afterUpdate, returning');
        return;
    }

    public virtual void afterDelete(Map<Id, SObject> oldItems){
        system.debug('Using default implementation of afterDelete, returning');
        return;
    }

    public virtual void afterUndelete(Map<Id, SObject> oldItems){
        system.debug('Using default implementation of afterUndelete, returning');
        return;
    }

    public virtual Boolean isDisabled(){
        return true;
    }
}

// Interfaces are also inherited
// Since DefaultTriggerHandler implements TriggerHandler, AccountTriggerHandler 
//   implicitly implements that interface as well
// You can still explicitly add "implements TriggerHandler" to this class, it's just 
//   not strictly required.
public class AccountTriggerHandler extends DefaultTriggerHandler{
    public override void beforeInsert(List<SObject> newItems){
        system.debug('Using concrete implementation of beforeInsert');
        // your logic here
        return;
    }
}

Answer 2

Disclaimer: I am a bit biased because I original ported PMD from Java to Apex and also created this rule.

Derek already answered tghe question 99% but I want to add two things.

1. PMD is not perfect and there are always false positves (PMD marks code as issue while it is not). You can eighter create an issue in PMDs Github repo or fix it yourself as part of a pull request.

2. I think this is not a false positive but a very seldom case that the rule cannot really recognize. You can just ignore such cases by putting the @SuppressWarnings annotation as described here: https://pmd.github.io/pmd-6.8.0/pmd_userdocs_suppressing_warnings.html