diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java index 870f9d0bb1..b193ede887 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java @@ -122,15 +122,24 @@ public class RuleSet implements ChecksumAware { * Add a new rule to this ruleset. Note that this method does not check * for duplicates. * - * @param rule + * @param newRule * the rule to be added * @return The same builder, for a fluid programming interface */ - public RuleSetBuilder addRule(final Rule rule) { - if (rule == null) { + public RuleSetBuilder addRule(final Rule newRule) { + if (newRule == null) { throw new IllegalArgumentException(MISSING_RULE); } - rules.add(rule); + + // check for duplicates - adding more than one rule with the same name will + // be problematic - see #RuleSet.getRuleByName(String) + for (Rule rule : rules) { + if (rule.getName().equals(newRule.getName()) && rule.getLanguage() == newRule.getLanguage()) { + LOG.warning("Duplicated rule: " + newRule.getName()); + } + } + + rules.add(newRule); return this; } @@ -387,6 +396,17 @@ public class RuleSet implements ChecksumAware { public RuleSet build() { return new RuleSet(this); } + + public void filterRulesByPriority(RulePriority minimumPriority) { + Iterator iterator = rules.iterator(); + while (iterator.hasNext()) { + Rule rule = iterator.next(); + if (rule.getPriority().compareTo(minimumPriority) > 0) { + LOG.fine("Removing rule " + rule.getName() + " due to priority: " + rule.getPriority() + " required: " + minimumPriority); + iterator.remove(); + } + } + } } /** diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index 3fe00f9f1f..953f36ee49 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -380,6 +380,8 @@ public class RuleSetFactory { ruleSetBuilder.withDescription("Missing description"); } + ruleSetBuilder.filterRulesByPriority(minimumPriority); + return ruleSetBuilder.build(); } catch (ClassNotFoundException cnfe) { return classNotFoundProblem(cnfe); @@ -496,7 +498,9 @@ public class RuleSetFactory { } final RuleSetReference ruleSetReference = new RuleSetReference(ref, true, excludedRulesCheck); - RuleSetFactory ruleSetFactory = new RuleSetFactory(this, warnDeprecated); + // load the ruleset with minimum priority low, so that we get all rules, to be able to exclude any rule + // minimum priority will be applied again, before constructing the final ruleset + RuleSetFactory ruleSetFactory = new RuleSetFactory(resourceLoader, RulePriority.LOW, warnDeprecated, this.compatibilityFilter != null); RuleSet otherRuleSet = ruleSetFactory.createRuleSet(RuleSetReferenceId.parse(ref).get(0)); for (Rule rule : otherRuleSet.getRules()) { excludedRulesCheck.remove(rule.getName()); @@ -504,12 +508,12 @@ public class RuleSetFactory { RuleReference ruleReference = new RuleReference(); ruleReference.setRuleSetReference(ruleSetReference); ruleReference.setRule(rule); - ruleSetBuilder.addRuleIfNotExists(ruleReference); - // override the priority if (priority != null) { ruleReference.setPriority(RulePriority.valueOf(Integer.parseInt(priority))); } + + ruleSetBuilder.addRuleIfNotExists(ruleReference); } } if (!excludedRulesCheck.isEmpty()) { @@ -639,12 +643,9 @@ public class RuleSetFactory { throw new IllegalArgumentException(UNEXPECTED_ELEMENT + nodeName + "> encountered as child of element for Rule " + rule.getName()); } + } - } - if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName()) - || rule.getPriority().compareTo(minimumPriority) <= 0) { - ruleSetBuilder.addRule(rule); - } + ruleSetBuilder.addRule(rule); } private static boolean hasAttributeSetTrue(Element element, String attributeId) { @@ -679,7 +680,9 @@ public class RuleSetFactory { return; } - RuleSetFactory ruleSetFactory = new RuleSetFactory(this, warnDeprecated); + // load the ruleset with minimum priority low, so that we get all rules, to be able to exclude any rule + // minimum priority will be applied again, before constructing the final ruleset + RuleSetFactory ruleSetFactory = new RuleSetFactory(resourceLoader, RulePriority.LOW, warnDeprecated, this.compatibilityFilter != null); boolean isSameRuleSet = false; RuleSetReferenceId otherRuleSetReferenceId = RuleSetReferenceId.parse(ref).get(0); @@ -756,11 +759,8 @@ public class RuleSetFactory { } } - if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName()) - || referencedRule.getPriority().compareTo(minimumPriority) <= 0) { - if (withDeprecatedRuleReferences || !isSameRuleSet || !ruleReference.isDeprecated()) { - ruleSetBuilder.addRuleReplaceIfExists(ruleReference); - } + if (withDeprecatedRuleReferences || !isSameRuleSet || !ruleReference.isDeprecated()) { + ruleSetBuilder.addRuleReplaceIfExists(ruleReference); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java index 5278b74f9e..6afe8bf3c5 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java @@ -363,6 +363,45 @@ public class RuleSetFactoryTest { assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef")); } + @Test + public void testOverridePriorityLoadWithMinimum() throws RuleSetNotFoundException { + RuleSetFactory rsf = new RuleSetFactory(new ResourceLoader(), RulePriority.MEDIUM_LOW, true, true); + RuleSet ruleset = rsf.createRuleSet("net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml"); + // only one rule should remain, since we filter out the other rules by minimum priority + assertEquals("Number of Rules", 1, ruleset.getRules().size()); + + // Priority is overridden and applied, rule is missing + assertNull(ruleset.getRuleByName("DummyBasicMockRule")); + + // that's the remaining rule + assertNotNull(ruleset.getRuleByName("SampleXPathRule")); + + // now, load with default minimum priority + rsf = new RuleSetFactory(); + ruleset = rsf.createRuleSet("net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml"); + assertEquals("Number of Rules", 2, ruleset.getRules().size()); + Rule dummyBasicMockRule = ruleset.getRuleByName("DummyBasicMockRule"); + assertEquals("Wrong Priority", RulePriority.LOW, dummyBasicMockRule.getPriority()); + } + + @Test + public void testExcludeWithMinimumPriority() throws RuleSetNotFoundException { + RuleSetFactory rsf = new RuleSetFactory(new ResourceLoader(), RulePriority.HIGH, true, true); + RuleSet ruleset = rsf.createRuleSet("net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml"); + // no rules should be loaded + assertEquals("Number of Rules", 0, ruleset.getRules().size()); + + // now, load with default minimum priority + rsf = new RuleSetFactory(); + ruleset = rsf.createRuleSet("net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml"); + // only one rule, we have excluded one... + assertEquals("Number of Rules", 1, ruleset.getRules().size()); + // rule is excluded + assertNull(ruleset.getRuleByName("DummyBasicMockRule")); + // that's the remaining rule + assertNotNull(ruleset.getRuleByName("SampleXPathRule")); + } + @Test public void testOverrideMessage() throws RuleSetNotFoundException { Rule r = loadFirstRule(REF_OVERRIDE_ORIGINAL_NAME); diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml b/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml new file mode 100644 index 0000000000..564335d810 --- /dev/null +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml @@ -0,0 +1,15 @@ + + + + + Ruleset used by test RuleSetFactoryTest + + + + + + + + + \ No newline at end of file diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml b/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml new file mode 100644 index 0000000000..9693540999 --- /dev/null +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml @@ -0,0 +1,17 @@ + + + + + Ruleset used by test RuleSetFactoryTest + + + + + + + + 5 + + + \ No newline at end of file diff --git a/pmd-core/src/test/resources/rulesets/dummy/basic.xml b/pmd-core/src/test/resources/rulesets/dummy/basic.xml index 0a141832a2..30ffe0cc47 100644 --- a/pmd-core/src/test/resources/rulesets/dummy/basic.xml +++ b/pmd-core/src/test/resources/rulesets/dummy/basic.xml @@ -32,5 +32,5 @@ Just for test ]]> - " + \ No newline at end of file