From 1447ac8a4a2896a85e809054d87ffebd31589eb1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 30 Nov 2017 22:38:14 +0100 Subject: [PATCH 1/4] [java] Merge GuardDebugLogging and GuardLogStatementJavaUtil into the rule GuardLogStatement --- .../bestpractices/GuardLogStatementRule.java | 107 +++++---- .../bestpractices/xml/GuardDebugLogging.xml | 134 ----------- .../bestpractices/xml/GuardLogStatement.xml | 208 +++++++++++++++++- .../xml/GuardLogStatementJavaUtil.xml | 100 --------- 4 files changed, 263 insertions(+), 286 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java index 10cdc04d20..a4e595c8a2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java @@ -30,19 +30,52 @@ import net.sourceforge.pmd.properties.StringMultiProperty; * */ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { - + /* + * guard methods and log levels: + * + * log4j + apache commons logging (jakarta): + * trace -> isTraceEnabled + * debug -> isDebugEnabled + * info -> isInfoEnabled + * warn -> isWarnEnabled + * error -> isErrorEnabled + * + * + * java util: + * log(Level.FINE) -> isLoggable + * finest -> isLoggable + * finer -> isLoggable + * fine -> isLoggable + * info -> isLoggable + * warning -> isLoggable + * severe -> isLoggable + */ public static final StringMultiProperty LOG_LEVELS = new StringMultiProperty("logLevels", "LogLevels to guard", - new String[] {}, 1.0f, ','); + new String[] {"trace", "debug", "info", "warn", "error", + "log", "finest", "finer", "fine", "info", "warning", "severe"}, 1.0f, ','); public static final StringMultiProperty GUARD_METHODS = new StringMultiProperty("guardsMethods", - "method use to guard the log statement", new String[] {}, 2.0f, ','); + "method use to guard the log statement", + new String[] {"isTraceEnabled", "isDebugEnabled", "isInfoEnabled", "isWarnEnabled", "isErrorEnabled", + "isLoggable"}, 2.0f, ','); protected Map guardStmtByLogLevel = new HashMap<>(5); - private static final String XPATH_EXPRESSION = "//PrimaryPrefix[ends-with(Name/@Image, 'LOG_LEVEL')]" - + "[count(../descendant::AdditiveExpression) > 0]" - + "[count(ancestor::IfStatement/Expression/descendant::PrimaryExpression[" - + "ends-with(descendant::PrimaryPrefix/Name/@Image,'GUARD')]) = 0]"; + private static final String XPATH_EXPRESSION = + // first part deals with log4j / apache commons logging + "//StatementExpression/PrimaryExpression/PrimaryPrefix[ends-with(Name/@Image, 'LOG_LEVEL')]\n" + + "[..//AdditiveExpression]\n" + + "[not(ancestor::IfStatement) or\n" + + " not(ancestor::IfStatement/Expression/PrimaryExpression/PrimaryPrefix/Name[contains('GUARD', substring-after(@Image, '.'))])]\n" + + "|\n" + // this part deals with java util + + "//StatementExpression/PrimaryExpression/PrimaryPrefix[ends-with(Name/@Image, 'LOG_LEVEL_UPPERCASE')]\n" + + "[../../..//AdditiveExpression]\n" + + "[not(ancestor::IfStatement) or\n" + + " not(ancestor::IfStatement/Expression/PrimaryExpression\n" + + " [contains('GUARD', substring-after(PrimaryPrefix/Name/@Image, '.'))]\n" + + " [ends-with(PrimarySuffix//Name/@Image, 'LOG_LEVEL_UPPERCASE')])\n" + + "]"; public GuardLogStatementRule() { definePropertyDescriptor(LOG_LEVELS); @@ -67,7 +100,7 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { protected void findViolationForEachLogStatement(ASTCompilationUnit unit, Object data, String xpathExpression) { for (Entry entry : guardStmtByLogLevel.entrySet()) { - List nodes = findViolations(unit, entry.getKey(), entry.getValue(), xpathExpression); + List nodes = findViolations(unit, entry.getKey(), entry.getValue(), xpathExpression); for (Node node : nodes) { super.addViolation(data, node); } @@ -75,33 +108,18 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { } @SuppressWarnings("unchecked") - private List findViolations(ASTCompilationUnit unit, String logLevel, String guard, + private List findViolations(ASTCompilationUnit unit, String logLevel, String guard, String xpathExpression) { try { - return unit - .findChildNodesWithXPath(xpathExpression.replaceAll("LOG_LEVEL_UPPERCASE", logLevel.toUpperCase()) - .replaceAll("LOG_LEVEL", logLevel).replaceAll("GUARD", guard)); + String xpath = xpathExpression.replaceAll("LOG_LEVEL_UPPERCASE", logLevel.toUpperCase()) + .replaceAll("LOG_LEVEL", logLevel).replaceAll("GUARD", guard); + return unit.findChildNodesWithXPath(xpath); } catch (JaxenException e) { e.printStackTrace(); } return Collections.EMPTY_LIST; } - private void setPropertiesDefaultValues(List logLevels, List guardMethods) { - logLevels.add("trace"); - logLevels.add("debug"); - logLevels.add("info"); - logLevels.add("warn"); - logLevels.add("error"); - - guardMethods.clear(); - guardMethods.add("isTraceEnabled"); - guardMethods.add("isDebugEnabled"); - guardMethods.add("isInfoEnabled"); - guardMethods.add("isWarnEnabled"); - guardMethods.add("isErrorEnabled"); - } - protected void extractProperties() { if (guardStmtByLogLevel.isEmpty()) { @@ -109,11 +127,18 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { List guardMethods = new ArrayList<>(super.getProperty(GUARD_METHODS)); if (guardMethods.isEmpty() && !logLevels.isEmpty()) { - throw new IllegalArgumentException("Can't specify guardMethods without specifiying logLevels."); + throw new IllegalArgumentException("Can't specify logLevels without specifying guardMethods."); } - - if (logLevels.isEmpty()) { - setPropertiesDefaultValues(logLevels, guardMethods); + if (logLevels.size() > guardMethods.size()) { + // reuse the last guardMethod for the remaining log levels + int needed = logLevels.size() - guardMethods.size(); + String lastGuard = guardMethods.get(guardMethods.size() - 1); + for (int i = 0; i < needed; i++) { + guardMethods.add(lastGuard); + } + } + if (logLevels.size() != guardMethods.size()) { + throw new IllegalArgumentException("For each logLevel a guardMethod must be specified."); } buildGuardStatementMap(logLevels, guardMethods); @@ -121,18 +146,14 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { } protected void buildGuardStatementMap(List logLevels, List guardMethods) { - for (String logLevel : logLevels) { - boolean found = false; - for (String guardMethod : guardMethods) { - if (!found && guardMethod.toLowerCase().contains(logLevel.toLowerCase())) { - found = true; - guardStmtByLogLevel.put("." + logLevel, guardMethod); - } - } - - if (!found) { - throw new IllegalArgumentException("No guard method associated to the logLevel:" + logLevel - + ". Should be something like 'is" + logLevel + "Enabled'."); + for (int i = 0; i < logLevels.size(); i++) { + String logLevel = "." + logLevels.get(i); + if (guardStmtByLogLevel.containsKey(logLevel)) { + String combinedGuard = guardStmtByLogLevel.get(logLevel); + combinedGuard += "|" + guardMethods.get(i); + guardStmtByLogLevel.put(logLevel, combinedGuard); + } else { + guardStmtByLogLevel.put(logLevel, guardMethods.get(i)); } } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml index 46c089fd8e..734a2e9f04 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml @@ -3,138 +3,4 @@ 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"> - - - 0 - - - - - 2 - - - - - 1 - - - - ok #1189 GuardLogStatementRule and GuardDebugLoggingRule broken for log4j2 - 0 - - - - violation - wrong guard #1189 GuardLogStatementRule and GuardDebugLoggingRule broken for log4j2 - 1 - - - - violation - no if #1189 GuardLogStatementRule and GuardDebugLoggingRule broken for log4j2 - 1 - - - - #1224 GuardDebugLogging broken in 5.1.1 - missing additive statement check in log statement - 0 - - - - #1341 pmd:GuardDebugLogging violates LOGGER.debug with format "{}" - 0 - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml index 2c43ef76ad..0be29f8096 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml @@ -4,9 +4,7 @@ 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"> - + OK, guard is here - log4j 0 - + ok, no error expected - apache commons logging + 0 + + + + Guarded call - OK - java util + 0 + + + + KO, missing guard 1 - log4j 1 - + KO, missing guard 2 - log4j debug,trace isDebugEnabled,isTraceEnabled 1 @@ -56,6 +79,57 @@ public class Foo { } ]]> + + Complex logging without guard - apache commons logging + 2 + + + + Complex logging with misplaced guard - apache commons logging + 1 + + + + Unguarded call - KO - java util + 1 + + ok #1189 GuardLogStatementRule and GuardDebugLoggingRule broken for log4j2 0 @@ -90,4 +164,120 @@ public class Test { } ]]> + + violation - no if #1189 GuardLogStatementRule and GuardDebugLoggingRule broken for log4j2 + 1 + + + + #1224 GuardDebugLogging broken in 5.1.1 - missing additive statement check in log statement - apache commons logging + 0 + + + + #1341 pmd:GuardDebugLogging violates LOGGER.debug with format "{}" - slf4j + 0 + + + + #1203 GuardLogStatementJavaUtil issues warning for severe level not being specified as property + finest,finer,fine,info + isLoggable + 0 + + + + #1227 GuardLogStatementJavaUtil doesn't catch log(Level.FINE, "msg" + " msg") calls + 1 + 8 + + + + #1347 False positive for GuardLogStatementJavaUtil with slf4j + 0 + + + + #1398 False positive for GuardLogStatementJavaUtil with Log4j + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml index 29235c132c..130c3bfaa7 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml @@ -3,74 +3,6 @@ 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"> - - - 0 - - - - - 1 - - - - #1203 GuardLogStatementJavaUtil issues warning for severe level not being specified as property - finest,finer,fine,info - isLoggable - 0 - - - - #1227 GuardLogStatementJavaUtil doesn't catch log(Level.FINE, "msg" + " msg") calls - 1 - 8 - - #1335 GuardLogStatementJavaUtil should not apply to SLF4J Logger 0 @@ -87,36 +19,4 @@ public class GuardLogTest { } ]]> - - #1347 False positive for GuardLogStatementJavaUtil - 0 - - - - #1398 False positive for GuardLogStatementJavaUtil with Log4j - 0 - - From 5e3b4504d89db26bb272fdeba54d52cf7ecaa3f5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 30 Nov 2017 22:43:35 +0100 Subject: [PATCH 2/4] [java] Remove old rules GuardDebugLogging and GuardLogStatementJavaUtil --- .../bestpractices/GuardDebugLoggingRule.java | 21 ----- .../GuardLogStatementJavaUtilRule.java | 81 ------------------- .../bestpractices/GuardLogStatementRule.java | 16 ++-- .../resources/category/java/bestpractices.xml | 60 -------------- .../rulesets/java/logging-jakarta-commons.xml | 3 +- .../resources/rulesets/java/logging-java.xml | 3 +- .../bestpractices/BestPracticesRulesTest.java | 2 - .../bestpractices/xml/GuardDebugLogging.xml | 6 -- .../xml/GuardLogStatementJavaUtil.xml | 22 ----- 9 files changed, 12 insertions(+), 202 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardDebugLoggingRule.java delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementJavaUtilRule.java delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardDebugLoggingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardDebugLoggingRule.java deleted file mode 100644 index 355e050331..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardDebugLoggingRule.java +++ /dev/null @@ -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 - } - -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementJavaUtilRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementJavaUtilRule.java deleted file mode 100644 index 277bd6be0b..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementJavaUtilRule.java +++ /dev/null @@ -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 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(); - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java index a4e595c8a2..dc47904610 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java @@ -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 guardStmtByLogLevel = new HashMap<>(5); + private Map 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 entry : guardStmtByLogLevel.entrySet()) { List 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 logLevels = new ArrayList<>(super.getProperty(LOG_LEVELS)); @@ -145,7 +145,7 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule { } } - protected void buildGuardStatementMap(List logLevels, List guardMethods) { + private void buildGuardStatementMap(List logLevels, List guardMethods) { for (int i = 0; i < logLevels.size(); i++) { String logLevel = "." + logLevels.get(i); if (guardStmtByLogLevel.containsKey(logLevel)) { diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 086bb8c244..236dd78dcc 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -383,44 +383,6 @@ public class MyClass { - - -When log messages are composed by concatenating strings, the whole section should be guarded -by a isDebugEnabled() check to avoid performance and memory issues. - - 3 - - - - - - - -Whenever using a log level, one should check if the loglevel is actually enabled, or -otherwise skip the associate String creation and manipulation. - - 2 - - - - - - + + diff --git a/pmd-java/src/main/resources/rulesets/java/logging-java.xml b/pmd-java/src/main/resources/rulesets/java/logging-java.xml index 85bfdcf06e..59b3de4c8f 100644 --- a/pmd-java/src/main/resources/rulesets/java/logging-java.xml +++ b/pmd-java/src/main/resources/rulesets/java/logging-java.xml @@ -15,7 +15,8 @@ The Java Logging ruleset contains a collection of rules that find questionable u - + + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/BestPracticesRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/BestPracticesRulesTest.java index 76b4e74ce3..f63196f527 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/BestPracticesRulesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/BestPracticesRulesTest.java @@ -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"); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml deleted file mode 100644 index 734a2e9f04..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardDebugLogging.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml deleted file mode 100644 index 130c3bfaa7..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatementJavaUtil.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - - #1335 GuardLogStatementJavaUtil should not apply to SLF4J Logger - 0 - - - From 0f96c82695cf3f7977e3e3717065b8576fa5e47c Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 30 Nov 2017 22:49:34 +0100 Subject: [PATCH 3/4] Update release notes, closes #457 --- docs/pages/release_notes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index eb1a123a63..f275603edf 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -272,6 +272,10 @@ The rule reference documentation has been updated to reflect these changes. * The Java rule `EmptyStaticInitializer` in ruleset `java-empty` is deprecated. Use the rule `EmptyInitializer` from the category `errorprone`, which covers both static and non-static empty initializers.` +* The Java rules `GuardDebugLogging` (ruleset `java-logging-jakarta-commons`) and `GuardLogStatementJavaUtil` + (ruleset `java-logging-java`) have been deprecated. Use the rule `GuardLogStatement` from the + category `bestpractices`, which covers all cases regardless of the logging framework. + #### Removed Rules * The deprecated Java rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed @@ -363,6 +367,7 @@ a warning will now be produced suggesting users to adopt it for better performan * [#438](https://github.com/pmd/pmd/issues/438): \[java] Relax AbstractClassWithoutAnyMethod when class is annotated by @AutoValue * [#590](https://github.com/pmd/pmd/issues/590): \[java] False positive on MissingStaticMethodInNonInstantiatableClass * java-logging + * [#457](https://github.com/pmd/pmd/issues/457): \[java] Merge all log guarding rules * [#721](https://github.com/pmd/pmd/issues/721): \[java] NPE in PMD 5.8.1 InvalidSlf4jMessageFormat * java-sunsecure * [#468](https://github.com/pmd/pmd/issues/468): \[java] ArrayIsStoredDirectly false positive From e84800b34cc0c241802c9d6a4aa0381b739edf54 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 30 Nov 2017 22:54:07 +0100 Subject: [PATCH 4/4] Update ruleset factory compatibility --- .../java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index f732f86a82..0dee6a59fc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -78,6 +78,10 @@ public class RuleSetFactoryCompatibility { addFilterRuleRenamed("java", "naming", "MisleadingVariableName", "MIsLeadingVariableName"); addFilterRuleRenamed("java", "unnecessary", "UnnecessaryFinalModifier", "UnnecessaryModifier"); addFilterRuleRenamed("java", "empty", "EmptyStaticInitializer", "EmptyInitializer"); + // GuardLogStatementJavaUtil moved and renamed... + addFilterRuleMoved("java", "logging-java", "logging-jakarta-commons", "GuardLogStatementJavaUtil"); + addFilterRuleRenamed("java", "logging-jakarta-commons", "GuardLogStatementJavaUtil", "GuardLogStatement"); + addFilterRuleRenamed("java", "logging-jakarta-commons", "GuardDebugLogging", "GuardLogStatement"); } void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) {