Added static code analysis / PMD (#201)

* Fixes #149 by adding PMD through sfdx-scanner * Apologizing to prettier by correctly running pmd-ruleset.xml through * Correcting debug statement ordering - LoggingLevel comes first! * Adding SFDX prior to running scanner * Removing unused line as per @jongpie's feedback
jongpie · Sep 13, 2021 · de176f8 · de176f8
1 parent 07d82af
commit de176f8
Show file tree

Hide file tree

Showing 23 changed files with 130 additions and 77 deletions.
diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
@@ -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'

diff --git a/config/pmd-ruleset.xml b/config/pmd-ruleset.xml
@@ -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>
diff --git a/extra-tests/classes/ExampleInboundEmailHandler.cls b/extra-tests/classes/ExampleInboundEmailHandler.cls
@@ -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;

diff --git a/extra-tests/tests/Logger_Tests_InboundEmailHandler.cls b/extra-tests/tests/Logger_Tests_InboundEmailHandler.cls
@@ -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

diff --git a/nebula-logger-plugins/Slack/classes/SlackLoggerPlugin.cls b/nebula-logger-plugins/Slack/classes/SlackLoggerPlugin.cls
@@ -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() {
     }
@@ -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);
@@ -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);

diff --git a/nebula-logger-recipes/main/default/classes/ExampleInboundEmailHandler.cls b/nebula-logger-recipes/main/default/classes/ExampleInboundEmailHandler.cls
@@ -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;

diff --git a/nebula-logger-recipes/tests/Logger_Tests_InboundEmailHandler.cls b/nebula-logger-recipes/tests/Logger_Tests_InboundEmailHandler.cls
@@ -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

diff --git a/nebula-logger/main/log-management/classes/LogEntryEventHandler.cls b/nebula-logger/main/log-management/classes/LogEntryEventHandler.cls
@@ -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
@@ -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>();
 
@@ -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';
@@ -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 : [
@@ -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;
     }
 

diff --git a/nebula-logger/main/log-management/classes/LogEntryHandler.cls b/nebula-logger/main/log-management/classes/LogEntryHandler.cls
@@ -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.

diff --git a/nebula-logger/main/log-management/classes/LogHandler.cls b/nebula-logger/main/log-management/classes/LogHandler.cls
@@ -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()) {
@@ -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`
@@ -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;
             }
         }
     }

diff --git a/nebula-logger/main/log-management/classes/LogMassDeleteExtension.cls b/nebula-logger/main/log-management/classes/LogMassDeleteExtension.cls
@@ -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

diff --git a/nebula-logger/main/log-management/classes/LoggerSObjectHandler.cls b/nebula-logger/main/log-management/classes/LoggerSObjectHandler.cls
@@ -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
@@ -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();
 

diff --git a/nebula-logger/main/log-management/classes/RelatedLogEntriesController.cls b/nebula-logger/main/log-management/classes/RelatedLogEntriesController.cls
@@ -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,
@@ -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);
     }

diff --git a/nebula-logger/main/logger-engine/classes/ComponentLogger.cls b/nebula-logger/main/logger-engine/classes/ComponentLogger.cls
@@ -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) {

diff --git a/nebula-logger/main/logger-engine/classes/LogEntryEventBuilder.cls b/nebula-logger/main/logger-engine/classes/LogEntryEventBuilder.cls
@@ -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();
@@ -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`