From c00d84aa9afe4b47ccd2e638b09bfd961078caed Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Thu, 2 Sep 2021 16:18:03 +0200
Subject: [PATCH] [core] Fix XPath rulechain optimization bug

For XPath query "//dummyNode[ends-with(@Image,
'foo')][pmd-dummy:typeIs('bar')]"
we lost the first condition (ends-with...) and
only applied the second one (pmd-dummy:typeIs).
---
 .../rule/xpath/internal/RuleChainAnalyzer.java    | 14 +++++++++++++-
 .../lang/rule/xpath/SaxonXPathRuleQueryTest.java  | 10 ++++++++++
 .../xml/AvoidAccessibilityAlteration.xml          | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java
index e093c07dd7..6a704b2035 100644
--- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java
+++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java
@@ -4,8 +4,10 @@
 
 package net.sourceforge.pmd.lang.rule.xpath.internal;
 
+import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.Deque;
 
 import net.sourceforge.pmd.lang.ast.Node;
 
@@ -69,7 +71,17 @@ public class RuleChainAnalyzer extends SaxonExprVisitor {
                     Expression step = newPath.getStepExpression();
                     if (step instanceof FilterExpression) {
                         FilterExpression filterExpression = (FilterExpression) newPath.getStepExpression();
-                        result = new FilterExpression(new AxisExpression(Axis.SELF, null), filterExpression.getFilter());
+
+                        Deque<Expression> filters = new ArrayDeque<>();
+                        Expression walker = filterExpression;
+                        while (walker instanceof FilterExpression) {
+                            filters.push(((FilterExpression) walker).getFilter());
+                            walker = ((FilterExpression) walker).getBaseExpression();
+                        }
+                        result = new FilterExpression(new AxisExpression(Axis.SELF, null), filters.pop());
+                        while (!filters.isEmpty()) {
+                            result = new FilterExpression(result, filters.pop());
+                        }
                         rootElementReplaced = true;
                     } else if (step instanceof AxisExpression) {
                         if (newPath.getStartExpression() instanceof RootExpression) {
diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java
index bfe925bac3..48c2ad3694 100644
--- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java
+++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java
@@ -193,6 +193,16 @@ public class SaxonXPathRuleQueryTest {
         assertExpression("DocumentSorter((LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (((/)/descendant::element(dummyNode, xs:anyType))[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))/child::element(foo, xs:anyType)))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
     }
 
+    @Test
+    public void ruleChainVisitWithTwoFunctions() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[ends-with(@Image, 'foo')][pmd-dummy:typeIs('bar')]");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("((self::node()[ends-with(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))), \"foo\")])[pmd-dummy:typeIs(\"bar\")])", query.nodeNameToXPaths.get("dummyNode").get(0));
+    }
+
     private static void assertExpression(String expected, Expression actual) {
         Assert.assertEquals(normalizeExprDump(expected),
                             normalizeExprDump(actual.toString()));
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidAccessibilityAlteration.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidAccessibilityAlteration.xml
index 8fd4834958..9e215b28eb 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidAccessibilityAlteration.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidAccessibilityAlteration.xml
@@ -150,4 +150,19 @@ public class Violation {
 }
         ]]></code>
     </test-code>
+
+    <test-code>
+        <description>false positive when accessible object is used as primary prefix</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.lang.reflect.Constructor;
+import java.util.List;
+
+public class NoViolation { {
+    List<Constructor<?>> list = new ArrayList<>();
+    Constructor<?> ctor = NoViolation.class.getConstructor();
+    list.add(ctor);
+} }
+        ]]></code>
+    </test-code>
 </test-data>
\ No newline at end of file