Prerequisites:
VSCode with Salesforce Extensions installed, configured and connected to both Azure DevOps and your ScratchOrg.
A SonarCloud account (Free for public projects / ~$10mo for most Salesforce projects) - with a project set up which will be used for the SFDX code analysis, note the projectKey as it will be used in step 14.
Configuration Process:
Install the SonarLint Extension into VSCode, and restart VSCode
Open your VSCode project related to your ScratchOrg
Visit SonarCloud and log in using the Azure DevOps button.
Visit your security page
Under the section titled Generate Tokens, enter vscode
and click the Generate button.
You will get a new token, copy the token value secure location we will use it later and you will not be able to retrieve it once it is generated.
Open Visual Studio Code, and navigate to the setting section
On Windows/Linux - File > Preferences > Settings
On macOS - Code > Preferences > Settings
Select User Settings and Search for SonarLint
Ensure that you select User directly under the search box so that you are configuring user settings.
Pick any of the settings and click **Edit in settings.json
Paste the following into the settings.json file
Replace the <<YOUR VSCODE TOKEN GOES HERE>> with the token you created earlier.
Replace the <<YOUR COMPANY NAME>> with your organization key
You can look up the organization key on the organization key page.
Replace the <<YOUR PROJECT KEY>> with your project key.
You can look up the project key under Administration > Projects Management on your organization's page.
You will also likely want to set the Sonarlint > LS: Java Home to the follow the path to your JDK in the same manner you had do to when installing AdoptOpenJavaSDK
Restart VSCode and reopen your VSCode Project related to your ScratchOrg
The following is a sample settings.json file for reference:
This intent of this document is not to dictate or restrain how you write code, rather it’s intended to be a guide to help you write better code. The document starts with a short list of items to look for when conducting code reviews. The list below identifies, objectively, common programming faults and follies. The rest of this document goes into more detail on these “code smells” and provides some guidance as to how these problems can be avoided.
Rule of Thumb: “A code review should only judge the clarity and substance of the code – not the style.” It is important to note that we are not reviewing the style of coding, only whether that code violates best practices or potentially introduces performance or maintenance issues. To sum up, if the code can stand on its own (is understandable without explanation), there are no potential faults from performance or maintenance issues, and has proper unit tests - it should pass code review.
During code review
Critique the code, not the coder
Relate comments to standards, specs, performance etc.
Avoid “Why didn’t you” and replace with “What was the reasoning behind the deviation of standards here..”
Code formatting is important and style, to a certain extent, is as well, but style should not be the basis for rejecting otherwise readable and valid code.
Does the code implement the design?
Is the code easily understandable?
Is the code developed using a TDD approach, or at the very least have tests for code written / modified?
Reasons for Rejecting Code:
Unit test code coverage doesn’t cover the code under review.
Code doesn’t compile / doesn’t function as expected (smoke test).
Code that doesn’t follow:
Overview / Guidelines
Code should be reviewed before being made available to QA
During development
Static code analysis should be run to ensure:
Code adheres to recognized standards and coding styles
Doesn’t introduce security issues
Developer should strive to produce code that meets the business need and is easy to maintain
Developed using a TDD approach, or at the very least have tests for code written
Code Smells Detail - warnings, something that should be reviewed in detail
Consider using null object pattern instead of null checks
If there is a comment above a variable or method that does a better job explaining the functionality of the variable or method name, consider changing the name of the variable or method to be more descriptive.
NO magic numbers, magic strings,
Instead try Enums, constant numbers/strings with meaningful names.
NO commented-out code
Instead write more descriptive check-in comments when removing
If you see the word new (constructing an object)
Instead declare dependencies through constructor arguments that are of the type's interface
Coding should in most cases be done against interfaces
If you need to scroll to see the entire contents of a procedure or method:
It is likely too complex and may benefit from being broken down into smaller pieces.
Specific-Case Code Smells:
Duplicate Code: Don’t repeat yourself; if you are doing the same thing in more than one spot, extract it to a method.
Combinatorial Explosion: When you see a lot of code doing almost the same thing, it may be a case of combinatorial explosion, a form of repeating code. Consider refactoring methods to interfaces or make use of generics.
Long Parameter List: If you see a method that takes 4 or more arguments it may be too complex - and it will definitely prove difficult to test. Aim for smaller functions that take 0-2 arguments, if possible, OR consider combining the parameter list into an object.
Large Class: A bunch of smaller more focused classes are preferable to a large and complex single class. If you encounter a large class consider refactoring it out into smaller more easily testable classes.
Types Embedded in Names: Don’t put the type of the variable in the name since the variable’s name will need to change if its type changes.
Uncommunicative Name: Methods should describe what they do in their name, variable names should describe the value they hold. If the method name says it does one thing, but appears to do many things, that can mislead you and cause bugs. When naming a method, keep it descriptive, if the name is too long, it’s likely doing too much.
Standardization: Follow standard naming and coding conventions
Apex
Additional Resources:
Apex Enterprise Patterns:
Javascript:
Avoid anonymous callback functions, especially nested anonymous callback functions, (which are not testable) and instead call public functions on the return of asynchronous code.
use strict should be set
Variable Declaration
Check to see if variable declaration is inline with variable hoisting
Items addressed by Human Code Review:
Questions to ask
Did the method you modified become easier or harder to maintain after you added your changes?
It is desirable to segment the codebase into business domains that are not interdependent so that each business domain can evolve at its own rate and change can be isolated and changes moved to production independently.
In a microservices driven architecture there are significant advantages of ensuring that each microservice own the data that it interacts with and that no other microservice interacts with another's data directly. The net result allows microservice teams to evolve their codebase and underlying data structures/data stores with confidence that it will not impact any other part of the system.
Resources:
Eric Evans -
Andrew Fawcett -
Resources:
Martin Fowler -
Andrew Fawcett -
To help ensure queries are optimized, organized and reusable it may be important to implement a selector layer.
Resources:
To minimize impact to production users of new features and to allow for better understanding of user usage it may desirable to employ canary releases and AB testing functionality. Canary releases allow releasing into production functionality but control the audience impacted, for example starting with production test accounts to allow QA to validate functionality in production, then to a smaller portion of the userbase and then finally to the entire userbase. We desire AB testing to allow the business to perform trials of functionality to determine if yields the expected outcome.
Resources:
Andrew Fawcett suggests using a factory pattern to resolve services in Salesforce, it would be ideal have Service Registry / Discovery implementation. Developing an approahc resolving services would be beneficial. (i.e. ideally not having a direct reference between a visual force or lightning component controller and the service(s) it consumes) Having the ability to switch out implementations, or have multiple implementations with different functionality running in parallel is desired (for example speech to text with both Salesforce Eintstein and Amazon services)
Resources:
Providing runtime analytics to SREs at the org enables detailed behavior understanding.
Resources:
Leverage online tools for quickly checking code
Andrew Fawcett - Video -
Andrew Fawcett -
Martin Fowler -
Salesforce Maximizer Blog:
Service Registry -
Event Monitoring - '
Financial Times Approach -
Using with LogStash - Plugin -
Integration Example :
Don'ts
Do's
Long parameter lists (>3 params)
Code should follow SOLID principles
Null checks
Unnecessary comments / commented-out code
Magic numbers / strings
Object instantiation
Duplicate code
Combinatorial explosion
Large classes
Type names embedded in names
Uncommunicative Names