Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added static code analysis / PMD #201

Merged
merged 5 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ jobs:
- name: 'Prettier code formatting verification'
run: npm run prettier:verify

# TODO setup apex-scanner and PMD rules - https://github.com/forcedotcom/sfdx-scanner
# - name: Install & run SFDX Scanner
# run: |
# sfdx plugins:install @salesforce/sfdx-scanner
# sfdx scanner:run --pmdconfig config/pmd-ruleset.xml --target . --engine pmd --severity-threshold 3
- name: Install Salesforce CLI
run: npm install sfdx-cli --global

- name: Install & run SFDX Scanner
run: |
sfdx plugins:install @salesforce/sfdx-scanner
sfdx scanner:run --pmdconfig config/pmd-ruleset.xml --target . --engine pmd --severity-threshold 3

lwc-tests:
name: 'LWC Tests'
Expand Down
45 changes: 45 additions & 0 deletions config/pmd-ruleset.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?xml version="1.0" ?>
<ruleset
name="Nebula Logger Rules"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd"
>
<description>
Nebula Logger custom PMD ruleset
</description>
<rule ref="category/apex/bestpractices.xml">
<exclude name="ApexAssertionsShouldIncludeMessage" />
<exclude name="AvoidGlobalModifier" />
<exclude name="AvoidLogicInTrigger" />
</rule>
<rule ref="category/apex/codestyle.xml">
<exclude name="FieldNamingConventions" />
<exclude name="MethodNamingConventions" />
<exclude name="PropertyNamingConventions" />
<exclude name="VariableNamingConventions" />
</rule>
<rule ref="category/apex/design.xml">
<exclude name="AvoidDeeplyNestedIfStmts" />
<exclude name="CognitiveComplexity" />
<exclude name="CyclomaticComplexity" />
<exclude name="ExcessiveClassLength" />
<exclude name="ExcessiveParameterList" />
<exclude name="ExcessivePublicCount" />
<exclude name="NcssConstructorCount" />
<exclude name="NcssMethodCount" />
<exclude name="StdCyclomaticComplexity" />
<exclude name="TooManyFields" />
</rule>
<rule ref="category/apex/documentation.xml">
<exclude name="ApexDoc" />
</rule>
<rule ref="category/apex/errorprone.xml">
<exclude name="EmptyStatementBlock" />
</rule>
<rule ref="category/apex/performance.xml">
<exclude name="AvoidDebugStatements" />
</rule>
<rule ref="category/apex/security.xml">
</rule>
</ruleset>
4 changes: 2 additions & 2 deletions extra-tests/classes/ExampleInboundEmailHandler.cls
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ global with sharing class ExampleInboundEmailHandler implements Messaging.Inboun
Logger.error(logEntryMessage, apexException);
} finally {
result.success = true;
System.debug('Logger buffer size before save: ' + Logger.getBufferSize());
System.debug(LoggingLevel.DEBUG, 'Logger buffer size before save: ' + Logger.getBufferSize());
Logger.saveLog();
System.debug('Logger buffer size after save: ' + Logger.getBufferSize());
System.debug(LoggingLevel.DEBUG, 'Logger buffer size after save: ' + Logger.getBufferSize());
}

