phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] Remove old rules GuardDebugLogging and GuardLogStatementJavaUtil

Browse Source
This commit is contained in:
Andreas Dangel
2017-11-30 22:43:35 +01:00
parent 1447ac8a4a
commit 5e3b4504d8
9 changed files with 12 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardDebugLoggingRule.java
View File

@ -1,21 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.HashMap;
public class GuardDebugLoggingRule extends GuardLogStatementRule {
public GuardDebugLoggingRule() {
super.guardStmtByLogLevel = new HashMap<>(1);
super.guardStmtByLogLevel.put(".debug", "isDebugEnabled");
}
@Override
protected void extractProperties() {
// This rule is not configurable
}
}

81
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementJavaUtilRule.java
View File

@ -1,81 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.List;
import java.util.logging.Level;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
public class GuardLogStatementJavaUtilRule extends GuardLogStatementRule {
private static final String GUARD_METHOD_NAME = "isLoggable";
private static String extendedXPath = "//PrimaryPrefix[ends-with(Name/@Image, '.log')]\n"
+ "[following-sibling::PrimarySuffix\n"
+ " [ends-with(.//PrimaryPrefix/Name/@Image, 'LOG_LEVEL_UPPERCASE')]\n"
+ " [count(../descendant::AdditiveExpression) > 0]\n" + "]\n"
+ "[count(ancestor::IfStatement/Expression/descendant::PrimaryExpression\n"
+ " [ends-with(descendant::PrimaryPrefix[1]/Name/@Image,'GUARD')]) = 0\n" + "or\n"
+ "count(ancestor::IfStatement/Expression/descendant::PrimaryExpression\n"
+ " [ends-with(descendant::PrimaryPrefix[2]/Name/@Image,'LOG_LEVEL_UPPERCASE')]) = 0]";
@Override
public Object visit(ASTCompilationUnit unit, Object data) {
if (isSlf4jOrLog4jImported(unit)) {
return data;
}
String[] logLevels = getProperty(LOG_LEVELS).toArray(new String[0]); // TODO:cf convert to list
String[] guardMethods = getProperty(GUARD_METHODS).toArray(new String[0]);
if (super.guardStmtByLogLevel.isEmpty() && logLevels.length > 0 && guardMethods.length > 0) {
configureGuards(logLevels, guardMethods);
} else if (super.guardStmtByLogLevel.isEmpty()) {
configureDefaultGuards();
}
findViolationForEachLogStatement(unit, data, extendedXPath);
return super.visit(unit, data);
}
private boolean isSlf4jOrLog4jImported(ASTCompilationUnit unit) {
List<ASTImportDeclaration> imports = unit.findChildrenOfType(ASTImportDeclaration.class);
for (ASTImportDeclaration i : imports) {
if (i.getImportedName().startsWith("org.slf4j") || i.getImportedName().startsWith("org.apache.log4j")) {
return true;
}
}
return false;
}
private void configureGuards(String[] logLevels, String[] guardMethods) {
String[] methods = guardMethods;
if (methods.length != logLevels.length) {
String firstMethodName = guardMethods[0];
methods = new String[logLevels.length];
for (int i = 0; i < logLevels.length; i++) {
methods[i] = firstMethodName;
}
}
for (int i = 0; i < logLevels.length; i++) {
super.guardStmtByLogLevel.put("." + logLevels[i], methods[i]);
}
}
private void configureDefaultGuards() {
super.guardStmtByLogLevel.put(formatLogLevelString(Level.FINEST), GUARD_METHOD_NAME);
super.guardStmtByLogLevel.put(formatLogLevelString(Level.FINER), GUARD_METHOD_NAME);
super.guardStmtByLogLevel.put(formatLogLevelString(Level.FINE), GUARD_METHOD_NAME);
super.guardStmtByLogLevel.put(formatLogLevelString(Level.INFO), GUARD_METHOD_NAME);
super.guardStmtByLogLevel.put(formatLogLevelString(Level.WARNING), GUARD_METHOD_NAME);
super.guardStmtByLogLevel.put(formatLogLevelString(Level.SEVERE), GUARD_METHOD_NAME);
}
private String formatLogLevelString(Level logLevel) {
return "." + logLevel.toString().toLowerCase();
}
}

16
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java
View File

