> Other options (raised on the Hadoop list) are Checkstyle:
PMD seems to be the best choice for an Apache project and they all seem
to perform at a similar level.
Anything that generates a lot of false positives is bad: it either
causes us to skip analysis of lots of files, or ignore the warnings.
Skipping the JavaCC-generated classes is reasonable, but I'm wary of
skipping much else.
I thought a bit about this. The warnings PMD may actually make sense to
fix. Take a look at maxDoc here:
class LuceneQueryOptimizer {
private static class LimitExceeded extends RuntimeException {
private int maxDoc;
public LimitExceeded(int maxDoc) { this.maxDoc = maxDoc; }
}
...
maxDoc is accessed from LuceneQueryOptimizer which requires a synthetic
accessor in LimitExceeded. It also may look confusing because you
declare a field private to a class, but use it from the outside...
changing declarations to something like this:
class LuceneQueryOptimizer {
private static class LimitExceeded extends RuntimeException {
final int maxDoc;
public LimitExceeded(int maxDoc) { this.maxDoc = maxDoc; }
}
...
removes the warning and also seems to make more sense (note that package
scope of maxDoc doesn't really expose it much more than before because
the entire class is private).
So... if you agree to change existing warnings as shown above (there's
not that many) then integrating PMD with a set of sensible rules may
help detecting bad smells in the future (I couldn't resist -- it really
is called like this in software engineering :). I only used dead code
detection ruleset for now, other rulesets can be checked and we will see
if they help or quite the contrary.
If developers agree to the above I'll create a patch together with what
needs to be fixed to cleanly compile. Otherwise I see little sense in
integrating PMD.
D.
