hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-731'

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-11-22 18:03:49 -03:00
parent 204e64ea62 47a29fdb17
commit cbef7a5602
11 changed files with 199 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/pages/release_notes.md
View File

@ -313,6 +313,7 @@ a warning will now be produced suggesting users to adopt it for better performan
### Fixed Issues
* all
* [#394](https://github.com/pmd/pmd/issues/394): \[core] PMD exclude rules are failing with IllegalArgumentException with non-default minimumPriority
* [#532](https://github.com/pmd/pmd/issues/532): \[core] security concerns on URL-based rulesets
* [#538](https://github.com/pmd/pmd/issues/538): \[core] Provide an XML Schema for XML reports
* [#600](https://github.com/pmd/pmd/issues/600): \[core] Nullpointer while creating cache File
@ -423,6 +424,9 @@ a warning will now be produced suggesting users to adopt it for better performan
* The class `net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition` is now abstract, and has been enhanced
to provide several new methods.
* The constructor of `net.sourceforge.pmd.RuleSetFactory`, which took a `ClassLoader` is deprecated.
Please use the alternative constructor with the `net.sourceforge.pmd.util.ResourceLoader` instead.
### External Contributions
* [#287](https://github.com/pmd/pmd/pull/287): \[apex] Make Rule suppression work - [Robert Sösemann](https://github.com/up2go-rsoesemann)

45
pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java
View File

@ -122,15 +122,26 @@ 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("The rule with name " + newRule.getName() + " is duplicated. "
+ "Future versions of PMD will reject to load such rulesets.");
break;
}
}
rules.add(newRule);
return this;
}
@ -163,15 +174,22 @@ public class RuleSet implements ChecksumAware {
* same language was added before, so that the existent rule
* configuration won't be overridden.
*
* @param rule
* @param ruleOrRef
* the new rule to add
* @return The same builder, for a fluid programming interface
*/
public RuleSetBuilder addRuleIfNotExists(final Rule rule) {
if (rule == null) {
public RuleSetBuilder addRuleIfNotExists(final Rule ruleOrRef) {
if (ruleOrRef == null) {
throw new IllegalArgumentException(MISSING_RULE);
}
// resolve the underlying rule, to avoid adding duplicated rules
// if the rule has been renamed/merged and moved at the same time
Rule rule = ruleOrRef;
while (rule instanceof RuleReference) {
rule = ((RuleReference) rule).getRule();
}
boolean exists = false;
for (final Rule r : rules) {
if (r.getName().equals(rule.getName()) && r.getLanguage() == rule.getLanguage()) {
@ -180,7 +198,7 @@ public class RuleSet implements ChecksumAware {
}
}
if (!exists) {
addRule(rule);
addRule(ruleOrRef);
}
return this;
}
@ -387,6 +405,17 @@ public class RuleSet implements ChecksumAware {
public RuleSet build() {
return new RuleSet(this);
}
public void filterRulesByPriority(RulePriority minimumPriority) {
Iterator<Rule> 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();
}
}
}
}
/**

60
pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java
View File

@ -62,6 +62,16 @@ public class RuleSetFactory {
this(new ResourceLoader(), RulePriority.LOW, false, true);
}
/**
* @deprecated Use {@link #RuleSetFactory(ResourceLoader, RulePriority, boolean, boolean)} with
* {@link ResourceLoader} instead of a {@link ClassLoader}.
*/
@Deprecated // to be removed with PMD 7.0.0.
public RuleSetFactory(final ClassLoader classLoader, final RulePriority minimumPriority,
final boolean warnDeprecated, final boolean enableCompatibility) {
this(new ResourceLoader(classLoader), minimumPriority, warnDeprecated, enableCompatibility);
}
public RuleSetFactory(final ResourceLoader resourceLoader, final RulePriority minimumPriority,
final boolean warnDeprecated, final boolean enableCompatibility) {
this.resourceLoader = resourceLoader;
@ -367,6 +377,8 @@ public class RuleSetFactory {
ruleSetBuilder.withDescription("Missing description");
}
ruleSetBuilder.filterRulesByPriority(minimumPriority);
return ruleSetBuilder.build();
} catch (ClassNotFoundException cnfe) {
return classNotFoundProblem(cnfe);
@ -483,22 +495,46 @@ 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));
List<RuleReference> potentialRules = new ArrayList<>();
int countDeprecated = 0;
for (Rule rule : otherRuleSet.getRules()) {
excludedRulesCheck.remove(rule.getName());
if (!ruleSetReference.getExcludes().contains(rule.getName()) && !rule.isDeprecated()) {
if (!ruleSetReference.getExcludes().contains(rule.getName())) {
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)));
}
if (rule.isDeprecated()) {
countDeprecated++;
}
potentialRules.add(ruleReference);
}
}
boolean rulesetDeprecated = false;
if (potentialRules.size() == countDeprecated) {
// all rules in the ruleset have been deprecated - the ruleset itself is considered to be deprecated
rulesetDeprecated = true;
LOG.warning("The RuleSet " + ref + " has been deprecated.");
}
for (RuleReference r : potentialRules) {
if (rulesetDeprecated || !r.getRule().isDeprecated()) {
// add the rule, if either the ruleset itself is deprecated (then we add all rules)
// or if the rule is not deprecated (in that case, the ruleset might contain deprecated as well
// as valid references to rules)
ruleSetBuilder.addRuleIfNotExists(r);
}
}
if (!excludedRulesCheck.isEmpty()) {
throw new IllegalArgumentException(
"Unable to exclude rules " + excludedRulesCheck + "; perhaps the rule name is mispelled?");
@ -529,10 +565,7 @@ public class RuleSetFactory {
Rule rule = new RuleFactory().buildRule(ruleElement);
rule.setRuleSetName(ruleSetBuilder.getName());
if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName())
|| rule.getPriority().compareTo(minimumPriority) <= 0) {
ruleSetBuilder.addRule(rule);
}
ruleSetBuilder.addRule(rule);
}
@ -564,7 +597,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);
@ -611,11 +646,8 @@ public class RuleSetFactory {
ruleReference.setRuleSetReference(ruleSetReference);
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);
}
}

