Merge branch 'saxon-rulechain'

docs/pages/release_notes.md

@@ -14,6 +14,24 @@ This is a {{ site.pmd.release_type }} release.
 
 ### New and noteworthy
 
+#### Performance improvements for XPath 2.0 rules
+
+XPath rules written with XPath 2.0 now support conversion to a rulechain rule, which
+improves their performance. The rulechain is a mechanism that allows several rules
+to be executed in a single tree traversal. Conversion to the rulechain is possible if
+your XPath expression looks like `//someNode/... | //someOtherNode/...  | ...`, that
+is, a union of one or more path expressions that start with `//`. Instead of traversing
+the whole tree once per path expression (and per rule), a single traversal executes all
+rules in your ruleset as needed.
+
+This conversion is performed automatically and cannot be disabled. *The conversion should
+not change the result of your rules*, if it does, please report a bug at https://github.com/pmd/pmd/issues
+
+Note that XPath 1.0 support, the default XPath version, is deprecated since PMD 6.22.0.
+**We highly recommend that you upgrade your rules to XPath 2.0**. Please refer to the [migration guide](https://pmd.github.io/latest/pmd_userdocs_extending_writing_xpath_rules.html#migrating-from-10-to-20).
+
+
+
 ### Fixed Issues
 
 *   apex

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

@@ -157,9 +157,13 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor {
                 Rule rule = ruleIterator.next();
                 if (rule.isRuleChain()) {
                     visitedNodes.addAll(rule.getRuleChainVisits());
+
+                    logXPathRuleChainUsage(true, rule);
                 } else {
                     // Drop rules which do not participate in the rule chain.
                     ruleIterator.remove();
+
+                    logXPathRuleChainUsage(false, rule);
                 }
             }
             // Drop RuleSets in which all Rules have been dropped.
@@ -178,6 +182,23 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor {
         }
     }
 
+    private void logXPathRuleChainUsage(boolean usesRuleChain, Rule rule) {
+        if (LOG.isLoggable(Level.FINE)) {
+            Rule r;
+            if (rule instanceof RuleReference) {
+                r = ((RuleReference) rule).getRule();
+            } else {
+                r = rule;
+            }
+            if (r instanceof XPathRule) {
+                String message = (usesRuleChain ? "Using " : "no ")
+                        + "rule chain for XPath " + rule.getProperty(XPathRule.VERSION_DESCRIPTOR)
+                        + " rule: " + rule.getName() + " (" + rule.getRuleSetName() + ")";
+                LOG.fine(message);
+            }
+        }
+    }
+
     /**
      * Clears the internal data structure used to manage the nodes visited
      * between visiting different ASTs.

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

@@ -45,11 +45,11 @@ public class JaxenXPathRuleQuery extends AbstractXPathRuleQuery {
         NONE, PARTIAL, FULL
     }
 
-    // Mapping from Node name to applicable XPath queries
     private InitializationStatus initializationStatus = InitializationStatus.NONE;
-    private Map<String, List<XPath>> nodeNameToXPaths;
+    // Mapping from Node name to applicable XPath queries
+    Map<String, List<XPath>> nodeNameToXPaths;
 
-    private static final String AST_ROOT = "_AST_ROOT_";
+    static final String AST_ROOT = "_AST_ROOT_";
 
     @Override
     public boolean isSupportedVersion(String version) {

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

@@ -1,24 +1,32 @@
-/**
+/*
  * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
  */
 
 package net.sourceforge.pmd.lang.rule.xpath;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import net.sourceforge.pmd.RuleContext;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.xpath.saxon.DocumentNode;
 import net.sourceforge.pmd.lang.ast.xpath.saxon.ElementNode;
+import net.sourceforge.pmd.lang.rule.xpath.internal.RuleChainAnalyzer;
 import net.sourceforge.pmd.lang.xpath.Initializer;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 
+import net.sf.saxon.expr.Expression;
 import net.sf.saxon.om.Item;
 import net.sf.saxon.om.NamespaceConstant;
+import net.sf.saxon.om.SequenceIterator;
 import net.sf.saxon.om.ValueRepresentation;
 import net.sf.saxon.sxpath.AbstractStaticContext;
 import net.sf.saxon.sxpath.IndependentContext;