return result;
Expand Down
1 change: 0 additions & 1 deletion extra-tests/tests/Logger_Tests_InboundEmailHandler.cls
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ private class Logger_Tests_InboundEmailHandler {
Messaging.InboundEmail email = new Messaging.InboundEmail();
email.plainTextBody = 'Example email content';
email.fromAddress = '[email protected]';
String contactEmail = '[email protected]';
email.subject = 'My example email';

// Create an instance of the example handler class
Expand Down
10 changes: 5 additions & 5 deletions nebula-logger-plugins/Slack/classes/SlackLoggerPlugin.cls
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ public without sharing class SlackLoggerPlugin extends LoggerSObjectHandlerPlugi
@TestVisible
private static LoggingLevel notificationLoggingLevel;

private List<Log__c> logs;

static {
endpoint = LoggerParameter.Plugin.getString('SlackEndpoint');

String notificationLoggingLevelName = LoggerParameter.Plugin.getString('SlackNotificationLoggingLevel');
notificationLoggingLevel = Logger.getLoggingLevel(notificationLoggingLevelName);
}

private List<Log__c> logs;

// Constructors
public SlackLoggerPlugin() {
}
Expand Down Expand Up @@ -78,8 +78,8 @@ public without sharing class SlackLoggerPlugin extends LoggerSObjectHandlerPlugi
request.setBody(notificationJson);

HttpResponse response = new Http().send(request);
System.debug('response.getStatusCode()==' + response.getStatusCode());
System.debug('response.getStatus()==' + response.getStatus());
System.debug(notificationLoggingLevel, 'response.getStatusCode()==' + response.getStatusCode());
System.debug(notificationLoggingLevel, 'response.getStatus()==' + response.getStatus());

log.SlackNotificationDate__c = System.now();
sentLogs.add(log);
Expand Down Expand Up @@ -163,7 +163,7 @@ public without sharing class SlackLoggerPlugin extends LoggerSObjectHandlerPlugi
}

