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 calledinit 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, sinceusing 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):
publicclassFoo{// initializer{insertdata;}// static initializerstatic{insertdata;}// constructorpublicFoo(){insertdata;}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/ApexCSRF"/>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[TriggerVariableExpressionandLiteralExpression]Example(s):
triggerAccountTriggeronAccount(beforeinsert,beforeupdate){Accounta=Trigger.new[0];//Bad: Accessing the trigger array directly is not recommended.for(Accounta:Trigger.new){//Good: Iterate through the trigger.new array instead.}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/AvoidDirectAccessTriggerMap"/>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):
publicwithoutsharingclassFoo{voidfoo(){//Error - hardcoded the record type idif(a.RecordTypeId=='012500000009WAr'){//do some logic here.....}elseif(a.RecordTypeId=='0123000000095Km'){//do some logic here for a different record type...}}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/AvoidHardcodingId"/>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):
@NonExistentAnnotationpublicclassClassWithNonexistentAnnotation{@NonExistentAnnotationpublicvoidmethodWithNonExistentAnnotation(){// ...}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/AvoidNonExistentAnnotations"/>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.DeleteResultDatabase.EmptyRecycleBinResultDatabase.MergeResultDatabase.SaveResultDatabase.UndeleteResultDatabase.UpsertResultThis error occurs inconsistently and asynchronously with an obscure error message - making it particularly challenging to troubleshoot.Seethis issue for more details.
These errors can be avoided by marking the variable as static, transient, or using a differentdata 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):
// ViolatingpublicclassExampleimplementsDatabase.Batchable<SObject>,Database.Stateful{List<Database.SaveResult>results=newList<Database.SaveResult>();// This can cause failurespublicDatabase.Querylocatorstart(Database.BatchableContextcontext){returnDatabase.getQueryLocator('SELECTIdFROMAccount');}publicvoidexecute(Database.BatchableContextcontext,List<SObject>scope){Database.SaveResult[]saveResults=Database.update(scope,false);results.addAll(saveResults);}publicvoidfinish(database.BatchableContextcontext){}}// CompliantpublicclassExampleimplementsDatabase.Batchable<SObject>,Database.Stateful{List<StatefulResult>results=newList<StatefulResult>();// Use a different custom type to persist statepublicDatabase.Querylocatorstart(Database.BatchableContextcontext){returnDatabase.getQueryLocator('SELECTIdFROMAccount');}publicvoidexecute(Database.BatchableContextcontext,List<SObject>scope){Database.SaveResult[]saveResults=Database.update(scope,false);for(Database.SaveResultresult:saveResults){results.add(newStatefulResult(result));}}publicvoidfinish(database.BatchableContextcontext){}}publicclassStatefulResult{privateBooleanisSuccess;privateIdid;privateDatabase.Error[]errors;publicStatefulResult(Database.SaveResultresult){isSuccess=result.isSuccess();id=result.getId();errors=result.getErrors();}publicBooleanisSuccess(){returnisSuccess;}publicIdgetId(){returnid;}publicDatabase.Error[]getErrors(){returnerrors;}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/AvoidStatefulDatabaseResult"/>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 onor reported.
This rule is defined by the following XPath expression:
//CatchBlockStatement[./BlockStatement[count(*)=0]andnot(matches(@VariableName,$allowExceptionNameRegex))and($allowCommentedBlocks=false()or@ContainsComment=false())]Example(s):
publicvoiddoSomething(){...try{insertaccounts;}catch(DmlExceptiondmle){// 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:
<ruleref="category/apex/errorprone.xml/EmptyCatchBlock"/>Use this rule and customize it:
<ruleref="category/apex/errorprone.xml/EmptyCatchBlock"><properties><propertyname="allowCommentedBlocks"value="false"/><propertyname="allowExceptionNameRegex"value="^(ignored|expected)$"/></properties></rule>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):
publicclassFoo{publicvoidbar(Integerx){if(x==0){// empty!}}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/EmptyIfStmt"/>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):
publicclassFoo{privateInteger_bar;publicvoidsetBar(Integerbar){// 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:
<ruleref="category/apex/errorprone.xml/EmptyStatementBlock"/>Use this rule and customize it:
<ruleref="category/apex/errorprone.xml/EmptyStatementBlock"><properties><propertyname="reportEmptyPrivateNoArgConstructor"value="true"/><propertyname="reportEmptyVirtualMethod"value="true"/></properties></rule>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):
publicclassFoo{publicvoidbar(){try{// empty !}catch(Exceptione){e.printStackTrace();}}}publicclassFoo{publicvoidbar(){try{Integerx=2;}finally{// empty!}}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/EmptyTryOrFinallyBlock"/>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 isa 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):
publicvoidbar(Integera,Integerb){while(a==b){// empty!}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/EmptyWhileStmt"/>Since: PMD 6.36.0
Priority: Medium (3)
In the Summer ‘21 release, a mandatory security update enforces access modifiers on Apex properties inLightning component markup. The update prevents access to private or protected Apex getters from Auraand Lightning Web Components.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.apex.rule.errorprone.InaccessibleAuraEnabledGetterRule
Example(s):
publicclassFoo{@AuraEnabledpublicIntegercounter{privateget;set;}// Violating - Private getter is inaccessible to Lightning components@AuraEnabledpublicstaticFoobar(){Foofoo=newFoo();foo.counter=2;returnfoo;}}publicclassFoo{@AuraEnabledpublicIntegercounter{protectedget;set;}// Violating - Protected getter is inaccessible to Lightning components@AuraEnabledpublicstaticFoobar(){Foofoo=newFoo();foo.counter=2;returnfoo;}}publicclassFoo{@AuraEnabledpublicIntegercounter{get;set;}// Compliant - Public getter is accessible to Lightning components@AuraEnabledpublicstaticFoobar(){Foofoo=newFoo();foo.counter=2;returnfoo;}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/InaccessibleAuraEnabledGetter"/>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):
publicclassMyClass{// this is OK because it is a constructorpublicMyClass(){}// this is bad because it is a methodpublicvoidMyClass(){}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/MethodWithSameNameAsEnclosingClass"/>Since: PMD 6.31.0
Priority: Medium (3)
Override bothpublic Boolean equals(Object obj), andpublic Integer hashCode(), or override neither.Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitlydelegating to your superclass.
This is especially important whenUsing 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):
publicclassBar{// poor, missing a hashCode() methodpublicBooleanequals(Objecto){// do some comparison}}publicclassBaz{// poor, missing an equals() methodpublicIntegerhashCode(){// return some hash value}}publicclassFoo{// perfect, both methods providedpublicBooleanequals(Objectother){// do some comparison}publicIntegerhashCode(){// return some hash value}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/OverrideBothEqualsAndHashcode"/>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 properclass 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 whendealing 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):
// ViolatingprivateclassTestClass{@IsTeststaticvoidmyTest(){// Code here}}privateclassTestClass{statictestMethodvoidmyTest(){// Code here}}// Compliant@IsTestprivateclassTestClass{@IsTeststaticvoidmyTest(){// Code here}}@IsTestprivateclassTestClass{statictestMethodvoidmyTest(){// Code here}}Use this rule by referencing it:
<ruleref="category/apex/errorprone.xml/TestMethodsMustBeInTestClasses"/>