# Error Prone

Rules to detect constructs that are either broken, extremely confusing or prone to runtime errors.
Table of Contents

## ApexCSRF

Since: PMD 5.5.3

Priority: Medium (3)

Having DML operations in Apex class constructor or initializers can have unexpected side effects: By just accessing a page, the DML statements would be executed and the database would be modified. Just querying the database is permitted.

In addition to constructors and initializers, any method called `init` is checked as well.

Salesforce Apex already protects against this scenario and raises a runtime exception.

Note: This rule has been moved from category "Security" to "Error Prone" with PMD 6.21.0, since using DML in constructors is not a security problem, but crashes the application.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.ApexCSRFRule

Example(s):

    public class Foo {
        // initializer
        {
            insert data;
        }

        // static initializer
        static {
            insert data;
        }

        // constructor
        public Foo() {
            insert data;
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/ApexCSRF" />

## 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.

This rule is defined by the following XPath expression:

    //ArrayLoadExpression[TriggerVariableExpression and LiteralExpression]

Example(s):

    trigger AccountTrigger on Account (before insert, before update) {
       Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.

       for ( Account a : Trigger.new ) {
            //Good: Iterate through the trigger.new array instead.
       }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/AvoidDirectAccessTriggerMap" />

## AvoidHardcodingId

Since: PMD 6.0.0

Priority: Medium (3)

When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, the logic can dynamically identify the proper data to operate against and not fail.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidHardcodingIdRule

Example(s):

    public without sharing class Foo {
        void foo() {
            //Error - hardcoded the record type id
            if (a.RecordTypeId == '012500000009WAr') {
                //do some logic here.....
            } else if (a.RecordTypeId == '0123000000095Km') {
                //do some logic here for a different record type...
            }
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/AvoidHardcodingId" />

## AvoidNonExistentAnnotations

Since: PMD 6.5.0

Priority: Medium (3)

Apex supported non existent annotations for legacy reasons. In the future, use of such non-existent annotations could result in broken apex code that will not compile. This will prevent users of garbage annotations from being able to use legitimate annotations added to Apex in the future. A full list of supported annotations can be found at https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_annotation.htm

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidNonExistentAnnotationsRule

Example(s):

    @NonExistentAnnotation public class ClassWithNonexistentAnnotation {
        @NonExistentAnnotation public void methodWithNonExistentAnnotation() {
            // ...
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/AvoidNonExistentAnnotations" />

## AvoidStatefulDatabaseResult

Since: PMD 7.11.0

Priority: Medium High (2)

Using instance variables of the following types (or collections of these types) within a stateful batch class can cause serialization errors between batch iterations:

- `Database.DeleteResult`
- `Database.EmptyRecycleBinResult`
- `Database.MergeResult`
- `Database.SaveResult`
- `Database.UndeleteResult`
- `Database.UpsertResult`

This error occurs inconsistently and asynchronously with an obscure error message - making it particularly challenging to troubleshoot. See this issue for more details.

These errors can be avoided by marking the variable as static, transient, or using a different data type that is safe to serialize.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidStatefulDatabaseResultRule

Example(s):

    // Violating
    public class Example implements Database.Batchable<SObject>, Database.Stateful {
      List<Database.SaveResult> results = new List<Database.SaveResult>(); // This can cause failures

      public Database.Querylocator start(Database.BatchableContext context) {
        return Database.getQueryLocator('SELECT Id FROM Account');
      }

      public void execute(Database.BatchableContext context, List<SObject> scope) {
        Database.SaveResult[] saveResults = Database.update(scope, false);
        results.addAll(saveResults);
      }

      public void finish(database.BatchableContext context) {
      }
    }

    // Compliant
    public class Example implements Database.Batchable<SObject>, Database.Stateful {
      List<StatefulResult> results = new List<StatefulResult>(); // Use a different custom type to persist state

      public Database.Querylocator start(Database.BatchableContext context) {
        return Database.getQueryLocator('SELECT Id FROM Account');
      }

      public void execute(Database.BatchableContext context, List<SObject> scope) {
        Database.SaveResult[] saveResults = Database.update(scope, false);
        for (Database.SaveResult result : saveResults) {
          results.add(new StatefulResult(result));
        }
      }

      public void finish(database.BatchableContext context) {
      }

    }

    public class StatefulResult {
      private Boolean isSuccess;
      private Id id;
      private Database.Error[] errors;

      public StatefulResult(Database.SaveResult result) {
        isSuccess = result.isSuccess();
        id = result.getId();
        errors = result.getErrors();
      }

      public Boolean isSuccess() {
        return isSuccess;
      }

      public Id getId() {
        return id;
      }

      public Database.Error[] getErrors() {
        return errors;
      }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/AvoidStatefulDatabaseResult" />

## EmptyCatchBlock

Since: PMD 6.0.0

Priority: Medium (3)

Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this swallows an exception which should either be acted on or reported.

This rule is defined by the following XPath expression:

    //CatchBlockStatement[./BlockStatement[count(*) = 0] and
        not(matches(@VariableName, $allowExceptionNameRegex)) and
        ($allowCommentedBlocks = false() or @ContainsComment = false())]

Example(s):

    public void doSomething() {
        ...
        try {
            insert accounts;
        } catch (DmlException dmle) {
            // not good
        }
    }

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
allowCommentedBlocks | false | Empty blocks containing comments will be skipped
allowExceptionNameRegex | ^(ignored|expected)$ | Empty blocks catching exceptions with names matching this regular expression will be skipped

Use this rule with the default properties by just referencing it:

    <rule ref="category/apex/errorprone.xml/EmptyCatchBlock" />

Use this rule and customize it:

    <rule ref="category/apex/errorprone.xml/EmptyCatchBlock">
        <properties>
            <property name="allowCommentedBlocks" value="false" />
            <property name="allowExceptionNameRegex" value="^(ignored|expected)$" />
        </properties>
    </rule>

## EmptyIfStmt

Since: PMD 6.0.0

Priority: Medium (3)

Empty If Statement finds instances where a condition is checked but nothing is done about it.

This rule is defined by the following XPath expression:

    //IfBlockStatement
              [BlockStatement[count(*) = 0]]

Example(s):

    public class Foo {
        public void bar(Integer x) {
            if (x == 0) {
                // empty!
            }
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/EmptyIfStmt" />

## EmptyStatementBlock

Since: PMD 6.0.0

Priority: Medium (3)

Empty block statements serve no purpose and should be removed.

This rule is defined by the following XPath expression:

    //Method[$reportEmptyPrivateNoArgConstructor = true()
        or (@Constructor != true()
        or ./ModifierNode[@Private != true()]
        or ./Parameter[count(*) > 0])]/ModifierNode[@Abstract != true()
            and ($reportEmptyVirtualMethod = true() or @Virtual != true())
            and ../BlockStatement[count(*) = 0]
        ]
    | //Method/BlockStatement//BlockStatement
        [not(parent::CatchBlockStatement)]
        [count(*) = 0]
        [@RealLoc = true()]

Example(s):

    public class Foo {

       private Integer _bar;

       public void setBar(Integer bar) {
            // empty
       }

    }

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
reportEmptyPrivateNoArgConstructor | true | If false, empty private no-arg constructors are not flagged. This supports a common idiom used by singleton pattern implementations, utility classes, etc.
reportEmptyVirtualMethod | true | If false, empty virtual methods are not flagged. This supports abstract base classes with default no-op implementations.

Use this rule with the default properties by just referencing it:

    <rule ref="category/apex/errorprone.xml/EmptyStatementBlock" />

Use this rule and customize it:

    <rule ref="category/apex/errorprone.xml/EmptyStatementBlock">
        <properties>
            <property name="reportEmptyPrivateNoArgConstructor" value="true" />
            <property name="reportEmptyVirtualMethod" value="true" />
        </properties>
    </rule>

## EmptyTryOrFinallyBlock

Since: PMD 6.0.0

Priority: Medium (3)

Avoid empty try or finally blocks - what’s the point?

This rule is defined by the following XPath expression:

    //TryCatchFinallyBlockStatement[./BlockStatement[count(*) = 0]]

Example(s):

    public class Foo {
        public void bar() {
            try {
              // empty !
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    }

    public class Foo {
        public void bar() {
            try {
                Integer x=2;
            } finally {
                // empty!
            }
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/EmptyTryOrFinallyBlock" />

## EmptyWhileStmt

Since: PMD 6.0.0

Priority: Medium (3)

Empty While Statement finds all instances where a while statement does nothing. If it is a timing loop, then you should use Thread.sleep() for it; if it is a while loop that does a lot in the exit expression, rewrite it to make it clearer.

This rule is defined by the following XPath expression:

    //WhileLoopStatement[./BlockStatement[count(*) = 0]]

Example(s):

    public void bar(Integer a, Integer b) {
      while (a == b) {
        // empty!
      }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/EmptyWhileStmt" />

## InaccessibleAuraEnabledGetter

Since: PMD 6.36.0

Priority: Medium (3)

In the Summer ‘21 release, a mandatory security update enforces access modifiers on Apex properties in Lightning component markup. The update prevents access to private or protected Apex getters from Aura and Lightning Web Components.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.InaccessibleAuraEnabledGetterRule

Example(s):

    public class Foo {
        @AuraEnabled
        public Integer counter { private get; set; } // Violating - Private getter is inaccessible to Lightning components

        @AuraEnabled
        public static Foo bar()
        {
            Foo foo = new Foo();
            foo.counter = 2;
            return foo;
        }
    }

    public class Foo {
        @AuraEnabled
        public Integer counter { protected get; set; } // Violating - Protected getter is inaccessible to Lightning components

        @AuraEnabled
        public static Foo bar()
        {
            Foo foo = new Foo();
            foo.counter = 2;
            return foo;
        }
    }

    public class Foo {
        @AuraEnabled
        public Integer counter { get; set; } // Compliant - Public getter is accessible to Lightning components

        @AuraEnabled
        public static Foo bar()
        {
            Foo foo = new Foo();
            foo.counter = 2;
            return foo;
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/InaccessibleAuraEnabledGetter" />

## MethodWithSameNameAsEnclosingClass

Since: PMD 5.5.0

Priority: Medium (3)

Non-constructor methods should not have the same name as the enclosing class.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.MethodWithSameNameAsEnclosingClassRule

Example(s):

    public class MyClass {
        // this is OK because it is a constructor
        public MyClass() {}
        // this is bad because it is a method
        public void MyClass() {}
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/MethodWithSameNameAsEnclosingClass" />

## OverrideBothEqualsAndHashcode

Since: PMD 6.31.0

Priority: Medium (3)

Override both `public Boolean equals(Object obj)`, and `public Integer hashCode()`, or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass.

This is especially important when Using Custom Types in Map Keys and Sets.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.OverrideBothEqualsAndHashcodeRule

Example(s):

    public class Bar {        // poor, missing a hashCode() method
        public Boolean equals(Object o) {
          // do some comparison
        }
    }
    public class Baz {        // poor, missing an equals() method
        public Integer hashCode() {
          // return some hash value
        }
    }
    public class Foo {        // perfect, both methods provided
        public Boolean equals(Object other) {
          // do some comparison
        }
        public Integer hashCode() {
          // return some hash value
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/OverrideBothEqualsAndHashcode" />

## TestMethodsMustBeInTestClasses

Since: PMD 6.22.0

Priority: Medium (3)

Test methods marked as a testMethod or annotated with @IsTest, but not residing in a test class should be moved to a proper class or have the @IsTest annotation added to the class.

Support for tests inside functional classes was removed in Spring-13 (API Version 27.0), making classes that violate this rule fail compile-time. This rule is mostly usable when dealing with legacy code.

This rule is defined by the following XPath expression:

    //UserClass[
          not(./ModifierNode/Annotation[lower-case(@Name) = 'istest']) and
          (
            (./Method/ModifierNode/Annotation[lower-case(@Name) = 'istest']) or
            (./Method/ModifierNode[@Test = true()])
          )
        ]

Example(s):

    // Violating
    private class TestClass {
      @IsTest static void myTest() {
        // Code here
      }
    }

    private class TestClass {
      static testMethod void myTest() {
        // Code here
      }
    }

    // Compliant
    @IsTest
    private class TestClass {
      @IsTest static void myTest() {
        // Code here
      }
    }

    @IsTest
    private class TestClass {
      static testMethod void myTest() {
        // Code here
      }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/TestMethodsMustBeInTestClasses" />

## TypeShadowsBuiltInNamespace

Since: PMD 7.13.0

Priority: High (1)

This rule finds Apex classes, enums, and interfaces that have the same name as a class, enum, or interface in the `System` or `Schema` namespace. Shadowing these namespaces in this way can lead to confusion and unexpected behavior. Code that intends to reference a `System` or `Schema` class, enum, or interface may inadvertently reference the locally defined type instead. This can result in ambiguous code and unexpected runtime behavior. It is best to avoid naming your types the same as those in the `System` or `Schema` namespace to prevent these issues.

Note that the list of classes, enums, and interfaces in the `System` and `Schema` namespaces are determined through io.github.apex-dev-tools:standard-types. It is based on the contents of Salesforce’s Apex Reference Guide / System Namespace and Apex Reference Guide / Schema Namespace. As Salesforce introduces new types into the `System` and `Schema` namespaces, the rule might not always recognize the new types and produce false-negatives und the standard types are updated.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.apex.rule.errorprone.TypeShadowsBuiltInNamespaceRule

Example(s):

    // Violation: Causes a collision with the `System.Database` class.
    public class Database {
        public static String query() {
            return 'Hello World';
        }
    }

Use this rule by referencing it:

    <rule ref="category/apex/errorprone.xml/TypeShadowsBuiltInNamespace" />