@@ -44,6 +52,12 @@ import net.sf.saxon.value.Value;
  * This is a Saxon based XPathRule query.
  */
 public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
+    /**
+     * Special nodeName that references the root expression.
+     */
+    static final String AST_ROOT = "_AST_ROOT_";
+
+    private static final Logger LOG = Logger.getLogger(SaxonXPathRuleQuery.class.getName());
 
     private static final int MAX_CACHE_SIZE = 20;
     private static final Map<Node, DocumentNode> CACHE = new LinkedHashMap<Node, DocumentNode>(MAX_CACHE_SIZE) {
@@ -55,6 +69,11 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         }
     };
 
+    /**
+     * Contains for each nodeName a sub expression, used for implementing rule chain.
+     */
+    Map<String, List<Expression>> nodeNameToXPaths = new HashMap<>();
+
     /**
      * Representation of an XPath query, created at {@link #initializeXPathExpression()} using {@link #xpath}.
      */
@@ -82,24 +101,41 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
 
             // Map AST Node -> Saxon Node
             final ElementNode rootElementNode = documentNode.nodeToElementNode.get(node);
-
             final XPathDynamicContext xpathDynamicContext = createDynamicContext(rootElementNode);
-            final List<ElementNode> nodes = xpathExpression.evaluate(xpathDynamicContext);
+
+            final List<ElementNode> nodes = new LinkedList<>();
+            List<Expression> expressions = getXPathExpressionForNodeOrDefault(node.getXPathNodeName());
+            for (Expression expression : expressions) {
+                SequenceIterator iterator = expression.iterate(xpathDynamicContext.getXPathContextObject());
+                Item current = iterator.next();
+                while (current != null) {
+                    nodes.add((ElementNode) current);
+                    current = iterator.next();
+                }
+            }
 
             /*
              Map List of Saxon Nodes -> List of AST Nodes, which were detected to match the XPath expression
              (i.e. violation found)
               */
-            final List<Node> results = new ArrayList<>();
+            final List<Node> results = new ArrayList<>(nodes.size());
             for (final ElementNode elementNode : nodes) {
                 results.add((Node) elementNode.getUnderlyingNode());
             }
+            Collections.sort(results, RuleChainAnalyzer.documentOrderComparator());
             return results;
         } catch (final XPathException e) {
             throw new RuntimeException(super.xpath + " had problem: " + e.getMessage(), e);
         }
     }
 
+    private List<Expression> getXPathExpressionForNodeOrDefault(String nodeName) {
+        if (nodeNameToXPaths.containsKey(nodeName)) {
+            return nodeNameToXPaths.get(nodeName);
+        }
+        return nodeNameToXPaths.get(AST_ROOT);
+    }
+
     /**
      * Attempt to create a dynamic context on which to evaluate the {@link #xpathExpression}.
      *
@@ -171,6 +207,13 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         return root;
     }
 
+    private void addExpressionForNode(String nodeName, Expression expression) {
+        if (!nodeNameToXPaths.containsKey(nodeName)) {
+            nodeNameToXPaths.put(nodeName, new LinkedList<Expression>());
+        }
+        nodeNameToXPaths.get(nodeName).add(expression);
+    }
+
     /**
      * Initialize the {@link #xpathExpression} and the {@link #xpathVariables}.
      */
@@ -206,15 +249,48 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
                 }
             }
 
-            // TODO Come up with a way to make use of RuleChain. I had hacked up
-            // an approach which used Jaxen's stuff, but that only works for
-            // 1.0 compatibility mode. Rather do it right instead of kludging.
             xpathExpression = xpathEvaluator.createExpression(super.xpath);
+            analyzeXPathForRuleChain(xpathEvaluator);
         } catch (final XPathException e) {
             throw new RuntimeException(e);
         }
     }
