#1224 GuardDebugLogging broken in 5.1.1 - missing additive statement check in log statement

[gordz]

The example provided in the documentation for commons logging "GuardDebugLogging" fails validation on all log statements (included those documented as being valid) when checked with version 5.1.1, but passes with 4.3.

It appears since 4.3, the GuardDebugLogging rule was changed from a XPath implementation to Java. Doing a quick scan of the Java code, it looks like the code fails to check the log statement for string concatenation (previously done with a AdditiveExpression in the old XPath based rule).

http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-jakarta-commons.html#GuardDebugLogging

Below is the code and output from my analysis:

graham@graham-desktop:/opt/pmd-bin-5.1.1/bin$ ./run.sh pmd -d ./Test.java -f text -R /tmp/rules.xml -version 1.6 -language java
/opt/pmd-bin-5.1.1/bin/Test.java:2: Logger should be defined private static final and have the correct class
/opt/pmd-bin-5.1.1/bin/Test.java:5: There is log block not surrounded by if
/opt/pmd-bin-5.1.1/bin/Test.java:5: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks
/opt/pmd-bin-5.1.1/bin/Test.java:8: There is log block not surrounded by if
/opt/pmd-bin-5.1.1/bin/Test.java:8: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks
/opt/pmd-bin-5.1.1/bin/Test.java:11:    There is log block not surrounded by if
/opt/pmd-bin-5.1.1/bin/Test.java:11:    debug logging that involves string concatenation should be guarded with isDebugEnabled() checks
/opt/pmd-bin-5.1.1/bin/Test.java:14:    There is log block not surrounded by if
/opt/pmd-bin-5.1.1/bin/Test.java:14:    debug logging that involves string concatenation should be guarded with isDebugEnabled() checks

Contents of Test.java:

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 );
        }
    }
}

The problem class in 5.1.1 is net.sourceforge.pmd.lang.java.rule.logging.GuardDebugLoggingRule.

[gordz]

As an aside, this was discovered after upgrading to the latest SonarQube + PMD plugin, when our BLOCKERs jumped up by several thousand and caused quality gates to fail.

[gordz]

Ruleset:

<?xml version="1.0"?>
<ruleset name="Custom ruleset"
    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0
http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
  <description>
  This ruleset checks my code for bad stuff
  </description>
        <rule ref="rulesets/java/logging-jakarta-commons.xml" />

</ruleset>

On Tue, Jul 15, 2014 at 11:06 AM, gordz gordz@users.sf.net wrote:

As an aside, this was discovered after upgrading to the latest SonarQube +
PMD plugin, when our BLOCKERs jumped up by several thousand and caused
quality gates to fail.

---

• [bugs:#1224] http://sourceforge.net/p/pmd/bugs/1224 GuardDebugLogging
  broken in 5.1.1 - missing additive statement check in log statement*

Status: open
Milestone: New Tickets
Created: Tue Jul 15, 2014 01:02 AM UTC by gordz
Last Updated: Tue Jul 15, 2014 01:02 AM UTC
Owner: nobody

The example provided in the documentation for commons logging
"GuardDebugLogging" fails validation on all log statements (included those
documented as being valid) when checked with version 5.1.1, but passes with
4.3.

It appears since 4.3, the GuardDebugLogging rule was changed from a XPath
implementation to Java. Doing a quick scan of the Java code, it looks like
the code fails to check the log statement for string concatenation
(previously done with a AdditiveExpression in the old XPath based rule).

http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-jakarta-commons.html#GuardDebugLogging

Below is the code and output from my analysis:

graham@graham-desktop:/opt/pmd-bin-5.1.1/bin$ ./run.sh pmd -d ./Test.java -f text -R /tmp/rules.xml -version 1.6 -language java/opt/pmd-bin-5.1.1/bin/Test.java:2: Logger should be defined private static final and have the correct class/opt/pmd-bin-5.1.1/bin/Test.java:5: There is log block not surrounded by if/opt/pmd-bin-5.1.1/bin/Test.java:5: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks/opt/pmd-bin-5.1.1/bin/Test.java:8: There is log block not surrounded by if/opt/pmd-bin-5.1.1/bin/Test.java:8: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks/opt/pmd-bin-5.1.1/bin/Test.java:11: There is log block not surrounded by if/opt/pmd-bin-5.1.1/bin/Test.java:11: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks/opt/pmd-bin-5.1.1/bin/Test.java:14: There is log block not surrounded by if/opt/pmd-bin-5.1.1/bin/Test.java:14: debug logging that involves string concatenation should be guarded with isDebugEnabled() checks

Contents of Test.java:

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 );
    }
}}

The problem class in 5.1.1 is
net.sourceforge.pmd.lang.java.rule.logging.GuardDebugLoggingRule.

---

Sent from sourceforge.net because you indicated interest in
https://sourceforge.net/p/pmd/bugs/1224/

To unsubscribe from further messages, please visit
https://sourceforge.net/auth/subscriptions/

[Andreas Dangel]

Thanks for reporting!
This will be fixed with the next release (5.1.2).