Monday, September 22, 2008

PMD rule to check for unguarded log.debug() and install in Eclipse

Lately I've written a PMD rule to check that calls to log.debug() are guarded by some if (log.isDebugEnabled()) guard.

Adding it to the PMD commandline is relatively easy. Put the class in a jar and put it on the classpath:

snert:/devel/tools/pmd-4.2.3 hrupp$ java -cp \

  lib/pmd-4.2.3.jar:lib/jaxen-1.1.1.jar:lib/junit-4.4.jar:\

  lib/asm-3.1.jar:lib/my.jar \

  net.sourceforge.pmd.PMD /source/to/check text \

  ~/Documents/workspace/PMDRule/ruleset.xml


my.jar contains the classes and ruleset.xml the description for this ruleset

Now with Eclipse, things are a bit more complicated. There is a GUI to basically write
the ruleset.xml in the preferences, but how to add the rule class? I am sure, this can be
done via a Feature Extension, but I didn't find a description quickly.

So here is what I did:


  1. Shut down eclipse

  2. Locate the PMD core plugin in eclipse/plugins (e.g. net.sourceforge.pmd.core_4.2.1.v200804111600)

  3. Copy the rules jar (my.jar from above) to lib/

  4. Edit plugin.xml and add an entry for the jar:
          <library name="lib/commons-logging-1.1.1.jar">
    <export name="*"/>
    </library>
    <library name="lib/my.jar">
    <export name="*"/>
    </library>

    </runtime>


  5. Start eclipse with option -clean


  6. Now go to the Preferences to the PMD->Rules Configuration section, click 'Add rule...' and add an entry for our new rule:
    pmd_rule.png


  7. Start chasing bogus log.debug() usages :-)

This rule is not yet perfect, as it lacks proper type checking for the 'log' variable. It will probably also fail for Statements like "if (cond1 && log.isDebugEnabled())".

You can find the code here.

Please let me know when you have solved the type checking or how to better integrate it with Eclipse.

3 comments:

Winjo said...

Hi
In eclipse, PMD Custom rule also should follow eclipse plugin convention it seems.
Check following article.
http://www.eclipsezone.com/articles/pmd/

-Alwin

Tammo said...

Hi Heiko, small world... I recently needed the same thing and came up with the following XPath rule. It works only for .debug but also checks whether the expression to log is just a string or composed of a string concatenation. Downside of that is that it does not detect if call a single but expensive operation to get the message to be logged. Anyways, I'd like to share it with you, perhaps it will be helpful for others as well.

XPath:
//PrimaryPrefix[ends-with(Name/@Image, '.debug') and count(../descendant::AdditiveExpression) > 0 and count(ancestor::IfStatement/descendant::PrimaryExpression[ends-with(descendant::PrimaryPrefix/Name/@Image, 'isDebugEnabled')]) = 0]

Sample:
public class Test {
private static final Log __log = LogFactory.getLog(Test.class);
public void test() {

// okay:
__log.debug("log something");

// okay:
__log.debug("log something with exception", e);

// bad:
__log.debug("log something" + " and " + "concat strings");

// bad:
__log.debug("log something" + " and " + "concat strings", e);

// good:
if (__log.isDebugEnabled()) {
__log.debug("bla" + "",e );
}
}

Of course there is still a lot of potential for improvements :)

Tammo

Unknown said...

Hi ,
Can u share me the pmd rule u wrote,it will be a great help to me.

thanks .