+    
+    private void analyzeXPathForRuleChain(final XPathEvaluator xpathEvaluator) {
+        final Expression expr = xpathExpression.getInternalExpression();
 
+        boolean useRuleChain = true;
+
+        // First step: Split the union venn expressions into single expressions
+        Iterable<Expression> subexpressions = RuleChainAnalyzer.splitUnions(expr);
+
+        // Second step: Analyze each expression separately
+        for (Expression subexpression : subexpressions) {
+            RuleChainAnalyzer rca = new RuleChainAnalyzer(xpathEvaluator.getConfiguration());
+            Expression modified = rca.visit(subexpression);
+
+            if (rca.getRootElement() != null) {
+                addExpressionForNode(rca.getRootElement(), modified);
+            } else {
+                // couldn't find a root element for the expression, that means, we can't use rule chain at all
+                // even though, it would be possible for part of the expression.
+                useRuleChain = false;
+                break;
+            }
+        }
+
+        if (useRuleChain) {
+            super.ruleChainVisits.addAll(nodeNameToXPaths.keySet());
+        } else {
+            nodeNameToXPaths.clear();
+            if (LOG.isLoggable(Level.FINE)) {
+                LOG.log(Level.FINE, "Unable to use RuleChain for XPath: " + xpath);
+            }
+        }
+
+        // always add fallback expression
+        addExpressionForNode(AST_ROOT, xpathExpression.getInternalExpression());
+    }
 
     /**
      * Gets the Saxon representation of the parameter, if its type corresponds
@@ -267,4 +343,10 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         }
         return new SequenceExtent(converted);
     }
+
+    @Override
+    public List<String> getRuleChainVisits() {
+        initializeXPathExpression();
+        return super.getRuleChainVisits();
+    }
 }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/DocumentSorter.java

@@ -0,0 +1,38 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+
+package net.sourceforge.pmd.lang.rule.xpath.internal;
+
+import java.util.Comparator;
+
+import net.sourceforge.pmd.lang.ast.Node;
+
+/**
+ * Sorts nodes by document order.
+ */
+final class DocumentSorter implements Comparator<Node> {
+
+    public static final DocumentSorter INSTANCE = new DocumentSorter();
+
+    private DocumentSorter() {
+
+    }
+
+    @Override
+    public int compare(Node node1, Node node2) {
+        if (node1 == null && node2 == null) {
+            return 0;
+        } else if (node1 == null) {
+            return -1;
+        } else if (node2 == null) {
+            return 1;
+        }
+        int result = node1.getBeginLine() - node2.getBeginLine();
+        if (result == 0) {
+            result = node1.getBeginColumn() - node2.getBeginColumn();
+        }
+        return result;
+    }
+}

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/ExpressionPrinter.java

@@ -0,0 +1,53 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.rule.xpath.internal;
+
+import net.sf.saxon.expr.AxisExpression;
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.RootExpression;
+import net.sf.saxon.expr.Token;
+import net.sf.saxon.expr.VennExpression;
+import net.sf.saxon.om.Axis;
+
+/**
+ * Simple printer for saxon expressions. Might be useful for debugging / during development.
+ */
+public class ExpressionPrinter extends Visitor {
+    private int depth = 0;
+
+    private void print(String s) {
+        for (int i = 0; i < depth; i++) {
+            System.out.print("    ");
+        }
+        System.out.println(s);
+    }
+
+    @Override
+    public Expression visit(AxisExpression e) {
+        print("axis=" + Axis.axisName[e.getAxis()] + "(test=" + e.getNodeTest() + ")");
+        return super.visit(e);
+    }
+
+    @Override
+    public Expression visit(RootExpression e) {
+        print("/");
+        return super.visit(e);
+    }
+
+    @Override
+    public Expression visit(VennExpression e) {
+        print("venn=" + Token.tokens[e.getOperator()]);
+        return super.visit(e);
+    }
+
+    @Override
+    public Expression visit(Expression expr) {
+        depth++;
+        print(expr.getClass().getSimpleName());
+        Expression result = super.visit(expr);
+        depth--;
+        return result;
+    }
+}

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java