48
pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java
View File

@ -372,6 +372,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);
@ -461,7 +500,7 @@ public class RuleSetFactoryTest {
assertNotNull("RuleSet", ruleSet);
assertFalse("RuleSet empty", ruleSet.getRules().isEmpty());
// No deprecated Rules should be loaded when loading an entire RuleSet
// by reference.
// by reference - unless it contains only deprecated rules - then all rules would be added
Rule r = ruleSet.getRuleByName(DEPRECATED_RULE_NAME);
assertNull("Deprecated Rule Reference", r);
for (Rule rule : ruleSet.getRules()) {
@ -469,6 +508,13 @@ public class RuleSetFactoryTest {
}
}
@Test
public void testDeprecatedRuleSetReference() throws RuleSetNotFoundException {
RuleSetFactory ruleSetFactory = new RuleSetFactory();
RuleSet ruleSet = ruleSetFactory.createRuleSet("net/sourceforge/pmd/rulesets/ruleset-deprecated.xml");
assertEquals(2, ruleSet.getRules().size());
}
@Test
public void testExternalReferences() throws RuleSetNotFoundException {
RuleSet rs = loadRuleSet(EXTERNAL_REFERENCE_RULE_SET);

11
pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-deprecated.xml Normal file
View File

@ -0,0 +1,11 @@
<?xml version="1.0"?>
<ruleset name="Test Reference Deprecated Ruleset" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
Ruleset used by test RuleSetFactoryTest
</description>
<!-- Load in the complete deprecated ruleset -->
<rule ref="rulesets/dummy/deprecated.xml" />
</ruleset>

15
pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority-exclusion.xml Normal file
View File

@ -0,0 +1,15 @@
<?xml version="1.0"?>
<ruleset name="Test Reference Ruleset" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
Ruleset used by test RuleSetFactoryTest
</description>
<!-- Load in the complete basic ruleset -->
<rule ref="rulesets/dummy/basic.xml">
<!-- but exclude DummyBasicMockRule -->
<exclude name="DummyBasicMockRule"/>
</rule>
</ruleset>

17
pmd-core/src/test/resources/net/sourceforge/pmd/rulesets/ruleset-minimum-priority.xml Normal file
View File

@ -0,0 +1,17 @@
<?xml version="1.0"?>
<ruleset name="Test Reference Ruleset" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
Ruleset used by test RuleSetFactoryTest
</description>
<!-- Load in the complete basic ruleset -->
<rule ref="rulesets/dummy/basic.xml" />
<!-- Override the priority of a specific rule -->
<rule ref="rulesets/dummy/basic.xml/DummyBasicMockRule">
<priority>5</priority> <!-- original priority: 3 -->
</rule>
</ruleset>

2
pmd-core/src/test/resources/rulesets/dummy/basic.xml
View File

@ -32,5 +32,5 @@ Just for test
]]></value>
</property>
</properties>
</rule>"
</rule>
</ruleset>