@ -50,16 +50,16 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
* warning -> isLoggable
* severe -> isLoggable
*/
public static final StringMultiProperty LOG_LEVELS = new StringMultiProperty("logLevels", "LogLevels to guard",
private static final StringMultiProperty LOG_LEVELS = new StringMultiProperty("logLevels", "LogLevels to guard",
new String[] {"trace", "debug", "info", "warn", "error",
"log", "finest", "finer", "fine", "info", "warning", "severe"}, 1.0f, ',');
"log", "finest", "finer", "fine", "info", "warning", "severe", }, 1.0f, ',');
public static final StringMultiProperty GUARD_METHODS = new StringMultiProperty("guardsMethods",
private static final StringMultiProperty GUARD_METHODS = new StringMultiProperty("guardsMethods",
"method use to guard the log statement",
new String[] {"isTraceEnabled", "isDebugEnabled", "isInfoEnabled", "isWarnEnabled", "isErrorEnabled",
"isLoggable"}, 2.0f, ',');
"isLoggable", }, 2.0f, ',');
protected Map<String, String> guardStmtByLogLevel = new HashMap<>(5);
private Map<String, String> guardStmtByLogLevel = new HashMap<>(12);
private static final String XPATH_EXPRESSION =
// first part deals with log4j / apache commons logging
@ -98,7 +98,7 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
}
protected void findViolationForEachLogStatement(ASTCompilationUnit unit, Object data, String xpathExpression) {
private void findViolationForEachLogStatement(ASTCompilationUnit unit, Object data, String xpathExpression) {
for (Entry<String, String> entry : guardStmtByLogLevel.entrySet()) {
List<Node> nodes = findViolations(unit, entry.getKey(), entry.getValue(), xpathExpression);
for (Node node : nodes) {
@ -120,7 +120,7 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
return Collections.EMPTY_LIST;
}
protected void extractProperties() {
private void extractProperties() {
if (guardStmtByLogLevel.isEmpty()) {
List<String> logLevels = new ArrayList<>(super.getProperty(LOG_LEVELS));
@ -145,7 +145,7 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
}
}
protected void buildGuardStatementMap(List<String> logLevels, List<String> guardMethods) {
private void buildGuardStatementMap(List<String> logLevels, List<String> guardMethods) {
for (int i = 0; i < logLevels.size(); i++) {
String logLevel = "." + logLevels.get(i);
if (guardStmtByLogLevel.containsKey(logLevel)) {

60
pmd-java/src/main/resources/category/java/bestpractices.xml
View File

@ -383,44 +383,6 @@ public class MyClass {
</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.java.rule.bestpractices.GuardDebugLoggingRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.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>
<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>
<rule name="GuardLogStatement"
language="java"
since="5.1.0"
@ -441,28 +403,6 @@ otherwise skip the associate String creation and manipulation.
</example>
</rule>
<rule name="GuardLogStatementJavaUtil"
language="java"
since="5.1.0"
message="There is log block not surrounded by if"
class="net.sourceforge.pmd.lang.java.rule.bestpractices.GuardLogStatementJavaUtilRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#guardlogstatementjavautil">
<description>
Whenever using a log level, one should check if the loglevel is actually enabled, or
otherwise skip the associate String creation and manipulation.
</description>
<priority>2</priority>
<example>
<![CDATA[
//...
// Add this for performance
if (log.isLoggable(Level.FINE)) {
log.fine("log something" + " and " + "concat strings");
}
]]>
</example>
</rule>
<rule name="JUnit4SuitesShouldUseSuiteAnnotation"
language="java"
since="4.0"

3
pmd-java/src/main/resources/rulesets/java/logging-jakarta-commons.xml
View File

@ -12,7 +12,8 @@ The Jakarta Commons Logging ruleset contains a collection of rules that find que
<rule ref="category/java/errorprone.xml/ProperLogger" deprecated="true" />
<rule ref="category/java/errorprone.xml/UseCorrectExceptionLogging" deprecated="true" />
<rule ref="category/java/bestpractices.xml/GuardDebugLogging" deprecated="true" />
<!-- GuardDebugLogging has been merged into GuardLogStatement -->
<rule ref="category/java/bestpractices.xml/GuardLogStatement" name="GuardDebugLogging" deprecated="true" />
<rule ref="category/java/bestpractices.xml/GuardLogStatement" deprecated="true" />
</ruleset>

3
pmd-java/src/main/resources/rulesets/java/logging-java.xml
View File

@ -15,7 +15,8 @@ The Java Logging ruleset contains a collection of rules that find questionable u
<rule ref="category/java/errorprone.xml/MoreThanOneLogger" deprecated="true" />
<rule ref="category/java/bestpractices.xml/AvoidPrintStackTrace" deprecated="true" />
<rule ref="category/java/bestpractices.xml/GuardLogStatementJavaUtil" deprecated="true" />
<!-- GuardLogStatementJavaUtil has been merged into GuardLogStatement -->
<rule ref="category/java/bestpractices.xml/GuardLogStatement" name="GuardLogStatementJavaUtil" deprecated="true" />
<rule ref="category/java/bestpractices.xml/SystemPrintln" deprecated="true" />
</ruleset>

2
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/BestPracticesRulesTest.java
View File

@ -28,9 +28,7 @@ public class BestPracticesRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "ConstantsInInterface");
addRule(RULESET, "DefaultLabelNotLastInSwitchStmt");
addRule(RULESET, "ForLoopCanBeForeach");
addRule(RULESET, "GuardDebugLogging");
addRule(RULESET, "GuardLogStatement");
addRule(RULESET, "GuardLogStatementJavaUtil");
addRule(RULESET, "JUnit4SuitesShouldUseSuiteAnnotation");
addRule(RULESET, "JUnit4TestShouldUseAfterAnnotation");
addRule(RULESET, "JUnit4TestShouldUseBeforeAnnotation");

6
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml
View File

@ -1,6 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
</test-data>

22
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml
View File

@ -1,22 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>#1335 GuardLogStatementJavaUtil should not apply to SLF4J Logger</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class GuardLogTest {
Logger LOGGER = LoggerFactory.getLogger(getClass());
public void foo() {
LOGGER.info("foo" + "bar");
}
}
]]></code>
</test-code>
</test-data>