@@ -0,0 +1,116 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.rule.xpath.internal;
+
+import java.util.Collections;
+import java.util.Comparator;
+
+import net.sourceforge.pmd.lang.ast.Node;
+
+import net.sf.saxon.Configuration;
+import net.sf.saxon.expr.AxisExpression;
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.FilterExpression;
+import net.sf.saxon.expr.PathExpression;
+import net.sf.saxon.expr.RootExpression;
+import net.sf.saxon.om.Axis;
+import net.sf.saxon.pattern.NameTest;
+import net.sf.saxon.sort.DocumentSorter;
+import net.sf.saxon.type.Type;
+
+/**
+ * Analyzes the xpath expression to find the root path selector for a element. If found,
+ * the element name is available via {@link RuleChainAnalyzer#getRootElement()} and the
+ * expression is rewritten to start at "node::self()" instead.
+ *
+ * <p>It uses a visitor to visit all the different expressions.
+ *
+ * <p>Example: The XPath expression <code>//A[condition()]/B</code> results the rootElement "A"
+ * and the expression is rewritten to be <code>self::node[condition()]/B</code>.
+ *
+ * <p>DocumentSorter expression is removed. The sorting of the resulting nodes needs to be done
+ * after all (sub)expressions have been executed.
+ */
+public class RuleChainAnalyzer extends Visitor {
+    private final Configuration configuration;
+    private String rootElement;
+    private boolean rootElementReplaced;
+
+    public RuleChainAnalyzer(Configuration currentConfiguration) {
+        this.configuration = currentConfiguration;
+    }
+
+    public String getRootElement() {
+        return rootElement;
+    }
+
+    @Override
+    public Expression visit(DocumentSorter e) {
+        DocumentSorter result = (DocumentSorter) super.visit(e);
+        // sorting of the nodes must be done after all nodes have been found
+        return result.getBaseExpression();
+    }
+
+    @Override
+    public Expression visit(PathExpression e) {
+        if (rootElement == null) {
+            Expression result = super.visit(e);
+            if (rootElement != null && !rootElementReplaced) {
+                if (result instanceof PathExpression) {
+                    PathExpression newPath = (PathExpression) result;
+                    if (newPath.getStepExpression() instanceof FilterExpression) {
+                        FilterExpression filterExpression = (FilterExpression) newPath.getStepExpression();
+                        result = new FilterExpression(new AxisExpression(Axis.SELF, null), filterExpression.getFilter());
+                        rootElementReplaced = true;
+                    } else if (newPath.getStepExpression() instanceof AxisExpression) {
+                        if (newPath.getStartExpression() instanceof RootExpression) {
+                            result = new AxisExpression(Axis.SELF, null);
+                        } else {
+                            result = new PathExpression(newPath.getStartExpression(), new AxisExpression(Axis.SELF, null));
+                        }
+                        rootElementReplaced = true;
+                    }
+                } else {
+                    result = new AxisExpression(Axis.DESCENDANT_OR_SELF, null);
+                    rootElementReplaced = true;
+                }
+            }
+            return result;
+        } else {
+            return super.visit(e);
+        }
+    }
+
+    @Override
+    public Expression visit(AxisExpression e) {
+        if (rootElement == null && e.getNodeTest() instanceof NameTest) {
+            NameTest test = (NameTest) e.getNodeTest();
+            if (test.getPrimitiveType() == Type.ELEMENT && e.getAxis() == Axis.DESCENDANT) {
+                rootElement = configuration.getNamePool().getClarkName(test.getFingerprint());
+            } else if (test.getPrimitiveType() == Type.ELEMENT && e.getAxis() == Axis.CHILD) {
+                rootElement = configuration.getNamePool().getClarkName(test.getFingerprint());
+            }
+        }
+        return super.visit(e);
+    }
+
+    public static Comparator<Node> documentOrderComparator() {
+        return net.sourceforge.pmd.lang.rule.xpath.internal.DocumentSorter.INSTANCE;
+    }
+
+    /**
+     * Split union expressions into their components.
+     */
+    public static Iterable<Expression> splitUnions(Expression expr) {
+        SplitUnions unions = new SplitUnions();
+        unions.visit(expr);
+        if (unions.getExpressions().isEmpty()) {
+            return Collections.singletonList(expr);
+        } else {
+            return unions.getExpressions();
+        }
+    }
+
+}

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java