15
pmd-core/src/test/resources/rulesets/dummy/deprecated.xml Normal file
View File

@ -0,0 +1,15 @@
<?xml version="1.0"?>
<ruleset name="Test Deprecated Ruleset" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
Ruleset used by test RuleSetReferenceIdTest
</description>
<!-- the rule has been moved -->
<rule ref="rulesets/dummy/basic.xml/DummyBasicMockRule" deprecated="true" />
<!-- the rule has been renamed/merged and moved -->
<rule ref="rulesets/dummy/basic.xml/DummyBasicMockRule" name="DummyBasicMockRuleOldName" deprecated="true" />
<!-- the rule has been moved -->
<rule ref="rulesets/dummy/basic.xml/SampleXPathRule" deprecated="true" />
</ruleset>

6
pmd-java/src/main/resources/rulesets/java/controversial.xml
View File

@ -10,9 +10,6 @@ The Controversial ruleset contains rules that, for whatever reason, are consider
They are held here to allow people to include them as they see fit within their custom rulesets.
</description>
<!-- The rule UnnecessaryParentheses has been merged into unnecessary.xml/UselessParentheses -->
<rule name="UnnecessaryParentheses" deprecated="true" ref="rulesets/java/unnecessary.xml/UselessParentheses" />
<!-- Rules, that have been moved into a category -->
<rule ref="category/java/errorprone.xml/AssignmentInOperand" deprecated="true" />
<rule ref="category/java/errorprone.xml/AvoidAccessibilityAlteration" deprecated="true" />
@ -41,4 +38,7 @@ They are held here to allow people to include them as they see fit within their
<rule ref="category/java/design.xml/UseObjectForClearerAPI" deprecated="true" />
<!-- The rule UnnecessaryParentheses has been merged into unnecessary.xml/UselessParentheses -->
<rule name="UnnecessaryParentheses" deprecated="true" ref="rulesets/java/unnecessary.xml/UselessParentheses" />
</ruleset>

4
pmd-java/src/main/resources/rulesets/java/unnecessary.xml
View File

@ -13,7 +13,6 @@ The Unnecessary Ruleset contains a collection of rules for unnecessary code.
<rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals" deprecated="true" />
<rule ref="category/java/errorprone.xml/UselessOperationOnImmutable" deprecated="true" />
<rule ref="category/java/codestyle.xml/UnnecessaryModifier" name="UnnecessaryFinalModifier" deprecated="true" />
<rule ref="category/java/codestyle.xml/UnnecessaryModifier" deprecated="true" />
<rule ref="category/java/codestyle.xml/UnnecessaryReturn" deprecated="true" />
<rule ref="category/java/codestyle.xml/UselessParentheses" deprecated="true" />
@ -21,4 +20,7 @@ The Unnecessary Ruleset contains a collection of rules for unnecessary code.
<rule ref="category/java/design.xml/UselessOverridingMethod" deprecated="true" />
<!-- Rule has been renamed -->
<rule ref="category/java/codestyle.xml/UnnecessaryModifier" name="UnnecessaryFinalModifier" deprecated="true" />
</ruleset>