npx skills add ...
npx skills add github/awesome-copilot --skill salesforce-apex-quality
npx skills add github/awesome-copilot --skill salesforce-apex-quality
Apex code quality guardrails for Salesforce development. Enforces bulk-safety rules (no SOQL/DML in loops), sharing model requirements, CRUD/FLS security, SOQL injection prevention, PNB test coverage (Positive / Negative / Bulk), and modern Apex idioms. Use this skill when reviewing or generating Apex classes, trigger handlers, batch jobs, or test classes to catch governor limit risks, security gaps, and quality issues before deployment.
Apply these checks to every Apex class, trigger, and test file you write or review.
Scan for these patterns before declaring any Apex file acceptable:
Rule: if you see [SELECT or Database.query, insert, update, delete, upsert, merge inside a for loop body — stop and refactor before proceeding.
Every class must declare its sharing intent explicitly. Undeclared sharing inherits from the caller — unpredictable behaviour.
| Declaration | When to use |
|---|---|
public with sharing class Foo | Default for all service, handler, selector, and controller classes |
public without sharing class Foo | Only when the class must run elevated (e.g. system-level logging, trigger bypass). Requires a code comment explaining why. |
public inherited sharing class Foo | Framework entry points that should respect the caller's sharing context |
If a class does not have one of these three declarations, add it before writing anything else.
Apex code that reads or writes records on behalf of a user must verify object and field access. The platform does not enforce FLS or CRUD automatically in Apex.
Rule: any Apex method callable from a UI component, REST endpoint, or @InvocableMethod must enforce CRUD/FLS. Internal service methods called only from trusted contexts may use with sharing instead.
Prefer current language features (API 62.0 / Winter '25+):
| Old pattern | Modern replacement |
|---|---|
if (obj != null) { x = obj.Field__c; } | x = obj?.Field__c; |
x = (y != null) ? y : defaultVal; | x = y ?? defaultVal; |
System.assertEquals(expected, actual) | Assert.areEqual(expected, actual) |
System.assert(condition) | Assert.isTrue(condition) |
[SELECT ... WHERE ...] with no sharing context | [SELECT ... WHERE ... WITH USER_MODE] |
Every feature must be tested across all three paths. Missing any one of these is a quality failure:
Test.startTest() / Test.stopTest() to isolate governor limit counters for async work.with sharing unless the trigger requires elevated access.| Pattern | Action |
|---|---|
SOQL inside for loop | Refactor: query before the loop, operate on collections |
DML inside for loop | Refactor: collect mutations, DML once after the loop |
| Class missing sharing declaration | Add with sharing (or document why without sharing) |
escape="false" on user data (VF) | Remove — auto-escaping enforces XSS prevention |
Empty catch block | Add logging and appropriate re-throw or error handling |
| String-concatenated SOQL with user input | Replace with bind variable or whitelist validation |
| Test with no assertion | Add a meaningful Assert.* call |
System.assert / System.assertEquals style | Upgrade to Assert.isTrue / Assert.areEqual |
Hardcoded record ID ('001...') | Replace with queried or inserted test record ID |
// Check before querying a field
if (!Schema.sObjectType.Contact.fields.Email.isAccessible()) {
throw new System.NoAccessException();
}
// Or use WITH USER_MODE in SOQL (API 56.0+)
List<Contact> contacts = [SELECT Id, Email FROM Contact WHERE AccountId = :accId WITH USER_MODE];
// Or use Database.query with AccessLevel
List<Contact> contacts = Database.query('SELECT Id, Email FROM Contact', AccessLevel.USER_MODE);// ❌ NEVER — concatenates user input into SOQL string
String soql = 'SELECT Id FROM Account WHERE Name = \'' + userInput + '\'';
// ✅ ALWAYS — bind variable
String soql = [SELECT Id FROM Account WHERE Name = :userInput];
// ✅ For dynamic SOQL with user-controlled field names — validate against a whitelist
Set<String> allowedFields = new Set<String>{'Name', 'Industry', 'AnnualRevenue'};
if (!allowedFields.contains(userInput)) {
throw new IllegalArgumentException('Field not permitted: ' + userInput);
}@isTest(SeeAllData=false) // Required — no exceptions without a documented reason
private class AccountServiceTest {
@TestSetup
static void makeData() {
// Create all test data here — use a factory if one exists in the project
}
@isTest
static void givenValidInput_whenProcessAccounts_thenFieldsUpdated() {
// Positive path
List<Account> accounts = [SELECT Id FROM Account LIMIT 10];
Test.startTest();
AccountService.processAccounts(accounts);
Test.stopTest();
// Assert meaningful outcomes — not just no exception
List<Account> updated = [SELECT Status__c FROM Account WHERE Id IN :accounts];
Assert.areEqual('Processed', updated[0].Status__c, 'Status should be Processed');
}
}