```apex Bad theme={null} if (((condition1 && condition2) || (condition3 && condition4)) && condition5) { ... } ``` ```apex Fix theme={null} if ((myFirstCondition || mySecondCondition) && myLastCondition) { ... } ```

An Apex trigger may be called with a batch of records. This for example happens when a bulk DML is executed. All records provided by the triggers should be processed.

This rule raises an issue when specific records from the \`Trigger are referenced, i.e. when it finds one of the following patterns:

Trigger.new\[x]
Trigger.old\[x]
Trigger.oldmap.get(x)
Trigger.newmap.get(x)

where x\` is a hardcoded number.

```apex Bad theme={null} trigger CaseTrigger on Case (before insert, before update) { //This only handles the first record in the Trigger.new collection //But if more than 1 Case initiated this trigger, those additional records //will not be processed Case c1 = Trigger.old[0]; // Noncompliant Case c2 = Trigger.new[0]; // Noncompliant Case c3 = Trigger.oldmap.get(1); // Noncompliant Case c4 = Trigger.newmap.get(1); // Noncompliant // ... } ``` ```apex Fix theme={null} trigger CaseTrigger on Case (before insert, before update) { List Names = new List{}; //Loop through all records in the Trigger.new collection for(Case c: Trigger.new){ // Good: Iterate through the trigger.new array instead c.Subject = c.Number + ':' + c.Status } } ```

In order to protect shared resources, Salesforce enforces a maximum number of SOQL queries which can be executed inside a single transaction. This is part of Governor limits. If an SOQL query is nested inside a loop’s body (For/While/Do-While) it might be executed more times than the Governor limit allows, making the code fail.

Thus it is a best practice to not have SOQL queries nested in the body of loops and instead retrieve all relevant data in a single query.

This rule raises an issue when it detects SOQL queries inside loops. This includes static SOQL queries (i.e. \[SELECT ...]) and calls to Database.countQuery, Database.getQueryLocator and Database.query methods.

```apex Bad theme={null} // Query in a for loop public class MyClass { public static void myFunction() { for (Integer i = 0; i < 1000; i++) { string firstName = 'Bob' + i; Contact[] contacts = [ Select LastName From Contact Where FirstName = :firstName]; // Noncompliant } } } // Query in a while loop public class MyClass { public static void myFunction() { Integer i = 0; while ( i < 1000) { string firstName = 'Bob' + i; Contact[] contacts = [ Select LastName From Contact Where FirstName = :firstName]; // Noncompliant i = i+1; } } } // Query in a do-while loop public class MyClass { public static void myFunction() { Integer i = 0; do { string firstName = 'Bob' + i; Contact[] contacts = [ Select LastName From Contact Where FirstName = :firstName]; // Noncompliant i++; } while (i < 1000); } } ``` ```apex Fix theme={null} public class MySOQLLoop { public static void myFunction() { List names = new List(); for (Integer i = 0; i < 10; i++) { names.add('Bob' + i); } for (Contact contact: [Select LastName From Contact Where FirstName in :names]) { // ... } } } ```

There are three limits related to methods annotated with the \`@future keyword:

There is a maximum number of @future calls allowed per transaction.
There is a maximum number of @future calls allowed per organization and per a 24-hour period.
Salesforce protects its infrastructure from organizations calling too many @future methods: if more than 2000 @future requests are queued, any additional request will be delayed.

Thus it is a best practice to avoid calling @future methods in loops and instead call it once with all the necessary data.

See Execution Governors and Limits for the exact number of @future calls allowed in each context.

This rule raises an issue when a method annotated with the @future\` keyword is called in a loop.

```apex Bad theme={null} public class FutureClass { @future public static void processTaskFuture(ID taskId) { Task task = [SELECT Id, Subject FROM Task WHERE Task.Id = :taskId]; task.Subject = 'new task subject'; update task; } public static void updateUserTasks(ID ownerID) { List tasks = [SELECT Id, Subject FROM Task WHERE Task.ownerid = :ownerID]; for (Task task: tasks) { processTaskFuture(task.Id); // Noncompliant } } ``` ```apex Fix theme={null} public class FutureClass { @future public static void processTasksFuture(List taskIds) { List tasks = [SELECT Id, Subject FROM Task WHERE Task.Id IN :taskIds]; for (Task task : tasks) { task.Subject = 'new task subject'; } update tasks; } public static void updateUserTasks(ID ownerID) { List tasks = [SELECT Id, Subject FROM Task WHERE Task.ownerid = :ownerID]; List ids = new List(); for (Task task: tasks) { ids.add(task.Id); } processTasksFuture(ids); } ```

By default tests will run in system mode, i.e. without taking into account users permissions. In order to be realistic, a test needs to run Business logic code in User context. This is done by encapsulating the tested code in \`System.runAs().

This rule raises an issue when a test function, i.e. a function annotated with @isTest or testMethod, does not contain a System.runAs()\` call.

```apex Bad theme={null} @isTest private class TestClass { @isTest public static void testMethod() { // NonCompliant // Setup test data User u = new User(...); Case c = new Case (Name = 'Test'); Test.startTest(); Insert c; Test.stopTest(); } } ``` ```apex Fix theme={null} @isTest private class TestClass { @isTest public static void testMethod() { // Setup test data User u = new User(...); Case c = new Case (Name = 'Test'); System.runAs(u) { Test.startTest(); Insert c; Test.stopTest(); } } } ```

In order to protect shared resources, Salesforce enforces a maximum number of SOQL queries, DML queries, \`@future method, etc… execution inside a single transaction. This is part of Governor limits.

Every test method should check how close the tested code is to reach the governor limits. This is done by calling Test.StartTest() before the tested code and Test.StopTest() after it.

This rule raises an issue when a test method, i.e. a method annotated with @isTest or testmethod, does not contain a call to Test.StartTest() and Test.StopTest()\`.

```apex Bad theme={null} @isTest() public class MyTestClass { @isTest() static void myTestMethod() { // Noncompliant // Preparing test (creating records, etc...) // test code with no reference to Test.StartTest() or Test.StopTest() } static testmethod void myTestMethod2() { // Noncompliant // Preparing test (creating records, etc...) // test code with no reference to Test.StartTest() or Test.StopTest() } static void myHelper() { // ... } } ``` ```apex Fix theme={null} @isTest() public class MyTestClass { @isTest() static void myTestMethod() { // Preparing test (creating records, etc...) Test.StartTest(); // test code with no reference to Test.StartTest() or Test.StopTest() Test.StopTest(); } static testmethod void myTestMethod2() { // Preparing test (creating records, etc...) Test.StartTest(); // test code with no reference to Test.StartTest() or Test.StopTest() Test.StopTest(); } static void myHelper() { // ... } } ```

Its a best practice to avoid hardcoding of text strings in Apex. Ideally those text should be stored in a Custom Label and referenced via those in the code. This enables administrators and translators to update the messages without modifying the code. It also enables the translation of those text in international organizations.

This rule raises an issue when a call to the sObject.addError method is given a hardcoded string.

```apex Bad theme={null} public class MyClass{ public static myMethod(Case mycase) { mycase.addError('This is a hardcoded error'); } } ``` ```apex Fix theme={null} public class MyClass{ public static myMethod(Case case) { mycase.addError(System.Label.caseErrorMessage); } } ```

There is a limit to the number of batch jobs which can be executed at the same time. Executing batch jobs in triggers or in loops is complex as it can easily reach this limit (example: after a bulk update), thus it is recommended to avoid it. Note that checking if the limit is already reached is not enough as nothing prevents the addition of a job between the check and the subsequent call to \`executeBatch.

This rule raises an issue when Databasee.executeBatch()\` is called in a Trigger or in a loop.

```apex Bad theme={null} trigger MyBatchTrigger on Account (after insert) { MyBatch batch = new MyBatch(); Database.executeBatch(batch); // Noncompliant } public class MyBatchLoop { static void createManyBatch() { for (integer i = 0; i < 1000; ++i) { MyBatch batch = new MyBatch(); Database.executeBatch(batch); // Noncompliant } } } ``` ```apex Fix theme={null} ```

\`SeeAllData=true should not be used because it gives your tests access to all data in your organization. Activating this option makes your test dependent on existing data and more difficult to maintain. Tests should create their own data.

This rule raises an issue when it sees @isTest(SeeAllData=true)\`.

```apex Bad theme={null} @isTest(SeeAllData=true) // Noncompliant public class MyTestClass { @isTest(SeeAllData=true) // Noncompliant static void myTestMethod() { // Can access all data in the organization. } } ``` ```apex Fix theme={null} ```

A \`switch and a chain of if/else if statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.

Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.

For a switch, the second when\` will never be executed, rendering it dead code. Worse there is the risk in this situation that future maintenance will be done on the dead case, rather than on the one that’s actually used.

```apex Bad theme={null} if (param == 1) { openWindow(); } else if (param == 2) { closeWindow(); } else if (param == 1) { // Noncompliant moveWindowToTheBackground(); } switch on i { when 1 { // ... } when 2 { // ... } when 1 { // Noncompliant // ... } when else { // ... } } ``` ```apex Fix theme={null} if (param == 1) { openWindow(); } else if (param == 2) { closeWindow(); } else if (param == 3) { moveWindowToTheBackground(); } switch on i { when 1 { // ... } when 2 { // ... } when else { // ... } } ```

Batch Apex jobs should execute as fast as possible because other jobs wait in the queue. They will execute faster when their \`start method returns a QueryLocator object or an iterable which does not use subqueries to include related records. It is more efficient to execute these subqueries separately inside the execute method of the Batch class.

This rule raises an issue when, in the context of a Batch class' start method, the Database.getQueryLocator(query)\` method is called with a query containing one or more subqueries

```apex Bad theme={null} global class MyBatch implements Database.Batchable { global MyBatch(){ } global Database.QueryLocator start(Database.BatchableContext BC){ return Database.getQueryLocator([SELECT Id, (SELECT id FROM Contacts) FROM Account]); // Noncompliant } global void execute(Database.BatchableContext BC, List scope){ // ... } global void finish(Database.BatchableContext BC){ // ... } } ``` ```apex Fix theme={null} ```

In order to protect shared resources, Salesforce enforces a maximum number of SOQL queries which can be executed inside a single transaction. This is part of Governor limits.

Limiting the number of queries executed is necessary, and SOQL queries which have no chance of returning a result should be avoided. This is why SOQL Query of the form \`SELECT ... FROM ... WHERE ... in :filterValues should first check that the Set/Map (in this example named "filterValues") is not empty.

This rule raises an issue when such an SOQL query is not enclosed in one of

if (!filtervalues.isEmpty())
if (!filtervalues.size() != 0)
if (!filtervalues.size() > 0)\`

```apex Bad theme={null} public class fooClass{ Set setCasesId = new Set(); //Noncompliant - SOQL Query executed without checking whether the Set is Empty for(Case c :[SELECT Id FROM Case WHERE Id IN :setCasesId]){ //something } } public class fooClass{ Map mapCasesId = new Map(); //Noncompliant - SOQL Query executed without checking whether the Map is Empty for(Case c :[SELECT Id FROM Case WHERE Id IN :mapCasesId.keyset()]){ // ... } } public class fooClass{ Map mapCasesId = new Map(); //Noncompliant - SOQL Query executed without checking whether the Map is Empty or Set is Empty for(Case c :[SELECT Id, Subject FROM Case WHERE Id IN :mapCasesId.keyset() OR Id IN :setCasesId]){ // ... } } public class fooClass{ Map mapCasesId = new Map(); //Noncompliant - SOQL Query executed without checking whether the Map is Empty and Set is Empty for(Case c :[SELECT Id, Subject FROM Case WHERE Id IN :mapCasesId.keyset() AND Id IN :setCasesId]){ // ... } } ``` ```apex Fix theme={null} public class fooClass{ Set setCasesId = new Set(); if(!setCasesId.isEmpty()){ for(Case c :[SELECT Id FROM Case WHERE Id IN :setCasesId]){ //something } } // OR if(!setCasesId.size() > 0){ for(Case c :[SELECT Id FROM Case WHERE Id IN :setCasesId]){ //something } } // OR if(!setCasesId.size() != 0){ for(Case c :[SELECT Id FROM Case WHERE Id IN :setCasesId]){ //something } } } public class fooClass{ Set setCasesId = new Set(); if(!mapCasesId.isEmpty()){ for(Case c :[SELECT Id FROM Case WHERE Id IN :mapCasesId.keyset()]){ //something } } } public class fooClass{ Map mapCasesId = new Map(); if((!setCasesId.isEmpty()) || (!mapCasesId.isEmpty())){ for(Case c :[SELECT Id, Subject FROM Case WHERE Id IN :mapCasesId.keyset() OR Id IN :setCasesId]){ //something } } } public class fooClass{ Map mapCasesId = new Map(); //Recommended - SOQL Query executed after checking that the Map is Not Empty or Set is Not Empty if((!setCasesId.isEmpty()) && (!mapCasesId.isEmpty())){ for(Case c :[SELECT Id, Subject FROM Case WHERE Id IN :mapCasesId.keyset() AND Id IN :setCasesId]){ //something } } } ```

Moving the business logic out of Trigger functions and into dedicated "Trigger Handler" classes improves the application’s design:

the code is easier to test and maintain.
it helps avoiding some bugs such as trigger recursion.

The Trigger functions should only dispatch calls to the corresponding Trigger Handler classes. See the links below for examples of Trigger Handler designs.

This rule raises an issue when a Trigger function contains one of the following syntax elements: loops, switch-case, try blocks, SOQL queries, DML queries, SOSL queries. The goal is to detect Trigger functions which have a complex logic. In practice method calls and if statements are enough to dispatch records for processing.

```apex Bad theme={null} trigger MyTrigger on Account(after insert, after update) { // Noncompliant. The trigger is processing records itself instead of using a Trigger Handler. for(Account a : Trigger.New) { // ... } } ``` ```apex Fix theme={null} ```

Jump statements (return, break, continue) and throw expressions move control flow out of the current code block. So any statements that come after a jump are dead code.

```apex Bad theme={null} Integer foo(Integer a) { Integer i = 10; return i + a; // Noncompliant i++; // dead code } ``` ```apex Fix theme={null} Integer foo(Integer a) { Integer i = 10; return i + a; // Noncompliant } ```

Asserts should only be used in test code. Salesforce provide better ways to handle errors:

Classes containing test code, including test helper methods, should always have the \`@isTest annotation. There is a limit to the amount of code an organization can upload into Salesforce. Test code does not count towards this limit, and test code is recognized via the @isTest annotation.

This rule raises an issue when methods System.assert, System.assertEquals or System.assertNotEquals are called in a class which is not annotated with @isTest\`.

```apex Bad theme={null} class ApexTests { private static void testHelper(Task task) { System.assert("expected subject", task.subject, "task has not the expected subject"); // Noncompliant } } ``` ```apex Fix theme={null} @isTest class ApexTests { private static void testHelper(Task task) { System.assert("expected subject", task.subject, "task has not the expected subject"); } } ```

For example, with the default provided regular expression ^\[a-z]\[a-zA-Z0-9\_]\*\$, the function:

```apex Bad theme={null} public void DoSomething(){...} // Noncompliant ``` ```apex Fix theme={null} public void doSomething(){...} ```

Salesforce provides a library of Aggregate functions like COUNT() which efficiently process the data records in the database and return the count of records which meet the SOQL Query criteria. Using size() function instead of COUNT() involves more processing and is less efficient.

This rule raises an issue when the result of an SOQL query is only used to call size() on it.

```apex Bad theme={null} public class fooClass{ public static fooMethod { integer i = [SELECT Id FROM Case].size(); // Noncompliant } } ``` ```apex Fix theme={null} public class fooClass{ public static fooMethod { integer i = [SELECT COUNT() FROM Case]; } } ```

QL queries, are sensitive to injection attacks. An injection attack happens when a user-controlled value is inserted in a query without proper sanitization. This enables attackers to access sensitive information or even perform unauthorized data modification.

This rule raises an issue when one of the methods \`Database.query, Database.countQuery is called with a string which was build by:

calling String.format
concatenation (string + string)
calling String.replace, String.replaceAll, String.replaceFirst

AND the strings provided were not hardcoded nor sanitized by string.escapeSingleQuotes\`

```apex Bad theme={null} public class My { public getContactSafe(String firstname) { String query = 'SELECT id FROM Contact WHERE firstname =\''+String.escapeSingleQuotes(firstname)+'\''; return Database.execute(query); // Compliant } } ``` ```apex Fix theme={null} ```

In order to protect shared resources, Salesforce enforces a maximum number of DML statements which can be executed inside a single transaction. This is part of Governor limits. If a DML statement is nested inside a loop’s body (For/While/Do-While) it might be executed more times than the Governor limit allows, making the code fail.

Thus it is a best practice to not have DML statements nested in the body of loops and instead perform the DML operation on a list of sObjects.

This rule raises an issue when it detects DML statements inside a loop.

```apex Bad theme={null} public class myDMLLoop { public static void myFunction() { for (Task task: [Select Id, subject from Task]) { if (task.subject == 'foo') { task.subject = 'bar'; update task; // Noncompliant } } } } // Query in a while loop public class myDMLLoop { public static void myFunction(Task[] tasks) { Integer i = 0; while ( i < 1000) { Task task = tasks[i]; if (task.subject == 'foo') { task.subject = 'bar'; update task; // Noncompliant } } } } // Query in a do-while loop public class myDMLLoop { public static void myFunction(Task[] tasks) { Integer i = 0; do { Task task = tasks[i]; if (task.subject == 'foo') { task.subject = 'bar'; update task; // Noncompliant } i++; } while (i < 1000); } } ``` ```apex Fix theme={null} public class myDMLLoop { public static void myFunction() { List updatedTasks = new List(); for (Task task: [Select Id, subject from Task]) { if (task.subject == 'foo') { task.subject = 'bar'; updatedTasks.add(task); } } update updatedTasks; } } ```

Salesforce Governor Limits do not allow more than 10 calls to \`Messaging.sendEmail in a single transaction. There is a good chance that calling this method in a loop will reach that limit and fail. You can instead send a batch of emails with a single call to Messaging.sendEmail.

This rule raises an issue when a call to Messaging.sendEmail\` is found in a loop.

```apex Bad theme={null} trigger MyWelcomeTrigger on Contact (after insert) { List toIds = new List(); for (Contact contact : trigger.new) { if(contact.Email != null) { Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage(); String[] toAddresses = new String[] { contact.email }; mail.setToAddresses(toAddresses); mail.setSubject('Welcome'); mail.setPlainTextBody('Welcome'); Messaging.sendEmail(new Messaging.SingleEmailMessage[] { mail }); // Noncompliant } } ``` ```apex Fix theme={null} trigger MyWelcomeTrigger on Contact (after insert) { List toIds = new List(); for (Contact contact : trigger.new) { if(contact.Email != null) { toIds.add(contact.Id); } } string templateName = 'Welcome Email Template'; EmailTemplate template = [select Id, Name from EmailTemplate where name = :templateName]; Messaging.MassEmailMessage mail = new Messaging.MassEmailMessage(); mail.setTargetObjectIds(toIds); mail.setTemplateId(template.Id); Messaging.sendEmail(new Messaging.MassEmailMessage[] { mail }); } ```

SOQL queries might return too many results and exceed the maximum heap size. To avoid this issue the best solution is to use SOQL for loops, i.e. for loops with inline an SOQL query. The loop will then chunk efficiently the query’s results by calling "query" and "queryMore" methods.

This rule raises an issue when an SOQL query is executed and later a for loop iterates over its result.

```apex Bad theme={null} public class mySOQLLoop { public static void myFunction() { Task[] tasks = [Select Id, subject from Task]; for (Task task: tasks) { // ... } } ``` ```apex Fix theme={null} public class mySOQLLoop { public static void myFunction() { for (Task task: [Select Id, subject from Task]) { // ... } } ```

Using absolute URLs to Salesforce Pages is bug prone. Different sandboxes and production environments will have different instance names (like "na10", "na15" etc.). Code using absolute URLs will only work when it runs in the corresponding salesforce instances. It will break as soon as it is deployed in another one. Thus only relative URLs, i.e. without the domain and subdomain names, should be used when pointing to a salesforce page.

This rule raises an issue when a string matches the regular expression:

\{noformat}

(?\

\{noformat}

```apex Bad theme={null} public PageReference testRedirect() { String strUrl ='https://na8.salesforce.com/TestVFPage?AcoountId=999'; // Noncompliant PageReference newUrl = new PageReference(strUrl); newURL.setRedirect(true); return newURL; } ``` ```apex Fix theme={null} public PageReference testRedirect() { String strUrl = URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AcoountId=999'; PageReference newUrl = new PageReference(strUrl); newURL.setRedirect(true); return newURL; } OR public PageReference testRedirect() { String strUrl = URL.getOrgDomainUrl().toExternalForm() + '/TestVFPage?AcoountId=999'; PageReference newUrl = new PageReference(strUrl); newURL.setRedirect(true); return newURL; } ```

Using \`getRecordTypeInfosByName() requires you to pass the Record Type Name. A Record Type Name is a label, which means that it can be easily changed by an administrator. When this happens, the code will simply stop working until a developer updates the Record Type Name. Even worse, Labels are usually translated in multi-language environments, which will again make the code fail.

The method getRecordTypeInfosByDeveloperName() also enables you to get the Record Type Id, but it requires instead a Record Type API Name, which is rarely changed.

Thus it is safer to use getRecordTypeInfosByDeveloperName() than getRecordTypeInfosByName()\`.

```apex Bad theme={null} Id RTId = Schema.SObjectType.Case.getRecordTypeInfosByName().get('Asia Service').getRecordTypeId(); ``` ```apex Fix theme={null} Id RTId = Schema.SObjectType.Case.getRecordTypeInfosByDeveloperName().get('Asia_Service').getRecordTypeId(); ```

DML statements should not be executed in class constructors, instance initialization code blocks or static initialization code blocks for two reasons:

It is counter intuitive. Developers will not expect the instantiation of an object to modify records.
Constructor and/or initialization code blocks of VisualForce controllers will fail if they try to execute DML statements. This is also true for indirect calls to DML statements: they can’t instantiate objects which execute DML statements in their own constructors/initialization code. Having to check if a constructor can or cannot be called makes development more complex.

This rule raises an issue when a class constructor, an instance initialization block or a static initialization block executes a DML statement.

```apex Bad theme={null} public class MyController { { List tasks = [Select Id, Subject FROM Task]; for (Task t : tasks) { t.subject = 'instance initialization code block'; } update tasks; // Noncompliant } static { List tasks = [Select Id, Subject FROM Task]; for (Task t : tasks) { t.subject = 'static initialization code block'; } update tasks; // Noncompliant } public MyController() { List tasks = [Select Id, Subject FROM Task]; for (Task t : tasks) { t.subject = 'constructor'; } update tasks; // Noncompliant } } ``` ```apex Fix theme={null} public class MyController { public MyController() { } // For example, move the DML statement to a method called when a user clicks a button public updateTasks() { List tasks = [Select Id, Subject FROM Task]; for (Task t : tasks) { t.subject = 'test'; } update tasks; } } ```

The switch statement should be used only to clearly define some new branches in the control flow. As soon as a when clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of the when clause should be extracted into a dedicated function.

```apex Bad theme={null} public void foo(Integer value) { switch on value { when 1 { methodCall1(''); methodCall2(''); methodCall3(''); methodCall4(''); methodCall5(''); } when 2 { /* ... */ } } } ``` ```apex Fix theme={null} public void foo(Integer value) { switch on value { when 1 { doSomething(); } when 2 { /* ... */ } } } } private void doSomething() { methodCall1(''); methodCall2(''); methodCall3(''); methodCall4(''); methodCall5(''); } ```

The \`SystemModStamp and LastModifiedDate are similar in that they store last modification dates of a record. The main difference is that LastModifiedDate is only updated when users update a record, not when automated system do it.

This means that 'LastModifiedDate \<= SystemModStamp' but it is not possible to have 'LastModifiedDate > SystemModStamp'

The SystemModStamp field is indexed, whereas LastModifiedDate is not. In most case when an SOQL query filters on LastModifiedDate, the index of SystemModStamp will be used instead. This however does not apply when the filter is of the form where LastModifiedDate \< :mydate because SystemModStamp can be greater than LastModifiedDate.

This rule raises an issue when an SOQL query has a where filter with a condition of the form LastModifiedDate \< :mydate or LastModifiedDate \<= :mydate\`.

```apex Bad theme={null} [Select Id from Case where LastModifiedDate < :mydate]; // Noncompliant [Select Id from Case where LastModifiedDate <= :mydate]; // Noncompliant ``` ```apex Fix theme={null} ```

ecutes without checking permissions. Hence the code will not enforce field level security, sharing rules and user permissions during execution of Apex code in Triggers, Classes and Controllers. This creates the risk that unauthorized users may get access to sensitive data records or fields.

It is possible to specify different level of sharing via the keywords "with sharing", "without sharing" or "inherited sharing". The last two should be used very carefully as they can create security risks.

This rule raises an issue whenever a DML, SOSL or SOQL query is executed in a class marked as without sharing or inherited sharing.

```apex Bad theme={null} public with sharing class MyClass { // Compliant List lstCases = new List(); for(Case c:[SELECT Id FROM Case WHERE Status = 'In Progress']){ // ... } } ``` ```apex Fix theme={null} ```

Salesforce Record Ids vary across environments. Thus if a Record Id is hardcoded in Apex code, the code may not work as expected when it is deployed in another environment. This happens because the environment in which the code is deployed may not have the same hardcoded record Id. Hence it is a best practice to avoid hard coding record Ids.

This rule raises an issue when it detects hardcoded record Ids, i.e. strings which contain 15 or 18 characters and which start with any of the prefixes listed by Salesforce

```apex Bad theme={null} for(Case c: Trigger.new){ if(c.RecordTypeId=='012500000009tuy'){ // Noncompliant // ... } else if(c.RecordTypeId=='0123000000095Kmwer'){ // Noncompliant // ... } } ``` ```apex Fix theme={null} ```

By default Apex code executes without checking permissions. Hence the code will not enforce field level security, sharing rules and user permissions during execution of Apex code in Triggers, Classes and Controllers. This creates the risk that unauthorized users may get access to sensitive data records or fields.

To prevent this, developers should use \`with sharing keyword when declaring their classes if the class has SOQL or SOSL queries or DML Statements. This will ensure that current user’s permissions, field level security and sharing rules are enforced during code execution. Thus users will only see or modify records and fields which they have access to.

Use without sharing when a specific class should have full access to records without taking into account current user’s permissions. This should be used very carefully.

Use inherited sharing when the code should inherit the level of access from the calling class. This is more secure than not specifying a sharing level as the default will be equivalent to "with sharing".

This rule raises an issue when a class containing DML, SOSL or SOQL queries has no sharing level specified (with sharing, without sharing, inherited sharing\`).

```apex Bad theme={null} public class MyClass { // Noncompliant, no sharing specified List lstCases = new List(); for(Case c:[SELECT Id FROM Case WHERE Status = 'In Progress']){ // SOQL query // ... } } public class MyClass { // Noncompliant List> sList = [FIND 'TEST' IN ALL FIELDS RETURNING Case(Name), Contact(FirstName,LastName)]; // SOSL query } public class MyClass { // Noncompliant List lstCases = new List(); for(Case c:[SELECT Id, Status FROM Case WHERE Status = 'In Progress']){ c.Status = 'Closed'; lstCasesToBeUpdated.add(c); } Update lstCasesToBeUpdated; // DML query } ``` ```apex Fix theme={null} public with sharing class MyClass { List lstCases = new List(); for(Case c:[SELECT Id FROM Case WHERE Status = 'In Progress']){ // ... } } public without sharing class MyClass { List> sList = [FIND 'TEST' IN ALL FIELDS RETURNING Case(Name), Contact(FirstName,LastName)]; } public inherited sharing class MyClass { List lstCases = new List(); for(Case c:[SELECT Id, Status FROM Case WHERE Status = 'In Progress']){ c.Status = 'Closed'; lstCasesToBeUpdated.add(c); } Update lstCasesToBeUpdated; } ```

The requirement for a final when else clause is defensive programming. The clause should either take appropriate action, or contain a suitable comment as to why no action is taken.

```apex Bad theme={null} switch on i { // missing 'when else' when 2 { System.debug('when block 2'); } when -3 { System.debug('when block -3'); } } ``` ```apex Fix theme={null} switch on i { when 2 { System.debug('when block 2'); } when -3 { System.debug('when block -3'); } when else { System.debug('default'); } } ```

Nested \`switch structures are difficult to understand because you can easily confuse the when blocks of an inner switch as belonging to an outer statement. Therefore nested switch statements should be avoided.

Specifically, you should structure your code to avoid the need for nested switch statements, but if you cannot, then consider moving the inner switch\` to another function.

```apex Bad theme={null} public void foo(Integer i, Integer j) { switch on i { when 1 {System.debug(' 1'); } when 2 { System.debug(' 2'); } when -3 { switch on j { // Noncompliant when 1 {System.debug(' 3');} when else {System.debug(' 4');} } } } } ``` ```apex Fix theme={null} public void foo(Integer i, Integer j) { switch on i { when 1 {System.debug(' 1'); } when 2 { System.debug(' 2'); } when -3 {bar(j);} } } public void bar(Integer j) { switch on j { when 1 {System.debug(' 3');} when else {System.debug(' 4');} } } ```

A test case without assertions ensures only that no exceptions are thrown. Beyond basic runnability, it ensures nothing about the behavior of the code under test.

This rule raises an issue when a function with an \`@isTest annotation does not call at least one of the following functions once:

System.assert
System.assertEquals
System.assertNotEquals\`

```apex Bad theme={null} @isTest() class ApexTests { @isTest private static void mytest() { // Noncompliant MyClass.createTask(); } } ``` ```apex Fix theme={null} @isTest() class ApexTests { @isTest private static void mytest() { Task task = MyClass.createTask(); System.assert("expected subject", task.subject, "task has not the expected subject"); } } ```

It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.

```apex Bad theme={null} if ( !(a == 2)) { ...} // Noncompliant Boolean b = !(i < 10); // Noncompliant ``` ```apex Fix theme={null} if (a != 2) { ...} Boolean b = (i >= 10); ```

Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.

Unresolved directive in \ - include::\{noncompliant}\[]

Write one statement per line to improve readability.

Unresolved directive in \ - include::\{compliant}\[]

```apex Bad theme={null} a = true; return 0; // Noncompliant ``` ```apex Fix theme={null} a = true; return 0; ```

A boolean literal can be represented in two different ways: \{true} or \{false}. They can be combined with logical operators (\{ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.

```apex Bad theme={null} if (booleanMethod() || false) { /* ... */ } doSomething(!false); doSomething(booleanMethod() && true); ``` ```apex Fix theme={null} if (booleanMethod()) { /* ... */ } doSomething(true); doSomething(booleanMethod()); ```

Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.

Merging if statements when possible will decrease the nesting of the code and improve its readability.

```apex Bad theme={null} if (condition1) { if (condition2) // Noncompliant { ... } } ``` ```apex Fix theme={null} if (condition1 && condition2) // Compliant { ... } ```

An empty \{operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty \{operationName}s bring no functionality and are misleading to others as they might think the \{operationName} implementation fulfills a specific and identified requirement.

There are several reasons for a \{operationName} not to have a body:

It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

```apex Bad theme={null} public void shouldNotBeEmpty() { // Noncompliant - method is empty } public void notImplemented() { // Noncompliant - method is empty } public void emptyOnPurpose() { // Noncompliant - method is empty } ``` ```apex Fix theme={null} public void shouldNotBeEmpty() { doSomething(); } public void notImplemented() { throw new Exception('notImplemented() cannot be performed because...'); } public void emptyOnPurpose() { // comment explaining why the method is empty } ```

Clear-text protocols such as \`ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:

sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information

Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.

For example, attackers could successfully compromise prior security layers by:

bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)

In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.

Note that using the http\` protocol is being deprecated by major web browsers.

In the past, it has led to the following vulnerabilities:

```apex Bad theme={null} public class MyClient { public void SendRequest(){ HttpRequest req = new HttpRequest(); req.setEndpoint('http://example.com'); // Sensitive // ... } } ``` ```apex Fix theme={null} public class MyClient { public void SendRequest(){ HttpRequest req = new HttpRequest(); req.setEndpoint('https://example.com'); // ... } } ```

The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.

```apex Bad theme={null} Integer x = ((y / 2 + 1)); // Noncompliant if (a && ((x + y > 0))) { // Noncompliant return ((x + 1)); // Noncompliant. } ``` ```apex Fix theme={null} Integer x = (y / 2 + 1); if (a && (x + y > 0)) { return (x + 1); } ```

Using the same value on both sides of a binary operator is a code defect. In the case of logical operators, it is either a copy/paste error and, therefore, a bug, or it is simply duplicated code and should be simplified. In the case of bitwise operators and most binary mathematical operators, having the same value on both sides of an operator yields predictable results and should be simplified as well.

```apex Bad theme={null} if ( a == a ) { // always true foo(); } if ( a != a ) { // always false foo(); } if ( a == b && a == b ) { // if the first one is true, the second one is too foo(); } if ( a != b || a <> b ) { // if the first one is true, the second one is too foo(); } Integer i = 5 / 5; // always 1 Integer j = 5 - 5; // always 0 ``` ```apex Fix theme={null} ```

A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.

```apex Bad theme={null} void doSomething(Integer a, Integer b) { // Noncompliant, "b" is unused compute(a); } ``` ```apex Fix theme={null} void doSomething(Integer a) { compute(a); } ```

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a class name does not match a provided regular expression.

```apex Bad theme={null} class myClass {} // Noncompliant ``` ```apex Fix theme={null} class MyClass {} ```

This rule applies whenever an \`if statement is followed by one or more else if statements; the final else if should be followed by an else statement.

The requirement for a final else statement is defensive programming.

The else statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final default clause in a switch\` statement.

```apex Bad theme={null} if (x == 0) { doSomething(); } else if (x == 1) { doSomethingElse(); } ``` ```apex Fix theme={null} if (x == 0) { doSomething(); } else if (x == 1) { doSomethingElse(); } else { throw new MyException('Illegal state'); } ```

Nested control flow statements such as if, for, while, switch, and try are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to understand and maintain.

When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.

```apex Bad theme={null} switch on i { // Compliant - depth = 1 when 1 { /* ... */ for (Integer i = 0, j = 0; i < 10; i++) { // Compliant - depth = 2 /* ... */ Integer j = i + 1; if (j == 0){// Compliant - depth = 3, not exceeding the limit /* ... */ while (j < 10){ // Noncompliant - depth = 4 /* ... */ if (j == 1){ // Depth = 5, exceeding the limit, but issues are only reported on depth = 4 /* ... */ } } } } } } ``` ```apex Fix theme={null} ```

Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.

```apex Bad theme={null} class A { void doSomething(String msg) { System.debug('string literal'); // Noncompliant - 'string literal' is duplicated 3 times System.assertEquals(msg, 'string literal'); msg = 'string literal'; } void doSomethingElse(String msg) { System.debug('a'); // Compliant - literal 'a' has less than 5 characters and is excluded System.assertEquals(msg, 'a'); msg = 'a'; } } ``` ```apex Fix theme={null} class A { static final String STRING_CONST = 'string literal'; void doSomething(String msg) { System.debug(STRING_CONST); // Compliant System.assertEquals(msg, STRING_CONST); msg = STRING_CONST; } } ```

Two \{func\_name}s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.

```apex Bad theme={null} private String code = 'secret'; public String calculateCode() { doTheThing(); return code; } public String getName() { // Noncompliant: duplicates calculateCode doTheThing(); return code; } ``` ```apex Fix theme={null} private String code = 'secret'; public String getCode() { doTheThing(); return code; } public String getName() { // Intent is clear return getCode(); } ```

Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:

Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:

The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).

Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.

```apex Bad theme={null} StaticResource sr= [SELECT ip_address FROM StaticResource WHERE Name = 'configuration' LIMIT 1]; // Compliant String ip_address = sr.body.toString(); String clientIp = ApexPages.currentPage().getHeaders().get(‘True-Client-IP’); Boolean isKnown = ip_address.equals(clientIp); ``` ```apex Fix theme={null} ```

This rule raises an issue when a \{visibility} \{operationName} is never referenced in the code.

```apex Bad theme={null} public class Server { public void start() { // Compliant, publicly available log('start'); } private void clear() { // Noncompliant, not used anywhere } private void log(String msg) { // Compliant, called from 'start()' System.debug(msg); } } ``` ```apex Fix theme={null} ```

\`if statements with conditions that are always false have the effect of making blocks of code non-functional. if statements with conditions that are always true are completely redundant, and make the code less readable.

There are three possible causes for the presence of such code:

An if statement was changed during debugging and that debug code has been committed.
Some value was left unset.
Some logic is not doing what the programmer thought it did.

In any of these cases, unconditional if\` statements should be removed.

```apex Bad theme={null} if (true) { doSomething(); } // ... if (false) { doSomethingElse(); } ``` ```apex Fix theme={null} doSomething(); // ... ```

Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.

In the past, it has led to the following vulnerabilities:

Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.

This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.

It’s recommended to customize the configuration of this rule with additional credential words such as "oauthToken", "secret", …

```apex Bad theme={null} String password = retrievePassword(); ``` ```apex Fix theme={null} ```

An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.

```apex Bad theme={null} public Integer numberOfMinutes(Integer hours) { Integer seconds = 0; // Noncompliant - seconds is unused return hours * 60; } ``` ```apex Fix theme={null} public Integer numberOfMinutes(Integer hours) { return hours * 60; } ```

A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. \{identifier\_capital\_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.

This rule checks that \{identifier} names match a provided regular expression.

```apex Bad theme={null} public void doSomething(Integer PARAM) { // Noncompliant Integer LOCAL; // Noncompliant ... } ``` ```apex Fix theme={null} public void doSomething(Integer param) { Integer local; ... } ```

Having two whens in a switch statement or two branches in an if chain with the same implementation is at best duplicate code, and at worst a coding error.

```apex Bad theme={null} if (a >= 0 && a < 10) { doFirstThing(); doTheThing(); } else if (a >= 10 && a < 20) { doTheOtherThing(); } else if (a >= 20 && a < 50) { doFirstThing(); doTheThing(); // Noncompliant; duplicates first condition } else { doTheRest(); } ``` ```apex Fix theme={null} if ((a >= 0 && a < 10) || (a >= 20 && a < 50)) { doFirstThing(); doTheThing(); } else if (a >= 10 && a < 20) { doTheOtherThing(); } else { doTheRest(); } ```