@@ -0,0 +1,40 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+
+package net.sourceforge.pmd.lang.rule.xpath.internal;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.Token;
+import net.sf.saxon.expr.VennExpression;
+
+/**
+ * Splits a venn expression with the union operator into single expressions.
+ * 
+ * <p>E.g. "//A | //B | //C" will result in 3 expressions "//A", "//B", and "//C".
+ */
+class SplitUnions extends Visitor {
+    private List<Expression> expressions = new ArrayList<>();
+
+    @Override
+    public Expression visit(VennExpression e) {
+        if (e.getOperator() == Token.UNION) {
+            for (Expression operand : e.getOperands()) {
+                if (operand instanceof VennExpression) {
+                    visit(operand);
+                } else {
+                    expressions.add(operand);
+                }
+            }
+        }
+        return e;
+    }
+
+    public List<Expression> getExpressions() {
+        return expressions;
+    }
+}

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/Visitor.java

@@ -0,0 +1,89 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.rule.xpath.internal;
+
+import net.sf.saxon.expr.AxisExpression;
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.FilterExpression;
+import net.sf.saxon.expr.LetExpression;
+import net.sf.saxon.expr.PathExpression;
+import net.sf.saxon.expr.QuantifiedExpression;
+import net.sf.saxon.expr.RootExpression;
+import net.sf.saxon.expr.VennExpression;
+import net.sf.saxon.sort.DocumentSorter;
+
+abstract class Visitor {
+    public Expression visit(DocumentSorter e) {
+        Expression base = visit(e.getBaseExpression());
+        return new DocumentSorter(base);
+    }
+
+    public Expression visit(PathExpression e) {
+        Expression start = visit(e.getStartExpression());
+        Expression step = visit(e.getStepExpression());
+        return new PathExpression(start, step);
+    }
+
+    public Expression visit(RootExpression e) {
+        return e;
+    }
+
+    public Expression visit(AxisExpression e) {
+        return e;
+    }
+
+    public Expression visit(VennExpression e) {
+        final Expression[] operands = e.getOperands();
+        Expression operand0 = visit(operands[0]);
+        Expression operand1 = visit(operands[1]);
+        return new VennExpression(operand0, e.getOperator(), operand1);
+    }
+
+    public Expression visit(FilterExpression e) {
+        Expression base = visit(e.getBaseExpression());
+        Expression filter = visit(e.getFilter());
+        return new FilterExpression(base, filter);
+    }
+
+    public Expression visit(QuantifiedExpression e) {
+        return e;
+    }
+
+    public Expression visit(LetExpression e) {
+        Expression action = visit(e.getAction());
+        Expression sequence = visit(e.getSequence());
+        LetExpression result = new LetExpression();
+        result.setAction(action);
+        result.setSequence(sequence);
+        result.setVariableQName(e.getVariableQName());
+        result.setRequiredType(e.getRequiredType());
+        result.setSlotNumber(e.getLocalSlotNumber());
+        return result;
+    }
+
+    public Expression visit(Expression expr) {
+        Expression result;
+        if (expr instanceof DocumentSorter) {
+            result = visit((DocumentSorter) expr);
+        } else if (expr instanceof PathExpression) {
+            result = visit((PathExpression) expr);
+        } else if (expr instanceof RootExpression) {
+            result = visit((RootExpression) expr);
+        } else if (expr instanceof AxisExpression) {
+            result = visit((AxisExpression) expr);
+        } else if (expr instanceof VennExpression) {
+            result = visit((VennExpression) expr);
+        } else if (expr instanceof FilterExpression) {
+            result = visit((FilterExpression) expr);
+        } else if (expr instanceof QuantifiedExpression) {
+            result = visit((QuantifiedExpression) expr);
+        } else if (expr instanceof LetExpression) {
+            result = visit((LetExpression) expr);
+        } else {
+            result = expr;
+        }
+        return result;
+    }
+}

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

@@ -34,6 +34,98 @@ public class JaxenXPathRuleQueryTest {
         assertQuery(0, "//dummyNode[@EmptyList = \"A\"]", dummy);
     }
 
