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:

  1. Winjo9:51 am

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

    -Alwin

    ReplyDelete
  2. Tammo8:02 pm

    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

    ReplyDelete
  3. Unknown12:25 pm

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

    thanks .

    ReplyDelete