From 165bb3a24c9285933e08f6b66a580d56508c3686 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <adangel@users.sourceforge.net>
Date: Mon, 26 Sep 2011 19:40:51 +0000
Subject: [PATCH] Applied patch 3130615: New Rule GuardDebugLogging for
 jakarta-commons logging. Thanks to Tammo van Lessen.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@7355 51baf565-9d33-0410-a72c-fc3788e3496d
---
 pmd/etc/changelog.txt                         |  1 +
 .../LoggingJakartaCommonsRulesTest.java       |  1 +
 .../xml/GuardDebugLogging.xml                 | 55 +++++++++++++++++++
 pmd/rulesets/java/logging-jakarta-commons.xml | 48 +++++++++++++++-
 pmd/rulesets/releases/50.xml                  |  1 +
 pmd/xdocs/credits.xml                         |  1 +
 6 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/xml/GuardDebugLogging.xml

diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt
index dbde8e0618..f61639ce39 100644
--- a/pmd/etc/changelog.txt
+++ b/pmd/etc/changelog.txt
@@ -459,6 +459,7 @@ New Java rules:
 	StrictException ruleset: AvoidThrowingNewInstanceOfSameException, AvoidCatchingGenericException, AvoidLosingExceptionInformation
 	Unnecessary ruleset: UselessParentheses
 	JUnit ruleset: JUnitTestContainsTooManyAsserts, UseAssertTrueInsteadOfAssertEquals
+	Logging with Jakarta Commons ruleset: GuardDebugLogging
 
 New Java ruleset:
 	android.xml: new rules specific to the Android platform
diff --git a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/LoggingJakartaCommonsRulesTest.java b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/LoggingJakartaCommonsRulesTest.java
index a13602698e..d07ec82f16 100644
--- a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/LoggingJakartaCommonsRulesTest.java
+++ b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/LoggingJakartaCommonsRulesTest.java
@@ -12,6 +12,7 @@ public class LoggingJakartaCommonsRulesTest extends SimpleAggregatorTst {
     public void setUp() {
         addRule(RULESET, "ProperLogger");
         addRule(RULESET, "UseCorrectExceptionLogging");
+        addRule(RULESET, "GuardDebugLogging");
     }
 
     public static junit.framework.Test suite() {
diff --git a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/xml/GuardDebugLogging.xml b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/xml/GuardDebugLogging.xml
new file mode 100644
index 0000000000..97c8b37386
--- /dev/null
+++ b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/loggingjakartacommons/xml/GuardDebugLogging.xml
@@ -0,0 +1,55 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data>
+    <test-code>
+        <description><![CDATA[
+ok
+     ]]></description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+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);
+        
+        // good:
+        if (__log.isDebugEnabled()) {
+        __log.debug("bla" + "",e );
+        }
+    }
+}
+     ]]></code>
+    </test-code>
+    <test-code>
+        <description><![CDATA[
+Complex logging without guard
+     ]]></description>
+        <expected-problems>2</expected-problems>
+        <code><![CDATA[
+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 );
+        }
+    }
+}
+     ]]></code>
+    </test-code>
+</test-data>
diff --git a/pmd/rulesets/java/logging-jakarta-commons.xml b/pmd/rulesets/java/logging-jakarta-commons.xml
index fdaa42706b..864b93a43a 100644
--- a/pmd/rulesets/java/logging-jakarta-commons.xml
+++ b/pmd/rulesets/java/logging-jakarta-commons.xml
@@ -88,5 +88,51 @@ public class Foo {
  ]]>
             </example>
      </rule>
-
+     
+    <rule name="GuardDebugLogging"
+          language="Java"
+          since="4.3"
+          message="debug logging that involves string concatenation should be guarded with isDebugEnabled() checks"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          externalInfoUrl="http://pmd.sourceforge.net/rules/java/logging-jakarta-commons.html#GuardDebugLogging">
+        <description>
+            When log messages are composed by concatenating strings, the whole section should be guarded
+            by a isDebugEnabled() check to avoid performance and memory issues.
+        </description>
+        <priority>3</priority>
+        <properties>
+            <property name="xpath">
+            <value>
+            <![CDATA[
+                //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]
+            ]]>
+            </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+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 );
+        }
+    }
+}
+            ]]>
+        </example>
+    </rule>
 </ruleset>
\ No newline at end of file
diff --git a/pmd/rulesets/releases/50.xml b/pmd/rulesets/releases/50.xml
index f4a61ea024..ba9daf5fc8 100644
--- a/pmd/rulesets/releases/50.xml
+++ b/pmd/rulesets/releases/50.xml
@@ -40,6 +40,7 @@ This ruleset contains links to rules that are new in PMD v5.0
 	<rule ref="rulesets/java/imports.xml/UnnecessaryFullyQualifiedName"/>
 	<rule ref="rulesets/java/junit.xml/JUnitTestContainsTooManyAsserts"/>
 	<rule ref="rulesets/java/junit.xml/UseAssertTrueInsteadOfAssertEquals"/>
+    <rule ref="rulesets/java/logging-jakarta-commons.xml/GuardDebugLogging"/>
 	<rule ref="rulesets/java/naming.xml/ShortClassName"/>
 	<rule ref="rulesets/java/optimizations.xml/RedundantFieldInitializer"/>
 	<rule ref="rulesets/java/strictexception.xml/AvoidCatchingGenericException"/>
diff --git a/pmd/xdocs/credits.xml b/pmd/xdocs/credits.xml
index 321c47031c..e3de961fa4 100644
--- a/pmd/xdocs/credits.xml
+++ b/pmd/xdocs/credits.xml
@@ -321,6 +321,7 @@
             <li>Lucian Ciufudean - RedundantFieldInitializerRule</li>
             <li>Andreas Dangel - GodClass and LawOfDemeter rules, several bugfixes and cleanup</li>
             <li>Riku Nykanen - patch improving TooManyMethods rule</li>
+            <li>Tammo van Lessen - new rule GuardDebugLogging for Jakarta Commons Logging ruleset.</li>
         </ul>
         </subsection>
         <subsection name="Organizations">