+    @Test
+    public void ruleChainVisits() {
+        final String xpath = "//dummyNode[@Image='baz']/foo | //bar[@Public = 'true'] | //dummyNode[@Public = 'false'] | //dummyNode";
+        JaxenXPathRuleQuery query = createQuery(xpath);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(3, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertTrue(ruleChainVisits.contains("bar"));
+        // Note: Having AST_ROOT in the rule chain visits is probably a mistake. But it doesn't hurt, it shouldn't
+        // match a real node name.
+        Assert.assertTrue(ruleChainVisits.contains(JaxenXPathRuleQuery.AST_ROOT));
+
+        DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
+        RuleContext data = new RuleContext();
+        data.setLanguageVersion(LanguageRegistry.findLanguageByTerseName("dummy").getDefaultVersion());
+
+        query.evaluate(dummy, data);
+        // note: the actual xpath queries are only available after evaluating
+        Assert.assertEquals(3, query.nodeNameToXPaths.size());
+        Assert.assertEquals("self::node()", query.nodeNameToXPaths.get("dummyNode").get(0).toString());
+        Assert.assertEquals("self::node()[(attribute::Public = \"false\")]", query.nodeNameToXPaths.get("dummyNode").get(1).toString());
+        Assert.assertEquals("self::node()[(attribute::Image = \"baz\")]/child::foo", query.nodeNameToXPaths.get("dummyNode").get(2).toString());
+        Assert.assertEquals("self::node()[(attribute::Public = \"true\")]", query.nodeNameToXPaths.get("bar").get(0).toString());
+        Assert.assertEquals(xpath, query.nodeNameToXPaths.get(JaxenXPathRuleQuery.AST_ROOT).get(0).toString());
+    }
+
+    @Test
+    public void ruleChainVisitsMultipleFilters() {
+        final String xpath = "//dummyNode[@Test1 = 'false'][@Test2 = 'true']";
+        JaxenXPathRuleQuery query = createQuery(xpath);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(2, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        // Note: Having AST_ROOT in the rule chain visits is probably a mistake. But it doesn't hurt, it shouldn't
+        // match a real node name.
+        Assert.assertTrue(ruleChainVisits.contains(JaxenXPathRuleQuery.AST_ROOT));
+
+        DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
+        RuleContext data = new RuleContext();
+        data.setLanguageVersion(LanguageRegistry.findLanguageByTerseName("dummy").getDefaultVersion());
+
+        query.evaluate(dummy, data);
+        // note: the actual xpath queries are only available after evaluating
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        Assert.assertEquals("self::node()[(attribute::Test1 = \"false\")][(attribute::Test2 = \"true\")]", query.nodeNameToXPaths.get("dummyNode").get(0).toString());
+        Assert.assertEquals(xpath, query.nodeNameToXPaths.get(JaxenXPathRuleQuery.AST_ROOT).get(0).toString());
+    }
+
+    @Test
+    public void ruleChainVisitsNested() {
+        final String xpath = "//dummyNode/foo/*/bar[@Test = 'false']";
+        JaxenXPathRuleQuery query = createQuery(xpath);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(2, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        // Note: Having AST_ROOT in the rule chain visits is probably a mistake. But it doesn't hurt, it shouldn't
+        // match a real node name.
+        Assert.assertTrue(ruleChainVisits.contains(JaxenXPathRuleQuery.AST_ROOT));
+
+        DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
+        RuleContext data = new RuleContext();
+        data.setLanguageVersion(LanguageRegistry.findLanguageByTerseName("dummy").getDefaultVersion());
+
+        query.evaluate(dummy, data);
+        // note: the actual xpath queries are only available after evaluating
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        Assert.assertEquals("self::node()/child::foo/child::*/child::bar[(attribute::Test = \"false\")]", query.nodeNameToXPaths.get("dummyNode").get(0).toString());
+        Assert.assertEquals(xpath, query.nodeNameToXPaths.get(JaxenXPathRuleQuery.AST_ROOT).get(0).toString());
+    }
+
+    @Test
+    public void ruleChainVisitsNested2() {
+        final String xpath = "//dummyNode/foo[@Baz = 'a']/*/bar[@Test = 'false']";
+        JaxenXPathRuleQuery query = createQuery(xpath);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(2, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        // Note: Having AST_ROOT in the rule chain visits is probably a mistake. But it doesn't hurt, it shouldn't
+        // match a real node name.
+        Assert.assertTrue(ruleChainVisits.contains(JaxenXPathRuleQuery.AST_ROOT));
+
+        DummyNodeWithListAndEnum dummy = new DummyNodeWithListAndEnum(1);
+        RuleContext data = new RuleContext();
+        data.setLanguageVersion(LanguageRegistry.findLanguageByTerseName("dummy").getDefaultVersion());
+
+        query.evaluate(dummy, data);
+        // note: the actual xpath queries are only available after evaluating
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        Assert.assertEquals("self::node()/child::foo[(attribute::Baz = \"a\")]/child::*/child::bar[(attribute::Test = \"false\")]", query.nodeNameToXPaths.get("dummyNode").get(0).toString());
+        Assert.assertEquals(xpath, query.nodeNameToXPaths.get(JaxenXPathRuleQuery.AST_ROOT).get(0).toString());
+    }
+
     private static void assertQuery(int resultSize, String xpath, Node node) {
         JaxenXPathRuleQuery query = createQuery(xpath);
         RuleContext data = new RuleContext();

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

@@ -5,7 +5,9 @@
 package net.sourceforge.pmd.lang.rule.xpath;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -13,6 +15,10 @@ import org.junit.Test;
 import net.sourceforge.pmd.RuleContext;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
+import net.sourceforge.pmd.properties.PropertyFactory;
+
+import net.sf.saxon.expr.Expression;
+import net.sf.saxon.expr.XPathContext;
 
 public class SaxonXPathRuleQueryTest {
 
@@ -31,16 +37,130 @@ public class SaxonXPathRuleQueryTest {
         assertQuery(0, "//dummyNode[@EmptyList = (\"A\")]", dummy);
     }
 
+    @Test
+    public void ruleChainVisits() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[@Image='baz']/foo | //bar[@Public = 'true'] | //dummyNode[@Public = false()] | //dummyNode");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(2, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertTrue(ruleChainVisits.contains("bar"));
+
+        Assert.assertEquals(3, query.nodeNameToXPaths.size());
+        assertExpression("((self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Image, xs:anyAtomicType)), ($qq:qq106374177 singleton eq \"baz\"))])/child::element(foo, xs:anyType))", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("(self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq609962972 singleton eq false()))])", query.nodeNameToXPaths.get("dummyNode").get(1));
+        assertExpression("self::node()", query.nodeNameToXPaths.get("dummyNode").get(2));
+        assertExpression("(self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq232307208 singleton eq \"true\"))])", query.nodeNameToXPaths.get("bar").get(0));
+        assertExpression("(((DocumentSorter(((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Image, xs:anyAtomicType)), ($qq:qq000 singleton eq \"baz\"))])/child::element(foo, xs:anyType))) | (((/)/descendant::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq000 singleton eq \"true\"))])) | (((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq000 singleton eq false()))])) | ((/)/descendant::element(dummyNode, xs:anyType)))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    @Test
+    public void ruleChainVisitsMultipleFilters() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[@Test1 = false()][@Test2 = true()]");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("((self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Test2, xs:anyAtomicType)), ($qq:qq1741979653 singleton eq true()))])[QuantifiedExpression(Atomizer(attribute::attribute(Test1, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq false()))])", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test2, xs:anyAtomicType)), ($qq:qq1741979653 singleton eq true()))])[QuantifiedExpression(Atomizer(attribute::attribute(Test1, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq false()))])", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    public static class TestFunctions {
+        public static boolean typeIs(final XPathContext context, final String fullTypeName) {
+            return false;
+        }
+    }
+
+    @Test
+    public void ruleChainVisitsNested() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode/foo/*/bar[@Test = 'false']");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("((((self::node()/child::element(foo, xs:anyType))/child::element())/child::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test, xs:anyAtomicType)), ($qq:qq166794956 singleton eq \"false\"))])", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("DocumentSorter(((((((/)/descendant::element(dummyNode, xs:anyType))/child::element(foo, xs:anyType))/child::element())/child::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test, xs:anyAtomicType)), ($qq:qq166794956 singleton eq \"false\"))]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    @Test
+    public void ruleChainVisitsNested2() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode/foo[@Baz = 'a']/*/bar[@Test = 'false']");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("(((((self::node()/child::element(foo, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Baz, xs:anyAtomicType)), ($qq:qq306612792 singleton eq \"a\"))])/child::element())/child::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test, xs:anyAtomicType)), ($qq:qq1803669141 singleton eq \"false\"))])", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("DocumentSorter((((((((/)/descendant::element(dummyNode, xs:anyType))/child::element(foo, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Baz, xs:anyAtomicType)), ($qq:qq306612792 singleton eq \"a\"))])/child::element())/child::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test, xs:anyAtomicType)), ($qq:qq1803669141 singleton eq \"false\"))]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    @Test
+    public void ruleChainVisitWithVariable() {
+        PropertyDescriptor<String> testClassPattern = PropertyFactory.stringProperty("testClassPattern").desc("test").defaultValue("a").build();
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[matches(@SimpleName, $testClassPattern)]", testClassPattern);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (self::node()[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (((/)/descendant::element(dummyNode, xs:anyType))[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    @Test
+    public void ruleChainVisitWithVariable2() {
+        PropertyDescriptor<String> testClassPattern = PropertyFactory.stringProperty("testClassPattern").desc("test").defaultValue("a").build();
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[matches(@SimpleName, $testClassPattern)]/foo", testClassPattern);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("(LetExpression(LazyExpression(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer($testClassPattern))))), (self::node()[matches(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(SimpleName, xs:anyAtomicType))))), $zz:zz952562199)]))/child::element(foo, xs:anyType))", query.nodeNameToXPaths.get("dummyNode").get(0));
+        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));
+    }
+
+    private static void assertExpression(String expected, Expression actual) {
+        Assert.assertEquals(normalizeExprDump(expected),
+                            normalizeExprDump(actual.toString()));
+        //Assert.assertEquals(expected, actual);
+    }
+
+    private static String normalizeExprDump(String dump) {
+        return dump.replaceAll("\\$qq:qq-?\\d+", "\\$qq:qq000")
+                   .replaceAll("\\$zz:zz-?\\d+", "\\$zz:zz000");
+    }
+
+    @Test
+    public void ruleChainVisitsCompatibilityMode() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[@Image='baz']/foo | //bar[@Public = 'true'] | //dummyNode[@Public = 'false']");
+        query.setVersion(XPathRuleQuery.XPATH_1_0_COMPATIBILITY);
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(2, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertTrue(ruleChainVisits.contains("bar"));
+
+        Assert.assertEquals(3, query.nodeNameToXPaths.size());
+        assertExpression("((self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Image, xs:anyAtomicType)), ($qq:qq6519275 singleton eq \"baz\"))])/child::element(foo, xs:anyType))", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("(self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq \"false\"))])", query.nodeNameToXPaths.get("dummyNode").get(1));
+        assertExpression("(self::node()[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq1484171695 singleton eq \"true\"))])", query.nodeNameToXPaths.get("bar").get(0));
+        assertExpression("((DocumentSorter(((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Image, xs:anyAtomicType)), ($qq:qq692331943 singleton eq \"baz\"))])/child::element(foo, xs:anyType))) | (((/)/descendant::element(bar, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq2127036371 singleton eq \"true\"))])) | (((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Public, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq \"false\"))]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
     private static void assertQuery(int resultSize, String xpath, Node node) {
         SaxonXPathRuleQuery query = createQuery(xpath);
         List<Node> result = query.evaluate(node, new RuleContext());
         Assert.assertEquals(resultSize, result.size());
     }
 
-    private static SaxonXPathRuleQuery createQuery(String xpath) {
+    private static SaxonXPathRuleQuery createQuery(String xpath, PropertyDescriptor<?> ...descriptors) {
         SaxonXPathRuleQuery query = new SaxonXPathRuleQuery();
-        query.setVersion("2.0");
-        query.setProperties(Collections.<PropertyDescriptor<?>, Object>emptyMap());
+        query.setVersion(XPathRuleQuery.XPATH_2_0);
+        if (descriptors != null) {
+            Map<PropertyDescriptor<?>, Object> props = new HashMap<PropertyDescriptor<?>, Object>();
+            for (PropertyDescriptor<?> prop : descriptors) {
+                props.put(prop, prop.defaultValue());
+            }
+            query.setProperties(props);
+        } else {
+            query.setProperties(Collections.<PropertyDescriptor<?>, Object>emptyMap());
+        }
         query.setXPath(xpath);
         return query;
     }