forked from phoedos/pmd
Merge pull request #5223 from adangel/issue-5222-overridden-property-values
[core] Fix RuleReference / RuleSetWriter handling of properties
This commit is contained in:
commit
2c48dc0644
@ -24,6 +24,7 @@ This is a {{ site.pmd.release_type }} release.
|
||||
* core
|
||||
* [#5059](https://github.com/pmd/pmd/issues/5059): \[core] xml output doesn't escape CDATA inside its own CDATA
|
||||
* [#5201](https://github.com/pmd/pmd/issues/5201): \[core] PMD sarif schema file points to nonexistent location
|
||||
* [#5222](https://github.com/pmd/pmd/issues/5222): \[core] RuleReference/RuleSetWriter don't handle changed default property values correctly
|
||||
* java
|
||||
* [#5190](https://github.com/pmd/pmd/issues/5190): \[java] NPE in type inference
|
||||
* java-errorprone
|
||||
|
@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.rule;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@ -237,8 +236,7 @@ public class RuleReference implements Rule {
|
||||
|
||||
@Override
|
||||
public List<PropertyDescriptor<?>> getOverriddenPropertyDescriptors() {
|
||||
return propertyDescriptors == null ? Collections.<PropertyDescriptor<?>>emptyList()
|
||||
: new ArrayList<>(propertyDescriptors);
|
||||
return new ArrayList<>(getOverriddenPropertiesByPropertyDescriptor().keySet());
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -271,9 +271,11 @@ public class RuleSetWriter {
|
||||
// For each provided PropertyDescriptor
|
||||
|
||||
PropertyTypeId typeId = InternalApiBridge.getTypeId(descriptor);
|
||||
// RuleReferences can't define additional properties
|
||||
boolean isPropertyDefinition = typeId != null && !(propertySource instanceof RuleReference);
|
||||
|
||||
if (typeId == null // not defined externally
|
||||
&& !overridden.contains(descriptor)) {
|
||||
// skip properties, which neither are definitions nor override the default value
|
||||
if (!isPropertyDefinition && !overridden.contains(descriptor)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@ -281,10 +283,10 @@ public class RuleSetWriter {
|
||||
propertiesElement = createPropertiesElement();
|
||||
}
|
||||
|
||||
if (typeId != null) {
|
||||
if (isPropertyDefinition) {
|
||||
propertiesElement.appendChild(createPropertyDefinitionElementBR(descriptor, typeId));
|
||||
} else {
|
||||
propertiesElement.appendChild(propertyElementWithValue(propertySource, descriptor));
|
||||
propertiesElement.appendChild(propertyElementWithValueAttribute(propertySource, descriptor));
|
||||
}
|
||||
}
|
||||
|
||||
@ -292,8 +294,14 @@ public class RuleSetWriter {
|
||||
}
|
||||
|
||||
@NonNull
|
||||
private <T> Element propertyElementWithValue(PropertySource propertySource, PropertyDescriptor<T> descriptor) {
|
||||
return createPropertyValueElement(descriptor, propertySource.getProperty(descriptor));
|
||||
private <T> Element propertyElementWithValueAttribute(PropertySource propertySource, PropertyDescriptor<T> propertyDescriptor) {
|
||||
Element element = document.createElementNS(RULESET_2_0_0_NS_URI, "property");
|
||||
SchemaConstants.NAME.setOn(element, propertyDescriptor.name());
|
||||
|
||||
PropertySerializer<T> xmlStrategy = propertyDescriptor.serializer();
|
||||
T value = propertySource.getProperty(propertyDescriptor);
|
||||
SchemaConstants.PROPERTY_VALUE.setOn(element, xmlStrategy.toString(value));
|
||||
return element;
|
||||
}
|
||||
|
||||
private <T> Element createPropertyValueElement(PropertyDescriptor<T> propertyDescriptor, T value) {
|
||||
|
@ -6,11 +6,13 @@ package net.sourceforge.pmd.lang.rule;
|
||||
|
||||
import static net.sourceforge.pmd.PmdCoreTestUtils.dummyLanguage;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import net.sourceforge.pmd.lang.Dummy2LanguageModule;
|
||||
@ -61,6 +63,32 @@ class RuleReferenceTest {
|
||||
validateOverriddenValues(PROPERTY1_DESCRIPTOR, PROPERTY2_DESCRIPTOR, ruleReference);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testOverridingDefaultValueOfProperty() {
|
||||
final PropertyDescriptor<String> PROPERTY1_DESCRIPTOR = PropertyFactory.stringProperty("property1").desc("Test property").defaultValue("the-default").build();
|
||||
MockRule rule = new MockRule();
|
||||
rule.definePropertyDescriptor(PROPERTY1_DESCRIPTOR);
|
||||
Language dummyLang = dummyLanguage();
|
||||
rule.setLanguage(dummyLang);
|
||||
rule.setName("name1");
|
||||
rule.setProperty(PROPERTY1_DESCRIPTOR, "value1");
|
||||
rule.setMessage("message1");
|
||||
rule.setDescription("description1");
|
||||
rule.addExample("example1");
|
||||
rule.setExternalInfoUrl("externalInfoUrl1");
|
||||
rule.setPriority(RulePriority.HIGH);
|
||||
|
||||
RuleReference ruleReference = new RuleReference(rule, null);
|
||||
ruleReference.setProperty(PROPERTY1_DESCRIPTOR, "overridden-value");
|
||||
|
||||
assertTrue(ruleReference.isPropertyOverridden(PROPERTY1_DESCRIPTOR));
|
||||
assertEquals("overridden-value", ruleReference.getProperty(PROPERTY1_DESCRIPTOR), "Override failed");
|
||||
Map<PropertyDescriptor<?>, Object> overriddenPropertiesByPropertyDescriptor = ruleReference.getOverriddenPropertiesByPropertyDescriptor();
|
||||
assertTrue(overriddenPropertiesByPropertyDescriptor.containsKey(PROPERTY1_DESCRIPTOR));
|
||||
List<PropertyDescriptor<?>> overriddenPropertyDescriptors = ruleReference.getOverriddenPropertyDescriptors();
|
||||
assertTrue(overriddenPropertyDescriptors.contains(PROPERTY1_DESCRIPTOR));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testLanguageOverrideDisallowed() {
|
||||
MockRule rule = new MockRule();
|
||||
@ -137,7 +165,7 @@ class RuleReferenceTest {
|
||||
assertEquals("value3", ruleReference.getProperty(propertyDescriptor2), "Override failed");
|
||||
assertTrue(ruleReference.getPropertyDescriptors().contains(propertyDescriptor1), "Override failed");
|
||||
assertTrue(ruleReference.getPropertyDescriptors().contains(propertyDescriptor2), "Override failed");
|
||||
assertFalse(ruleReference.getOverriddenPropertyDescriptors().contains(propertyDescriptor1), "Override failed");
|
||||
assertTrue(ruleReference.getOverriddenPropertyDescriptors().contains(propertyDescriptor1), "Override failed");
|
||||
assertTrue(ruleReference.getOverriddenPropertyDescriptors().contains(propertyDescriptor2), "Override failed");
|
||||
assertTrue(ruleReference.getPropertiesByPropertyDescriptor().containsKey(propertyDescriptor1),
|
||||
"Override failed");
|
||||
|
@ -157,4 +157,23 @@ class RuleSetWriterTest extends RulesetFactoryTestBase {
|
||||
assertThat(written, not(containsString("min=\"")));
|
||||
assertThat(written, containsString("max=\"10\""));
|
||||
}
|
||||
|
||||
@Test
|
||||
void overridingDefaultValueOfPropertyInReference() throws Exception {
|
||||
RuleSet ruleSet = loadRuleSet("created-on-the-fly.xml",
|
||||
rulesetXml(
|
||||
ruleRef("net/sourceforge/pmd/lang/rule/rulesetwriter-test.xml/SampleXPathRuleWithProperty",
|
||||
properties(
|
||||
propertyWithValueAttr("minimum", "42")
|
||||
)
|
||||
)
|
||||
)
|
||||
);
|
||||
writer.write(ruleSet);
|
||||
String written = out.toString(StandardCharsets.UTF_8.name());
|
||||
assertThat(written, not(containsString("min=\"")));
|
||||
assertThat(written, not(containsString("max=\"")));
|
||||
assertThat(written, not(containsString("type=\"")));
|
||||
assertThat(written, containsString("value=\"42\""));
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,23 @@
|
||||
<?xml version="1.0"?>
|
||||
<ruleset name="Test 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 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
|
||||
|
||||
<description>
|
||||
Ruleset used by test net.sourceforge.pmd.lang.rule.RuleSetWriterTest
|
||||
</description>
|
||||
|
||||
<rule name="SampleXPathRuleWithProperty" language="dummy" since="1.1" message="Test Rule 2" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/rules/dummy/basic.xml#SampleXPathRule">
|
||||
<description>Test</description>
|
||||
<priority>3</priority>
|
||||
<properties>
|
||||
<property name="minimum" type="Integer" value="3" min="1" max="100" description="Sample property with default value."/>
|
||||
<property name="xpath">
|
||||
<value><![CDATA[
|
||||
//dummyRootNode
|
||||
]]></value>
|
||||
</property>
|
||||
</properties>
|
||||
<example> </example>
|
||||
</rule>
|
||||
</ruleset>
|
Loading…
x
Reference in New Issue
Block a user