[core] Fix RuleReference / RuleSetWriter handling of properties

RuleReference#getOverriddenPropertyDescriptors should behave
consistent to #getOverriddenPropertiesByPropertyDescriptor.
RuleSetWriter needs to make sure to
export only the values for properties
of rule references.

Fixes #5222

docs/pages/release_notes.md

@@ -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

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleReference.java

@@ -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()
+        return new ArrayList<>(getOverriddenPropertiesByPropertyDescriptor().keySet());
-                                           : new ArrayList<>(propertyDescriptors);
     }
 
     @Override

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleSetWriter.java

@@ -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
+            // skip properties, which neither are definitions nor override the default value
-                && !overridden.contains(descriptor)) {
+            if (!isPropertyDefinition && !overridden.contains(descriptor)) {
                 continue;
             }
 
@@ -281,7 +283,7 @@ public class RuleSetWriter {
                 propertiesElement = createPropertiesElement();
             }
 
-            if (typeId != null) {
+            if (isPropertyDefinition) {
                 propertiesElement.appendChild(createPropertyDefinitionElementBR(descriptor, typeId));
             } else {
                 propertiesElement.appendChild(propertyElementWithValue(propertySource, descriptor));

pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/RuleReferenceTest.java

@@ -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");

pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/RuleSetWriterTest.java

@@ -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("42"));
+    }
 }

pmd-core/src/test/resources/net/sourceforge/pmd/lang/rule/rulesetwriter-test.xml

@@ -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>