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

Rule references use rule factory. Properties declared in a rule ref throw an exception while parsing

Browse Source
This commit is contained in:
Clément Fournier
2017-09-16 01:23:34 +02:00
parent 845e7ca7a8
commit 86b0bb4cd7
4 changed files with 93 additions and 170 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -4,17 +4,13 @@
package net.sourceforge.pmd;
import static net.sourceforge.pmd.properties.PropertyDescriptorField.DEFAULT_VALUE;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.logging.Level;
@ -26,10 +22,8 @@ import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.commons.lang3.StringUtils;
import org.w3c.dom.Attr;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
@ -41,9 +35,6 @@ import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.rule.MockRule;
import net.sourceforge.pmd.lang.rule.RuleReference;
import net.sourceforge.pmd.lang.rule.XPathRule;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyDescriptorField;
import net.sourceforge.pmd.properties.PropertyDescriptorUtil;
import net.sourceforge.pmd.ruleset.RuleFactory;
import net.sourceforge.pmd.util.ResourceLoader;
@ -61,9 +52,6 @@ public class RuleSetFactory {
private static final String DESCRIPTION = "description";
private static final String UNEXPECTED_ELEMENT = "Unexpected element <";
private static final String PRIORITY = "priority";
private static final String FOR_RULE = "' for Rule ";
private static final String MESSAGE = "message";
private static final String EXTERNAL_INFO_URL = "externalInfoUrl";
private final ResourceLoader resourceLoader;
private final RulePriority minimumPriority;
@ -538,7 +526,7 @@ public class RuleSetFactory {
&& !isRuleName(ruleElement, ruleSetReferenceId.getRuleName())) {
return;
}
<< Rule rule = RuleFactory.INSTANCE.buildRule(ruleElement);
Rule rule = RuleFactory.INSTANCE.buildRule(ruleElement);
rule.setRuleSetName(ruleSetBuilder.getName());
if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName())
@ -547,9 +535,6 @@ public class RuleSetFactory {
}
}
private static boolean hasAttributeSetTrue(Element element, String attributeId) {
return element.hasAttribute(attributeId) && "true".equalsIgnoreCase(element.getAttribute(attributeId));
}
/**
* Parse a rule node as a RuleReference. A RuleReference is a single Rule
@ -622,39 +607,9 @@ public class RuleSetFactory {
RuleSetReference ruleSetReference = new RuleSetReference(otherRuleSetReferenceId.getRuleSetFileName(), false);
RuleReference ruleReference = new RuleReference();
RuleReference ruleReference = RuleFactory.INSTANCE.decorateRule(referencedRule, ruleElement);
ruleReference.setRuleSetReference(ruleSetReference);
ruleReference.setRule(referencedRule);
if (ruleElement.hasAttribute("deprecated")) {
ruleReference.setDeprecated(Boolean.parseBoolean(ruleElement.getAttribute("deprecated")));
}
if (ruleElement.hasAttribute("name")) {
ruleReference.setName(ruleElement.getAttribute("name"));
}
if (ruleElement.hasAttribute(MESSAGE)) {
ruleReference.setMessage(ruleElement.getAttribute(MESSAGE));
}
if (ruleElement.hasAttribute(EXTERNAL_INFO_URL)) {
ruleReference.setExternalInfoUrl(ruleElement.getAttribute(EXTERNAL_INFO_URL));
}
for (int i = 0; i < ruleElement.getChildNodes().getLength(); i++) {
Node node = ruleElement.getChildNodes().item(i);
if (node.getNodeType() == Node.ELEMENT_NODE) {
if (node.getNodeName().equals(DESCRIPTION)) {
ruleReference.setDescription(parseTextNode(node));
} else if (node.getNodeName().equals("example")) {
ruleReference.addExample(parseTextNode(node));
} else if (node.getNodeName().equals(PRIORITY)) {
ruleReference.setPriority(RulePriority.valueOf(Integer.parseInt(parseTextNode(node))));
} else if (node.getNodeName().equals("properties")) {
parsePropertiesNode(ruleReference, node);
} else {
throw new IllegalArgumentException(UNEXPECTED_ELEMENT + node.getNodeName()
+ "> encountered as child of <rule> element for Rule " + ruleReference.getName());
}
}
}
if (StringUtils.isNotBlank(ruleSetReferenceId.getRuleName())
|| referencedRule.getPriority().compareTo(minimumPriority) <= 0) {
@ -699,110 +654,6 @@ public class RuleSetFactory {
return node.getNodeType() == Node.ELEMENT_NODE && node.getNodeName().equals(name);
}
/**
* Parse a properties node.
*
* @param rule
* The Rule to which the properties should be added.
* @param propertiesNode
* Must be a properties element node.
*/
private static void parsePropertiesNode(Rule rule, Node propertiesNode) {
for (int i = 0; i < propertiesNode.getChildNodes().getLength(); i++) {
Node node = propertiesNode.getChildNodes().item(i);
if (isElementNode(node, "property")) {
parsePropertyNodeBR(rule, node);
}
}
}
private static String valueFrom(Node parentNode) {
final NodeList nodeList = parentNode.getChildNodes();
for (int i = 0; i < nodeList.getLength(); i++) {
Node node = nodeList.item(i);
if (isElementNode(node, "value")) {
return parseTextNode(node);
}
}
return null;
}
/**
* Sets the value of a property.
*
* @param rule The rule which has the property
* @param desc The property descriptor
* @param strValue The string value of the property, converted to a T
* @param <T> The type of values of the property descriptor
*/
private static <T> void setValue(Rule rule, PropertyDescriptor<T> desc, String strValue) {
T realValue = desc.valueFrom(strValue);
rule.setProperty(desc, realValue);
}
/**
* Parse a property node.
*
* @param rule The Rule to which the property should be added.
* @param propertyNode Must be a property element node.
*/
private static void parsePropertyNodeBR(Rule rule, Node propertyNode) {
Element propertyElement = (Element) propertyNode;
String typeId = propertyElement.getAttribute(PropertyDescriptorField.TYPE.attributeName());
String strValue = propertyElement.getAttribute(DEFAULT_VALUE.attributeName());
if (StringUtils.isBlank(strValue)) {
strValue = valueFrom(propertyElement);
}
// Setting of existing property, or defining a new property?
if (StringUtils.isBlank(typeId)) {
String name = propertyElement.getAttribute(PropertyDescriptorField.NAME.attributeName());
PropertyDescriptor<?> propertyDescriptor = rule.getPropertyDescriptor(name);
if (propertyDescriptor == null) {
throw new IllegalArgumentException(
"Cannot set non-existant property '" + name + "' on Rule " + rule.getName());
} else {
setValue(rule, propertyDescriptor, strValue);
}
return;
}
PropertyDescriptorExternalBuilder<?> pdFactory = PropertyDescriptorUtil.factoryFor(typeId);
if (pdFactory == null) {
throw new RuntimeException("No property descriptor factory for type: " + typeId);
}
Map<PropertyDescriptorField, String> values = new HashMap<>();
NamedNodeMap atts = propertyElement.getAttributes();
// populate a map of values for an individual descriptor
for (int i = 0; i < atts.getLength(); i++) {
Attr a = (Attr) atts.item(i);
values.put(PropertyDescriptorField.getConstant(a.getName()), a.getValue());
}
if (StringUtils.isBlank(values.get(DEFAULT_VALUE))) {
NodeList children = propertyElement.getElementsByTagName(DEFAULT_VALUE.attributeName());
if (children.getLength() == 1) {
values.put(DEFAULT_VALUE, children.item(0).getTextContent());
} else {
throw new RuntimeException("No value defined!");
}
}
// casting is not pretty but prevents the interface from having this method
PropertyDescriptor<?> desc = pdFactory.build(values);
rule.definePropertyDescriptor(desc);
setValue(rule, desc, strValue);
}
/**
* Parse a String from a textually type node.
*

81
pmd-core/src/main/java/net/sourceforge/pmd/ruleset/RuleFactory.java
View File

@ -18,6 +18,8 @@ import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RulePriority;
import net.sourceforge.pmd.lang.rule.RuleReference;
import net.sourceforge.pmd.properties.AbstractPropertyDescriptorFactory;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyDescriptorFactory;
@ -40,6 +42,63 @@ public class RuleFactory {
}
/**
* Decorates a referenced rule with the values that are overriden in the given rule element.
*
* @param referencedRule Referenced rule
* @param ruleElement Element overriding some metadata about the rule
*
* @return A rule reference to the referenced rule
*/
public RuleReference decorateRule(Rule referencedRule, Element ruleElement) {
RuleReference ruleReference = new RuleReference();
ruleReference.setRule(referencedRule);
if (ruleElement.hasAttribute("deprecated")) {
ruleReference.setDeprecated(Boolean.parseBoolean(ruleElement.getAttribute("deprecated")));
}
if (ruleElement.hasAttribute("name")) {
ruleReference.setName(ruleElement.getAttribute("name"));
}
if (ruleElement.hasAttribute("message")) {
ruleReference.setMessage(ruleElement.getAttribute("message"));
}
if (ruleElement.hasAttribute("externalInfoUrl")) {
ruleReference.setExternalInfoUrl(ruleElement.getAttribute("externalInfoUrl"));
}
for (int i = 0; i < ruleElement.getChildNodes().getLength(); i++) {
Node node = ruleElement.getChildNodes().item(i);
if (node.getNodeType() == Node.ELEMENT_NODE) {
switch (node.getNodeName()) {
case "description":
ruleReference.setDescription(parseTextNode(node));
break;
case "example":
ruleReference.addExample(parseTextNode(node));
break;
case "priority":
ruleReference.setPriority(RulePriority.valueOf(Integer.parseInt(parseTextNode(node))));
break;
case "properties":
setPropertyValues(ruleReference, (Element) node);
break;
default:
throw new IllegalArgumentException("Unexpected element <" + node.getNodeName()
+ "> encountered as child of <rule> element for Rule "
+ ruleReference.getName());
}
}
}
return ruleReference;
}
/**
* Parses a rule element and returns a new rule instance.
*
@ -123,7 +182,7 @@ public class RuleFactory {
if (propertiesElement != null) {
overrideProperties(rule, propertiesElement);
setPropertyValues(rule, propertiesElement);
}
return rule;
@ -131,8 +190,7 @@ public class RuleFactory {
private void checkRequiredAttributesArePresent(Element ruleElement) {
final List<String> required = Arrays.asList("name",
"class");
final List<String> required = Arrays.asList("name", "class");
for (String att : required) {
if (!ruleElement.hasAttribute(att)) {
@ -143,20 +201,19 @@ public class RuleFactory {
/**
* Parses a properties element looking only for property value overrides.
* Parses a properties element looking only for the values of the properties defined or overridden.
*
* @param propertiesNode Node to parse
*
* @return The map of overridden properties names to their value
* @return A map of property names to their value
*/
private Map<String, String> parsePropertiesForOverrides(Element propertiesNode) {
private Map<String, String> getPropertyValuesFrom(Element propertiesNode) {
Map<String, String> overridenProperties = new HashMap<>();
for (int i = 0; i < propertiesNode.getChildNodes().getLength(); i++) {
Node node = propertiesNode.getChildNodes().item(i);
if (node.getNodeType() == Node.ELEMENT_NODE && node.getNodeName().equals("property")
&& !isPropertyDefinition((Element) node)) {
Entry<String, String> overridden = parsePropertyOverride((Element) node);
if (node.getNodeType() == Node.ELEMENT_NODE && node.getNodeName().equals("property")) {
Entry<String, String> overridden = getPropertyValue((Element) node);
overridenProperties.put(overridden.getKey(), overridden.getValue());
}
}
@ -185,7 +242,7 @@ public class RuleFactory {
}
private Entry<String, String> parsePropertyOverride(Element propertyElement) {
private Entry<String, String> getPropertyValue(Element propertyElement) {
String name = propertyElement.getAttribute(PropertyDescriptorField.NAME.attributeName());
return new SimpleEntry<>(name, valueFrom(propertyElement));
}
@ -197,8 +254,8 @@ public class RuleFactory {
* @param rule The rule
* @param propertiesElt The {@literal <properties>} element
*/
private void overrideProperties(Rule rule, Element propertiesElt) {
Map<String, String> overridden = parsePropertiesForOverrides(propertiesElt);
private void setPropertyValues(Rule rule, Element propertiesElt) {
Map<String, String> overridden = getPropertyValuesFrom(propertiesElt);
for (Entry<String, String> e : overridden.entrySet()) {
PropertyDescriptor<?> descriptor = rule.getPropertyDescriptor(e.getKey());

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

@ -267,9 +267,6 @@ public class RuleSetFactoryTest {
PropertyDescriptor<?> test3Descriptor = r.getPropertyDescriptor("test3");
assertNotNull("test3 descriptor", test3Descriptor);
assertEquals("override3", r.getProperty(test3Descriptor));
PropertyDescriptor<?> test4Descriptor = r.getPropertyDescriptor("test4");
assertNotNull("test3 descriptor", test4Descriptor);
assertEquals("new property", r.getProperty(test4Descriptor));
}
@Test
@ -688,14 +685,14 @@ public class RuleSetFactoryTest {
private static final String REF_OVERRIDE = "<?xml version=\"1.0\"?>" + PMD.EOL + "<ruleset name=\"test\">" + PMD.EOL
+ " <description>testdesc</description>" + PMD.EOL + " <rule " + PMD.EOL
+ " ref=\"net/sourceforge/pmd/TestRuleset1.xml/MockRule1\" " + PMD.EOL + " name=\"TestNameOverride\" "
+ " ref=\"net/sourceforge/pmd/TestRuleset1.xml/MockRule4\" " + PMD.EOL + " name=\"TestNameOverride\" "
+ PMD.EOL + " message=\"Test message override\"> " + PMD.EOL
+ " <description>Test description override</description>" + PMD.EOL
+ " <example>Test example override</example>" + PMD.EOL + " <priority>3</priority>" + PMD.EOL
+ " <properties>" + PMD.EOL
+ " <property name=\"test2\" description=\"test2\" type=\"String\" value=\"override2\"/>" + PMD.EOL
+ " <property name=\"test3\" description=\"test3\" type=\"String\"><value>override3</value></property>"
+ PMD.EOL + " <property name=\"test4\" description=\"test4\" type=\"String\" value=\"new property\"/>"
+ " <property name=\"test2\" description=\"test2\" value=\"override2\"/>" + PMD.EOL
+ " <property name=\"test3\" description=\"test3\"><value>override3</value></property>"
// + PMD.EOL + " <property name=\"test4\" description=\"test4\" type=\"String\" value=\"new property\"/>" // Nonsense
+ PMD.EOL + " </properties>" + PMD.EOL + " </rule>" + PMD.EOL + "</ruleset>";
private static final String REF_INTERNAL_TO_INTERNAL = "<?xml version=\"1.0\"?>" + PMD.EOL

18
pmd-core/src/test/resources/net/sourceforge/pmd/TestRuleset1.xml
View File

@ -32,6 +32,24 @@ Just for test
<rule name="MockRule3" deprecated="true" ref="net/sourceforge/pmd/TestRuleset2.xml/TestRule"/>
<rule name="MockRule4" language="dummy" since="1.0" message="Test Rule"
class="net.sourceforge.pmd.lang.rule.MockRule"
externalInfoUrl="${pmd.website.baseurl}/rules/test/TestRuleset1.xml#MockRule">
<description>
Just for test
</description>
<priority>3</priority>
<properties>
<property name="test2" type="String" description="test 2" value="foo"/>
<property name="test3" type="String" description="test 3" value="bar"/>
</properties>
<example>
<![CDATA[
]]>
</example>
</rule>
<rule name="TestRuleRef" ref="net/sourceforge/pmd/TestRuleset2.xml/TestRule"/>
</ruleset>