ActionPMD
Frequently Asked Questions
Why static code analysis?
- to make sure the code is easy to understand to someone else / yourself after a few months
- to discover some bugs early
- to beat Eclipse in this rank: http://findbugs.cs.umd.edu/demo
Why PMD?
- it is fast to use (it works with source files as opposed to FindBugs that works with generated bytecode)
- it has a much more convenient UI than FindBugs
- most checks from other static analysis tools (like FindBugs) overlap, thus cleaning code using PMD will decrease the number of issues found with other tools as well
- we recommend to use PMD with the common ruleset for daily work and FindBugs from time to time
- we would like to see the most useful checks integrated into core NB one day (6.0?)
Why have a common ruleset?
- some rules are controversial (some find them useful, others not) or even contradictory, developers can save time on evaluating rules by themselves
- it is easier to maintain your own code clean if there are few problem reports around (other developers use PMD regularly with the same ruleset)
How do you decide, which rules to include in the common rule set
- we started with a full ruleset (assumed that every rule was useful)
- we browsed the descriptions and examples of use of all the rules and excluded those we disagreed with
- we applied the rules against existing NB modules and analyzed the output to see if we didn't overlook any rules in the previous step
- the ruleset is opened to changes, please share your feedback!
Setup
- Download PMD plugin
- Recommended PMD config file ( pmd-config-option_ActionPMD.settings ). Copy it to <userdir>/config/Services.
Surpassing warnings
- avoid doing it!
- always provide a comment
- exact procedure to be defined (do we want to commit the NOPMD/JSRXXX tags?)
How do we start?
- make sure there are no problems in newly developed code
- choose some parts of your existing code to be cleaned up and do it, just to familiarize yourself with the tool
- avoid mixing bug fixes and refactorings in one commit, it makes sustaining team's work more difficult
- eventually we might want to prohibit committing files with PMD-found problems to CVS and use the continues build infrastructure to enforce it
- the ultimate goal is 0 problems in our code
Excluded PMD rules (checks we do not want to perform)
Out of the list of currently excluded rules, we are considering re-including some of them:
| Rule | Why it should be reincluded |
|---|---|
| MethodNamingConvention | It should have been included from the beginning (ClassNamingConventions is there) |
| UseLocaleWithCaseConversions | http://www.netbeans.org/community/guidelines/code-conventions.html |
Additionally to the list of currently excluded rules, we are considering excluding a few more:
| Rule | Why it should be excluded |
|---|---|
| JUnitAssertionsShouldIncludeMessage | Usually a message is not needed and the meaning of the assert is clear from its context |
Rules currently excluded:
| Rule | Why we don't want it |
|---|---|
| AbstractClassWithoutAbstractMethod | |
| AbstractNaming | |
| AssignmentInOperand | |
| AtLeastOneConstructor | |
| AvoidDeeplyNestedIfStmts | |
| AvoidDuplicateLiterals | Often there is no good reason for defining a literal |
| AvoidInstantiatingObjectsInLoops | HotSpot should be smart enough |
| AvoidSynchronizedAtMethodLevel | |
| AvoidThrowingNullPointerException | |
| AvoidThrowingRawExceptionTypes | |
| BeanMembersShouldSerialize | |
| BooleanInversion | |
| CallSuperInConstructor | |
| CollapsibleIfStatements | |
| CompareObjectsWithEquals | |
| ConfusingTernary | |
| ConsecutiveLiteralAppends | |
| CouplingBetweenObjects | |
| CyclomaticComplexity | |
| DefaultPackage | |
| ExcessiveClassLength | |
| ExcessiveImports | |
| ExcessiveMethodLength | |
| ExcessiveParameterList | |
| ExcessivePublicCount | |
| ForLoopsMustUseBraces | |
| IfElseStmtsMustUseBraces | |
| IfStmtsMustUseBraces | |
| InefficientEmptyStringCheck | Workaround (a method which iterates over the string looking for non-whitespace) is too impractical |
| InsufficientStringBufferDeclaration | Too low-level, usually small performance impact |
| LocalVariableCouldBeFinal | |
| LongVariable | |
| LooseCoupling | |
| MethodArgumentCouldBeFinal | |
| MethodNamingConventions | |
| MissingSerialVersionUID | |
| NullAssignment | |
| OnlyOneReturn | |
| OptimizableToArrayCall | |
| SingularField | |
| SuspiciousConstantFieldName | |
| SuspiciousOctalEscape | |
| SwitchStmtsShouldHaveDefault | |
| TooManyFields | |
| UncommentedEmptyConstructor | |
| UncommentedEmptyMethod | |
| UnnecessaryConstructor | |
| UnnecessaryLocalBeforeReturn | |
| UnnecessaryParentheses | |
| UnusedFormalParameter | |
| UnusedModifier | |
| UseLocaleWithCaseConversions | |
| UseNotifyAllInsteadOfNotify | |
| UseSingleton | |
| WhileLoopsMustUseBraces | |
