Apex

The complexity of an expression is defined by the number of && and || operators it contains.

A single expression’s complexity should not become too high to keep the code readable.

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:

where x` is a hardcoded number.

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.

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

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.

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.

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()`.

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.

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.

`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)`.

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.

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

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

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

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.

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.

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

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

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.

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:

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

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.

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.

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.

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}

(?<!\w)(login|test|(dns|test|[a-z]{1,2})\d++).(salesforce|force|visual.force|content.force).com(?!\w)

{noformat}

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()`.

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

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

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.

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

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.

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

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`).

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.

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.

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:

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

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

Unresolved directive in <stdin> - include::{noncompliant}[]

Write one statement per line to improve readability.

Unresolved directive in <stdin> - include::{compliant}[]

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.

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.

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:

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:

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:

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:

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.

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.

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.

Shared naming conventions allow teams to collaborate efficiently.

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

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.

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.

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

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.

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:

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.

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

`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:

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

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”, …​

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.

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.

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.