diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 8935245a24..f3c2eed69d 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -1,6 +1,7 @@ ???? - 4.2.3: Fixes for exclude-pattern +Updates to RuleChain to honor RuleSet exclude-pattern May 20, 2008 - 4.2.2: diff --git a/pmd/regress/test/net/sourceforge/pmd/RuleSetTest.java b/pmd/regress/test/net/sourceforge/pmd/RuleSetTest.java index 586a52f9c7..bcbc0b557b 100644 --- a/pmd/regress/test/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/RuleSetTest.java @@ -19,11 +19,14 @@ import java.util.List; import java.util.Set; import net.sourceforge.pmd.MockRule; +import net.sourceforge.pmd.PMD; +import net.sourceforge.pmd.PMDException; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleReference; import net.sourceforge.pmd.RuleSet; +import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.TargetJDKVersion; import net.sourceforge.pmd.ast.ASTCompilationUnit; @@ -31,7 +34,9 @@ import net.sourceforge.pmd.ast.JavaParser; import org.junit.Test; -public class RuleSetTest { +import test.net.sourceforge.pmd.testframework.RuleTst; + +public class RuleSetTest extends RuleTst { private String javaCode = "public class Test { }"; @@ -328,6 +333,50 @@ public class RuleSetTest { assertTrue("Matching include", ruleSet.applies(file)); } + @Test + public void testIncludeExcludeMultipleRuleSetWithRuleChainApplies() throws PMDException { + File file = new File("C:\\myworkspace\\project\\some\\random\\package\\RandomClass.java"); + + RuleSet ruleSet1 = new RuleSet(); + ruleSet1.setName("RuleSet1"); + Rule rule = findRule("basic", "EmptyIfStmt"); + assertTrue("RuleChain rule", rule.usesRuleChain()); + ruleSet1.addRule(rule); + + RuleSet ruleSet2 = new RuleSet(); + ruleSet2.setName("RuleSet2"); + ruleSet2.addRule(rule); + + RuleSets ruleSets = new RuleSets(); + ruleSets.addRuleSet(ruleSet1); + ruleSets.addRuleSet(ruleSet2); + + // Two violations + PMD p = new PMD(); + RuleContext ctx = new RuleContext(); + Report r = new Report(); + ctx.setReport(r); + ctx.setSourceCodeFilename(file.getName()); + ctx.setSourceCodeFile(file); + p.processFile(new StringReader(TEST1), ruleSets, ctx); + assertEquals("Violations", 2, r.size()); + + // One violation + ruleSet1 = new RuleSet(); + ruleSet1.setName("RuleSet1"); + ruleSet1.addExcludePattern(".*/package/.*"); + ruleSet1.addRule(rule); + + ruleSets = new RuleSets(); + ruleSets.addRuleSet(ruleSet1); + ruleSets.addRuleSet(ruleSet2); + + r = new Report(); + ctx.setReport(r); + p.processFile(new StringReader(TEST1), ruleSets, ctx); + assertEquals("Violations", 1, r.size()); + } + protected void verifyRuleSet(RuleSet IUT, int size, Set values) throws Throwable { RuleContext context = new RuleContext(); @@ -359,6 +408,12 @@ public class RuleSetTest { RC.add(parser.CompilationUnit()); return RC; } + + private static final String TEST1 = "public class Foo {" + PMD.EOL + + " public void foo() {" + PMD.EOL + + " if (true) { }" + PMD.EOL + + " }" + PMD.EOL + + "}" + PMD.EOL; public static junit.framework.Test suite() { return new junit.framework.JUnit4TestAdapter(RuleSetTest.class); diff --git a/pmd/src/net/sourceforge/pmd/AbstractRuleChainVisitor.java b/pmd/src/net/sourceforge/pmd/AbstractRuleChainVisitor.java index 2adcc1e77a..536e1da49a 100644 --- a/pmd/src/net/sourceforge/pmd/AbstractRuleChainVisitor.java +++ b/pmd/src/net/sourceforge/pmd/AbstractRuleChainVisitor.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -19,9 +20,9 @@ import net.sourceforge.pmd.util.Benchmark; */ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { /** - * These are all the rules participating in the RuleChain. + * These are all the rules participating in the RuleChain, grouped by RuleSet. */ - protected List rules = new ArrayList(); + protected Map> ruleSetRules = new LinkedHashMap>(); /** * This is a mapping from node names to nodes instances for the current AST. @@ -29,10 +30,13 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { protected Map> nodeNameToNodes; /** - * @see RuleChainVisitor#add(Rule) + * @see RuleChainVisitor#add(RuleSet, Rule) */ - public void add(Rule rule) { - rules.add(rule); + public void add(RuleSet ruleSet, Rule rule) { + if (!ruleSetRules.containsKey(ruleSet)) { + ruleSetRules.put(ruleSet, new ArrayList()); + } + ruleSetRules.get(ruleSet).add(rule); } /** @@ -49,25 +53,31 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { long end = System.nanoTime(); Benchmark.mark(Benchmark.TYPE_RULE_CHAIN_VISIT, end - start, 1); - // For each rule, allow it to visit the nodes it desires - int visits = 0; - start = System.nanoTime(); - for (Rule rule: rules) { - final List nodeNames = rule.getRuleChainVisits(); - for (int j = 0; j < nodeNames.size(); j++) { - List nodes = nodeNameToNodes.get(nodeNames.get(j)); - for (SimpleNode node: nodes) { - // Visit with underlying Rule, not the RuleReference - while (rule instanceof RuleReference) { - rule = ((RuleReference)rule).getRule(); - } - visit(rule, node, ctx); - } - visits += nodes.size(); + // For each RuleSet, only if this source file applies + for (RuleSet ruleSet : ruleSetRules.keySet()) { + if (!ruleSet.applies(ctx.getSourceCodeFile())) { + continue; + } + // For each rule, allow it to visit the nodes it desires + int visits = 0; + start = System.nanoTime(); + for (Rule rule: ruleSetRules.get(ruleSet)) { + final List nodeNames = rule.getRuleChainVisits(); + for (int j = 0; j < nodeNames.size(); j++) { + List nodes = nodeNameToNodes.get(nodeNames.get(j)); + for (SimpleNode node: nodes) { + // Visit with underlying Rule, not the RuleReference + while (rule instanceof RuleReference) { + rule = ((RuleReference)rule).getRule(); + } + visit(rule, node, ctx); + } + visits += nodes.size(); + } + end = System.nanoTime(); + Benchmark.mark(Benchmark.TYPE_RULE_CHAIN_RULE, rule.getName(), end - start, visits); + start = end; } - end = System.nanoTime(); - Benchmark.mark(Benchmark.TYPE_RULE_CHAIN_RULE, rule.getName(), end - start, visits); - start = end; } } @@ -106,14 +116,21 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { // Determine all node types that need visiting Set visitedNodes = new HashSet(); - for (Iterator i = rules.iterator(); i.hasNext();) { - Rule rule = i.next(); - if (rule.usesRuleChain()) { - visitedNodes.addAll(rule.getRuleChainVisits()); + for (Iterator>> entryIterator = ruleSetRules.entrySet().iterator(); entryIterator.hasNext();) { + Map.Entry> entry = entryIterator.next(); + for (Iterator ruleIterator = entry.getValue().iterator(); ruleIterator.hasNext();) { + Rule rule = ruleIterator.next(); + if (rule.usesRuleChain()) { + visitedNodes.addAll(rule.getRuleChainVisits()); + } + else { + // Drop rules which do not participate in the rule chain. + ruleIterator.remove(); + } } - else { - // Drop rules which do not participate in the rule chain. - i.remove(); + // Drop RuleSets in which all Rules have been dropped. + if (entry.getValue().isEmpty()) { + entryIterator.remove(); } } diff --git a/pmd/src/net/sourceforge/pmd/RuleChain.java b/pmd/src/net/sourceforge/pmd/RuleChain.java index de8c593b30..521705fe55 100644 --- a/pmd/src/net/sourceforge/pmd/RuleChain.java +++ b/pmd/src/net/sourceforge/pmd/RuleChain.java @@ -28,22 +28,24 @@ public class RuleChain { public void add(RuleSet ruleSet) { Language language = ruleSet.getLanguage(); for (Rule r: ruleSet.getRules()) { - add(language, r); + add(ruleSet, r, language); } } /** * Add the given Rule if it wants to participate in the RuleChain. * - * @param language - * The Language used by the Rule. + * @param ruleSet + * The RuleSet to which the rule belongs. * @param rule * The Rule to add. + * @param language + * The Language used by the Rule. */ - public void add(Language language, Rule rule) { + private void add(RuleSet ruleSet, Rule rule, Language language) { RuleChainVisitor visitor = getRuleChainVisitor(language); if (visitor != null) { - visitor.add(rule); + visitor.add(ruleSet, rule); } } diff --git a/pmd/src/net/sourceforge/pmd/RuleChainVisitor.java b/pmd/src/net/sourceforge/pmd/RuleChainVisitor.java index 7f81784710..a10b16d297 100644 --- a/pmd/src/net/sourceforge/pmd/RuleChainVisitor.java +++ b/pmd/src/net/sourceforge/pmd/RuleChainVisitor.java @@ -12,10 +12,12 @@ public interface RuleChainVisitor { /** * Add the given rule to the visitor. * + * @param ruleSet + * The RuleSet to which the rule belongs. * @param rule * The rule to add. */ - void add(Rule rule); + void add(RuleSet ruleSet, Rule rule); /** * Visit all the given ASTCompilationUnits provided using the given