Avoid the "Clean Code Shock" with PMD

Your new year resolution includes "Write clean Apex code". So you run PMD with a full ruleset and get shocked by the number of violations. You drop the resolution in a blink.

Don't boil the Ocean

Even a journey of a thousand miles starts with a single step, so let's break down the task into manageable chunks to divide and rule.
There are 2 dimensions you can use: Type of code and priority levels. Using them you can turn your Clean Code journey into manageable stages.

Code Types

• Legacy code: all code that doesn't fall in any of the two other categories
• Changed code: code that needs change due to business requirements
• New code: new code written for new or changed functionality (applies to copy & paste too)

Priority Levels

• 1 = security and performance, will fail build
• 2 = bad code, will fail build
• 3 & 4 = hard to maintain code, will generate warning
• 5 = ugly code, will generate hint

PMD rules for code types should have different priorities. A different number of tests will fail a build:

• 11 for legacy code (all around performance and security)
• 33 for changed code
• 44 for new code

This will require to run PMD with different rulesets on subsets of your code base

A pragmatic rule set

|Rule Set |Rule Name |Legacy code|Updated Code|New Code|
|--------------|--------------------------------------|:---------:|:----------:|:------:|
|Best Practices|ApexUnitTestShouldNotUseSeeAllDataTrue| 3 | 2 | 2 |
| |ApexUnitTestClassShouldHaveAsserts | 3 | 2 | 2 |
| |AvoidLogicInTrigger | 2 | 2 | 2 |
| |AvoidGlobalModifier | 5 | 4 | 3 |
|Code Style |ClassNamingConventions | 5 | 3 | 1 |
| |ForLoopsMustUseBraces | 4 | 2 | 1 |
| |IfStmtsMustUseBraces | 4 | 2 | 1 |
| |IfElseStmtsMustUseBraces | 5 | 2 | 1 |
| |MethodNamingConventions | 5 | 3 | 1 |
| |OneDeclarationPerLine | 5 | 2 | 1 |
| |VariableNamingConventions | 5 | 3 | 1 |
| |WhileLoopsMustUseBraces | 4 | 2 | 1 |
|Design |AvoidDeeplyNestedIfStmts | 3 | 2 | 2 |
| |CyclomaticComplexity | 3 | 3 | 2 |
| |ExcessiveClassLength | 3 | 3 | 2 |
| |ExcessiveParameterList | 3 | 3 | 2 |
| |ExcessivePublicCount | 3 | 3 | 2 |
| |NcssConstructorCount | 3 | 2 | 2 |
| |NcssMethodCount | 3 | 3 | 2 |
| |NcssTypeCount | 3 | 3 | 2 |
| |StdCyclomaticComplexity | 3 | 3 | 2 |
| |TooManyFields | 3 | 3 | 3 |
|Error Prone |AvoidDirectAccessTriggerMap | 2 | 2 | 1 |
| |AvoidHardcodingId | 2 | 2 | 1 |
| |EmptyCatchBlock | 2 | 2 | 2 |
| |EmptyIfStmt | 3 | 2 | 2 |
| |EmptyStatementBlock | 3 | 3 | 2 |
| |EmptyTryOrFinallyBlock | 3 | 3 | 2 |
| |EmptyWhileStmt | 3 | 3 | 2 |
| |MethodWithSameNameAsEnclosingClass | 3 | 3 | 2 |
|Performance |AvoidDmlStatementsInLoops | 1 | 1 | 1 |
| |AvoidSoslInLoops | 1 | 1 | 1 |
| |AvoidSoqlInLoops | 1 | 1 | 1 |
|Security |ApexCRUDViolation | 3 | 2 | 1 |
| |ApexInsecureEndpoint | 2 | 2 | 2 |
| |ApexOpenRedirect | 3 | 2 | 2 |
| |ApexSharingViolations | 3 | 3 | 2 |
| |ApexSOQLInjection | 1 | 1 | 1 |
| |ApexXSSFromURLParam | 1 | 1 | 1 |
| |VfCsrf | 1 | 1 | 1 |
| |ApexXSSFromEscapeFalse | 3 | 2 | 1 |
| |ApexBadCrypto | 1 | 1 | 1 |
| |ApexCSRF | 3 | 2 | 2 |
| |ApexDangerousMethods | 3 | 2 | 2 |
| |ApexSuggestUsingNamedCred | 1 | 1 | 1 |
| |VfUnescapeEl | 1 | 1 | 1 |

As usual: YMMV

You need to tune the rule to match your code quality of your existing base. The most controversial would be the "updated code" rule.If a business requirement mandates to add 3-4 lines, but the PMD rule would fail on complexity, it becomes hard to justify to upfront work, so you might, initially, have more relaxed rules in place.