diff --git a/pmd/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index d634828fbc..a724a192da 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -321,9 +321,9 @@ public class RuleSetFactory { } String attribute = ruleElement.getAttribute("class"); - Class c = classLoader.loadClass(attribute); - Rule rule = (Rule) c.newInstance(); - + if ( attribute == null || "".equals(attribute)) + throw new IllegalArgumentException("The 'class' field of rule can't be null, nor empty."); + Rule rule = (Rule) classLoader.loadClass(attribute).newInstance(); rule.setName(ruleElement.getAttribute("name")); if (ruleElement.hasAttribute("language")) { diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/GuardLogStatementRule.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/GuardLogStatementRule.java new file mode 100644 index 0000000000..d276f8dd76 --- /dev/null +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/GuardLogStatementRule.java @@ -0,0 +1,88 @@ +package net.sourceforge.pmd.lang.java.rule.logging; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import net.sourceforge.pmd.Rule; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; +import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.rule.optimizations.AbstractOptimizationRule; + + +/** + * Check that log.debug and log.trace statements are guarded by some + * log.isDebugEnabled() or log.isTraceEnabled() checks. + * + * @author Heiko hwr@pilhuhn.de + * @author Romain Pelisse - + * + */ +public class GuardLogStatementRule extends AbstractOptimizationRule implements Rule { + + private static final Map guardStmtByLogLevel = new HashMap(5); + + public GuardLogStatementRule() { + guardStmtByLogLevel.put(".trace","isTraceEnabled"); + guardStmtByLogLevel.put(".debug","isDebugEnabled"); + guardStmtByLogLevel.put(".warn", "isWarnEnabled"); + guardStmtByLogLevel.put(".error", "isErrorEnabled"); + guardStmtByLogLevel.put(".info","isInfoEnabled"); + } + + private String lastPrefix(String string) { + return (string != null && ! "".equals(string)) ? string.substring(string.lastIndexOf('.'), string.length()) : string; + } + + public Object visit(ASTName name, Object data) { + Node node = name.jjtGetParent(); + if ( node instanceof ASTPrimaryPrefix ) { + + } else + return super.visit(name,data); + if (name != null) { + String lastPrefix = lastPrefix(name.getImage()); + if ( guardStmtByLogLevel.keySet().contains(lastPrefix)) { + // TODO check for type + Node parent1 = name.getNthParent(5); + boolean guardFound = false; + if (parent1 instanceof ASTIfStatement) { + guardFound = checkForGuard((ASTIfStatement) parent1, lastPrefix); + } + else if (parent1 instanceof ASTBlockStatement) { + Node parent2 = name.getNthParent(7); + if (parent2 instanceof ASTIfStatement) { + guardFound = checkForGuard((ASTIfStatement) parent2, lastPrefix); + } + } + if (!guardFound) + addViolation(data, name); + } + } + return super.visit(name, data); + } + + /** + * We stand on an if() check if it contains log.isDebugEnabled() + * @param stm + * @param isTrace true if log.trace() is used + * @return true if guard was found + */ + + private boolean checkForGuard(ASTIfStatement stm, String logLevel) { + + List names = stm.findDescendantsOfType(ASTName.class); + if (names == null || names.isEmpty() ) + return false; + + for ( ASTName name : names) { + String image = name.getImage(); + if ( guardStmtByLogLevel.get(logLevel).equals(name.getImage()) ); + return true; + } + return false; + } +} diff --git a/pmd/src/main/resources/rulesets/java/logging-java.xml b/pmd/src/main/resources/rulesets/java/logging-java.xml index c388da7b7d..ffdbbe2516 100644 --- a/pmd/src/main/resources/rulesets/java/logging-java.xml +++ b/pmd/src/main/resources/rulesets/java/logging-java.xml @@ -63,43 +63,27 @@ public class Foo{ } ]]> - - - - - + - - \ No newline at end of file + + diff --git a/pmd/src/test/java/net/sourceforge/pmd/lang/java/rule/loggingjava/LoggingJavaRulesTest.java b/pmd/src/test/java/net/sourceforge/pmd/lang/java/rule/loggingjava/LoggingJavaRulesTest.java index c77fefa6b0..d08b8ac36c 100644 --- a/pmd/src/test/java/net/sourceforge/pmd/lang/java/rule/loggingjava/LoggingJavaRulesTest.java +++ b/pmd/src/test/java/net/sourceforge/pmd/lang/java/rule/loggingjava/LoggingJavaRulesTest.java @@ -15,6 +15,7 @@ public class LoggingJavaRulesTest extends SimpleAggregatorTst { addRule(RULESET, "LoggerIsNotStaticFinal"); addRule(RULESET, "MoreThanOneLogger"); addRule(RULESET, "SystemPrintln"); + addRule(RULESET, "GuardLogStatement"); } public static junit.framework.Test suite() { diff --git a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/GuardLogStatement.xml b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/GuardLogStatement.xml new file mode 100644 index 0000000000..ddddff2a8f --- /dev/null +++ b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/GuardLogStatement.xml @@ -0,0 +1,34 @@ + + + + + 0 + + + + + 1 + + +