private HttpRequest createSlackHttpRequest() {
System.debug('endpoint==' + endpoint);
System.debug(notificationLoggingLevel, 'endpoint==' + endpoint);

HttpRequest request = new HttpRequest();
request.setEndpoint(endpoint);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ global with sharing class ExampleInboundEmailHandler implements Messaging.Inboun
Logger.error(logEntryMessage, apexException);
} finally {
result.success = true;
System.debug('Logger buffer size before save: ' + Logger.getBufferSize());
System.debug(LoggingLevel.DEBUG, 'Logger buffer size before save: ' + Logger.getBufferSize());
Logger.saveLog();
System.debug('Logger buffer size after save: ' + Logger.getBufferSize());
System.debug(LoggingLevel.DEBUG, 'Logger buffer size after save: ' + Logger.getBufferSize());
}

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ private class Logger_Tests_InboundEmailHandler {
Messaging.InboundEmail email = new Messaging.InboundEmail();
email.plainTextBody = 'Example email content';
email.fromAddress = '[email protected]';
String contactEmail = '[email protected]';
email.subject = 'My example email';

// Create an instance of the example handler class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* @group Log Management
* @description Processes `LogEntryEvent__e` platform events and normalizes the data into `Log__c` and `LogEntry__c` records
*/
@SuppressWarnings('PMD.ApexCRUDViolation')
public without sharing class LogEntryEventHandler extends LoggerSObjectHandler {
private static final Map<String, Log__c> TRANSACTION_ID_TO_LOG = new Map<String, Log__c>();
@TestVisible
Expand Down Expand Up @@ -276,6 +277,7 @@ public without sharing class LogEntryEventHandler extends LoggerSObjectHandler {
insert new List<SObject>(tagAssignments);
}

@SuppressWarnings('PMD.ApexSOQLInjection')
private Map<String, Id> getTagNameToId(Schema.SObjectType tagSObjectType) {
Map<String, Id> tagNameToId = new Map<String, Id>();

Expand Down Expand Up @@ -396,7 +398,7 @@ public without sharing class LogEntryEventHandler extends LoggerSObjectHandler {

@future(callout=true)
private static void setStatusApiDetails() {
System.debug('Running setStatusApiDetails()');
System.debug(LoggingLevel.DEBUG, 'Running setStatusApiDetails()');

Organization organization = [SELECT InstanceName FROM Organization];
String statusApiEndpoint = 'https://api.status.salesforce.com/v1/instances/' + organization.InstanceName + '/status';
Expand All @@ -419,7 +421,7 @@ public without sharing class LogEntryEventHandler extends LoggerSObjectHandler {
}

StatusApiResponse statusApiResponse = (StatusApiResponse) JSON.deserialize(response.getBody(), StatusApiResponse.class);
System.debug('statusApiResponse==' + statusApiResponse);
System.debug(LoggingLevel.DEBUG, 'statusApiResponse==' + statusApiResponse);

List<Log__c> logsToUpdate = new List<Log__c>();
for (Log__c log : [
Expand All @@ -434,7 +436,7 @@ public without sharing class LogEntryEventHandler extends LoggerSObjectHandler {

logsToUpdate.add(log);
}
System.debug('logsToUpdate==' + logsToUpdate);
System.debug(LoggingLevel.DEBUG, 'logsToUpdate==' + logsToUpdate);
update logsToUpdate;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public without sharing class LogEntryHandler extends LoggerSObjectHandler {
}
}

@SuppressWarnings('PMD.OperationWithLimitsInLoop')
private void setRecordNames() {
// Assumption - only valid record IDs will be populated in LogEntry__c.RecordId__c
// If that changes, then extra checks may be needed before casting to Id, using getSObjectType(), etc.
Expand Down
19 changes: 9 additions & 10 deletions nebula-logger/main/log-management/classes/LogHandler.cls
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ public without sharing class LogHandler extends LoggerSObjectHandler {
@TestVisible
private static Map<String, LogStatus__mdt> logStatusByName = loadActiveLogStatuses();

@TestVisible
private List<Log__c> logs;
@TestVisible
private Map<Id, Log__c> oldLogsById;

private static Map<String, LogStatus__mdt> loadActiveLogStatuses() {
Map<String, LogStatus__mdt> logStatusByName = new Map<String, LogStatus__mdt>();
for (LogStatus__mdt logStatus : LogStatus__mdt.getAll().values()) {
Expand All @@ -22,12 +27,6 @@ public without sharing class LogHandler extends LoggerSObjectHandler {
return logStatusByName;
}

@TestVisible
private List<Log__c> logs;

@TestVisible
private Map<Id, Log__c> oldLogsById;

/**
* @description Returns SObject Type that the handler is responsible for processing
* @return The instance of `SObjectType`
Expand Down Expand Up @@ -133,16 +132,16 @@ public without sharing class LogHandler extends LoggerSObjectHandler {
// 1. Assume that that there will always be 3+ picklist values for the Priority__c field (out of the box, the values are: High, Medium, Low)
// 2. Assume that not everyone will want those exact values, so dynamiclly get picklist entries (e.g., some orgs may instead use Critical, High, Medium, Low)
// 3. Assume that the picklist entries are sorted in order of priority (not alphabetically, etc.)
final String FIRST_PRIORITY = picklistEntries.get(0).getValue();
final String SECOND_PRIORITY = picklistEntries.get(1).getValue();
final String firstPriority = picklistEntries.get(0).getValue();
final String secondPriority = picklistEntries.get(1).getValue();

for (Log__c log : this.logs) {
Log__c oldLog = this.oldLogsById.get(log.Id);

if (log.TotalERRORLogEntries__c != oldLog.TotalERRORLogEntries__c && log.TotalERRORLogEntries__c > 0) {
log.Priority__c = FIRST_PRIORITY;
log.Priority__c = firstPriority;
} else if (log.TotalWARNLogEntries__c != oldLog.TotalWARNLogEntries__c && log.TotalWARNLogEntries__c > 0) {
log.Priority__c = SECOND_PRIORITY;
log.Priority__c = secondPriority;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public with sharing class LogMassDeleteExtension {
* @description Filters the list of selected `Log__c` records to only include records that the current user can delete (based on object-level access)
* @return The matching `Log__c` records that the current user has access to delete
*/
@SuppressWarnings('PMD.ApexCRUDViolation')
public List<Log__c> getDeletableLogs() {
// The UserRecordAccess object is weird - RecordId is not an actual ID field, so you can't filter using `List<SObject>` or `List<Id>`, you have to use strings
// So, here's some code that would be unnecessary if RecordId were a polymorphic ID field instead
Expand Down
31 changes: 16 additions & 15 deletions nebula-logger/main/log-management/classes/LoggerSObjectHandler.cls
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,25 @@
* @group Log Management
* @description Abstract class used by trigger handlers for shared logic
*/
public abstract class LoggerSObjectHandler {
@SuppressWarnings('PMD.ApexCRUDViolation')
public without sharing abstract class LoggerSObjectHandler {
private static Map<SObjectType, LoggerSObjectHandler__mdt> configurationBySObjectType;
private static Map<SObjectType, List<LoggerSObjectHandlerPlugin__mdt>> pluginsBySObjectType;

@TestVisible
private TriggerOperation triggerOperationType;
@TestVisible
private List<SObject> triggerNew;
@TestVisible
private Map<Id, SObject> triggerNewMap;
@TestVisible
private List<SObject> triggerOld;
@TestVisible
private Map<Id, SObject> triggerOldMap;

private LoggerSObjectHandler__mdt handlerConfiguration;
private List<LoggerSObjectHandlerPlugin__mdt> plugins;

static {
// When using CMDT's getAll(), it does not return relationship fields for EntityDefinition fields or child CMDT objects...
// ... so instead query the LoggerSObjectHandler__mdt CMDT object
Expand Down Expand Up @@ -65,20 +80,6 @@ public abstract class LoggerSObjectHandler {
pluginsBySObjectType.put(sobjectType, plugins);
}

@TestVisible
private TriggerOperation triggerOperationType;
@TestVisible
private List<SObject> triggerNew;
@TestVisible
private Map<Id, SObject> triggerNewMap;
@TestVisible
private List<SObject> triggerOld;
@TestVisible
private Map<Id, SObject> triggerOldMap;

private LoggerSObjectHandler__mdt handlerConfiguration;
private List<LoggerSObjectHandlerPlugin__mdt> plugins;

public LoggerSObjectHandler() {
this.setConfigurations();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public with sharing class RelatedLogEntriesController {
* @param search An optional search term to filter by
* @return The instance of LogEntryQueryResult, containing matching records and metadata
*/
@SuppressWarnings('PMD.ApexCRUDViolation')
@AuraEnabled(cacheable=true)
public static LogEntryQueryResult getQueryResult(
Id recordId,
Expand Down Expand Up @@ -84,7 +85,7 @@ public with sharing class RelatedLogEntriesController {
};
String logEntryQuery = 'SELECT {0} FROM {1} WHERE {2} = :recordId ORDER BY {3} LIMIT {4}';
logEntryQuery = String.format(logEntryQuery, queryTextReplacements);
System.debug('logEntryQuery==' + logEntryQuery);
System.debug(LoggingLevel.DEBUG, 'logEntryQuery==' + logEntryQuery);

return (List<LogEntry__c>) Database.query(logEntryQuery);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public inherited sharing class ComponentLogger {

private static void setStackTraceDetails(LogEntryEventBuilder logEntryEventBuilder, String stackTraceString) {
String originLocation;
String originType;
Boolean isAuraComponent = false;

if (String.isNotBlank(stackTraceString) == true) {
Expand Down
23 changes: 12 additions & 11 deletions nebula-logger/main/logger-engine/classes/LogEntryEventBuilder.cls
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,19 @@
* @description Builder class that generates each `LogEntryEvent__e` record
* @see Logger
*/
@SuppressWarnings('PMD.NcssTypeCount, PMD.ApexCRUDViolation')
global with sharing class LogEntryEventBuilder {
private final LogEntryEvent__e logEntryEvent;
private final LoggingLevel entryLoggingLevel;

@TestVisible
private String debugMessage = '';

private Boolean detailsAreSet = false;
private Boolean shouldSave;
private Set<String> tags = new Set<String>();
private Boolean tagDetailsAreSet = true;

private static final String API_VERSION = getApiVersion();
private static final List<String> IGNORED_CLASSES = getIgnoredClasses();
private static final String NAMESPACE_PREFIX = getNamespacePrefix();
Expand Down Expand Up @@ -77,17 +89,6 @@ global with sharing class LogEntryEventBuilder {
set;
}

private final LogEntryEvent__e logEntryEvent;
private final LoggingLevel entryLoggingLevel;

@TestVisible
private String debugMessage = '';

private Boolean detailsAreSet = false;
private Boolean shouldSave;
private Set<String> tags = new Set<String>();
private Boolean tagDetailsAreSet = true;

/**
* @description Used by `Logger` to instantiate a new instance of `LogEntryEventBuilder`
* @param entryLoggingLevel The `LoggingLevel` enum to use for the builder's instance of `LogEntryEvent__e`
Expand Down
Loading