Merge branch 'pr-2407'

[core] Deprecate Jaxen and XPath internal API #2407

docs/pages/release_notes.md

@@ -69,6 +69,11 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD
 Those APIs are not intended to be used by clients, and will be hidden or removed with PMD 7.0.0.
 You can identify them with the `@InternalApi` annotation. You'll also get a deprecation warning.
 
+*   {% jdoc core::lang.rule.xpath.AbstractXPathRuleQuery %}
+*   {% jdoc core::lang.rule.xpath.JaxenXPathRuleQuery %}
+*   {% jdoc core::lang.rule.xpath.SaxonXPathRuleQuery %}
+*   {% jdoc core::lang.rule.xpath.XPathRuleQuery %}
+
 ##### In ASTs
 
 As part of the changes we'd like to do to AST classes for 7.0.0, we would like to
@@ -125,6 +130,9 @@ implementations, and their corresponding Parser if it exists (in the same packag
 *   {% jdoc !!core::lang.ast.AbstractTokenManager#setFileName(java.lang.String) %}
 *   {% jdoc !!core::lang.ast.AbstractTokenManager#getFileName(java.lang.String) %}
 *   {% jdoc !!core::cpd.token.AntlrToken#getType() %} - use `getKind()` instead.
+*   {% jdoc core::lang.rule.ImmutableLanguage %}
+*   {% jdoc core::lang.rule.MockRule %}
+*   Multiple fields, constructors and methods in {% jdoc core::lang.rule.XPathRule %}. See javadoc for details.
 
 ### External Contributions
 

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexHandler.java

@@ -12,7 +12,6 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
 import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface;
 import net.sourceforge.pmd.lang.apex.ast.ApexNode;
@@ -22,7 +21,6 @@ import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey;
 import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey;
 import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileVisitorFacade;
 import net.sourceforge.pmd.lang.apex.rule.ApexRuleViolationFactory;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider;
 import net.sourceforge.pmd.lang.metrics.internal.AbstractLanguageMetricsProvider;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
@@ -38,12 +36,6 @@ public class ApexHandler extends AbstractLanguageVersionHandler {
         return rootNode -> new ApexMultifileVisitorFacade().initializeWith((ApexNode<?>) rootNode);
     }
 
-
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return ApexRuleViolationFactory.INSTANCE;

pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java

@@ -305,7 +305,7 @@ public class RuleSetFactory {
     public RuleSet createSingleRuleRuleSet(final Rule rule) { // TODO make static?
         final long checksum;
         if (rule instanceof XPathRule) {
-            checksum = rule.getProperty(XPathRule.XPATH_DESCRIPTOR).hashCode();
+            checksum = ((XPathRule) rule).getXPathExpression().hashCode();
         } else {
             // TODO : Is this good enough? all properties' values + rule name
             checksum = rule.getPropertiesByPropertyDescriptor().values().hashCode() * 31 + rule.getName().hashCode();

pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java

@@ -18,6 +18,7 @@ import net.sourceforge.pmd.util.designerbindings.DesignerBindings;
  */
 public abstract class AbstractLanguageVersionHandler implements LanguageVersionHandler {
 
+
     @Override
     public DataFlowHandler getDataFlowHandler() {
         return DataFlowHandler.DUMMY;

pmd-core/src/main/java/net/sourceforge/pmd/lang/XPathHandler.java

@@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang;
 import org.jaxen.Navigator;
 
 import net.sourceforge.pmd.annotation.InternalApi;
+import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.xpath.Initializer;
 
 import net.sf.saxon.sxpath.IndependentContext;
@@ -19,22 +20,7 @@ import net.sf.saxon.sxpath.IndependentContext;
 @Deprecated
 public interface XPathHandler {
 
-    XPathHandler DUMMY = new XPathHandler() {
+    XPathHandler DUMMY = new DefaultASTXPathHandler();
-        @Override
-        public void initialize() {
-            // empty handler - does nothing
-        }
-
-        @Override
-        public void initialize(IndependentContext context) {
-            // empty handler - does nothing
-        }
-
-        @Override
-        public Navigator getNavigator() {
-            return null;
-        }
-    };
 
     /**
      * Initialize. This is intended to be called by {@link Initializer} to

pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AbstractASTXPathHandler.java

@@ -25,4 +25,14 @@ public abstract class AbstractASTXPathHandler implements XPathHandler {
     public void initialize(IndependentContext context, Language language, Class<?> functionsClass) {
         context.declareNamespace("pmd-" + language.getTerseName(), "java:" + functionsClass.getName());
     }
+
+    @Override
+    public void initialize() {
+        // override if needed
+    }
+
+    @Override
+    public void initialize(IndependentContext context) {
+        // override if needed
+    }
 }

pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/DefaultASTXPathHandler.java

@@ -12,6 +12,7 @@ import net.sf.saxon.sxpath.IndependentContext;
 @Deprecated
 @InternalApi
 public class DefaultASTXPathHandler extends AbstractASTXPathHandler {
+
     @Override
     public void initialize() {
         // override if needed

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

@@ -8,6 +8,10 @@ package net.sourceforge.pmd.lang.rule;
  * This is a tag interface to indicate that a Rule implementation class does not
  * support changes to it's Language. The Language is integral to the proper
  * functioning of the Rule.
+ *
+ * @deprecated No rule supports a change to their language. This will
+ *     be made the default behaviour with PMD 7.0.0.
  */
+@Deprecated
 public interface ImmutableLanguage {
 }

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

@@ -20,7 +20,12 @@ import net.sourceforge.pmd.properties.PropertyFactory;
  * functional Rule is not needed. For example, during unit testing, or as an
  * editable surrogate used by IDE plugins. The Language of this Rule defaults to
  * Java.
+ *
+ * @deprecated This is not a supported API. You need the pmd-test module
+ *     on your classpath, or pmd-core's test sources. This will be removed
+ *     in 7.0.0
  */
+@Deprecated
 public class MockRule extends AbstractRule {
 
     public MockRule() {

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

@@ -12,6 +12,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.commons.lang3.StringUtils;
 
@@ -20,17 +21,19 @@ import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.rule.xpath.JaxenXPathRuleQuery;
 import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
 import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery;
+import net.sourceforge.pmd.lang.rule.xpath.XPathVersion;
 import net.sourceforge.pmd.properties.EnumeratedProperty;
 import net.sourceforge.pmd.properties.StringProperty;
 
 /**
  * Rule that tries to match an XPath expression against a DOM view of an AST.
- *
- * <p>This rule needs a "xpath" property value in order to function.</p>
  */
 public class XPathRule extends AbstractRule {
 
-    // TODO 7.0.0 use PropertyDescriptor<String>
+    /**
+     * @deprecated Use {@link #XPathRule(XPathVersion, String)}
+     */
+    @Deprecated
     public static final StringProperty XPATH_DESCRIPTOR = StringProperty.named("xpath")
             .desc("XPath expression")
             .defaultValue("")
@@ -47,7 +50,11 @@ public class XPathRule extends AbstractRule {
         XPATH_VERSIONS = Collections.unmodifiableMap(tmp);
     }
 
-    // published, can't be converted
+
+    /**
+     * @deprecated Use {@link #XPathRule(XPathVersion, String)}
+     */
+    @Deprecated
     public static final EnumeratedProperty<String> VERSION_DESCRIPTOR = EnumeratedProperty.<String>named("version")
             .desc("XPath specification version")
             .mappings(XPATH_VERSIONS)
@@ -63,6 +70,8 @@ public class XPathRule extends AbstractRule {
 
     /**
      * Creates a new XPathRule without the corresponding XPath query.
+     *
+     * @deprecated Use {@link #XPathRule(XPathVersion, String)}
      */
     public XPathRule() {
         definePropertyDescriptor(XPATH_DESCRIPTOR);
@@ -73,6 +82,8 @@ public class XPathRule extends AbstractRule {
 
     /**
      * Creates a new XPathRule and associates the XPath query.
+     *
+     * @deprecated Use {@link #XPathRule(XPathVersion, String)}
      */
     public XPathRule(final String xPath) {
         this();
@@ -80,21 +91,54 @@ public class XPathRule extends AbstractRule {
     }
 
     /**
-     * Sets the XPath to query against the desired nodes in {@link #apply(List, RuleContext)}.
+     * Make a new XPath rule with the given version + expression
      *
-     * @param xPath the XPath query
+     * @param version    Version of the XPath language
+     * @param expression XPath expression
+     *
+     * @throws NullPointerException If any of the arguments is null
      */
+    public XPathRule(XPathVersion version, String expression) {
+        this();
+        Objects.requireNonNull(version, "XPath version is null");
+        Objects.requireNonNull(expression, "XPath expression is null");
+        setXPath(expression);
+        setVersion(version.getXmlName());
+    }
+
+    /**
+     * Returns the version for this rule. Returns null if this is not
+     * set or invalid.
+     */
+    public XPathVersion getVersion() {
+        return XPathVersion.ofId(getProperty(VERSION_DESCRIPTOR));
+    }
+
+    /**
+     * Returns the XPath expression that implements this rule.
+     */
+    public String getXPathExpression() {
+        return getProperty(XPATH_DESCRIPTOR);
+    }
+
+    /**
+     * @deprecated Use the constructor {@link #XPathRule(XPathVersion, String)},
+     *     don't set the expression after the fact.
+     */
+    @Deprecated
     public void setXPath(final String xPath) {
         setProperty(XPathRule.XPATH_DESCRIPTOR, xPath);
     }
 
+    /**
+     * @deprecated Use the constructor {@link #XPathRule(XPathVersion, String)},
+     *     don't set the version after the fact.
+     */
+    @Deprecated
     public void setVersion(final String version) {
         setProperty(XPathRule.VERSION_DESCRIPTOR, version);
     }
 
-    /**
-     * Apply the rule to all nodes.
-     */
     @Override
     public void apply(List<? extends Node> nodes, RuleContext ctx) {
         for (Node node : nodes) {
@@ -107,7 +151,10 @@ public class XPathRule extends AbstractRule {
      *
      * @param node The Node that to be checked.
      * @param data The RuleContext.
+     *
+     * @deprecated Use {@link #apply(List, RuleContext)}
      */
+    @Deprecated
     public void evaluate(final Node node, final RuleContext data) {
         if (xPathRuleQueryNeedsInitialization()) {
             initXPathRuleQuery();
@@ -124,13 +171,21 @@ public class XPathRule extends AbstractRule {
      * engine in which the query will be run it looks at the XPath version.
      */
     private void initXPathRuleQuery() {
-        String xpath = getProperty(XPATH_DESCRIPTOR);
+        String xpath = getXPathExpression();
-        String version = getProperty(VERSION_DESCRIPTOR);
+        XPathVersion version = getVersion();
 
-        initRuleQueryBasedOnVersion(version);
+        if (version == null) {
+            throw new IllegalStateException("Invalid XPath version, should have been caught by Rule::dysfunctionReason");
+        }
+
+        if (version == XPathVersion.XPATH_1_0) {
+            xpathRuleQuery = new JaxenXPathRuleQuery();
+        } else {
+            xpathRuleQuery = new SaxonXPathRuleQuery();
+        }
 
         xpathRuleQuery.setXPath(xpath);
-        xpathRuleQuery.setVersion(version);
+        xpathRuleQuery.setVersion(version.getXmlName());
         xpathRuleQuery.setProperties(getPropertiesByPropertyDescriptor());
     }
 
@@ -143,10 +198,6 @@ public class XPathRule extends AbstractRule {
         return xpathRuleQuery == null;
     }
 
-    private void initRuleQueryBasedOnVersion(final String version) {
-        xpathRuleQuery = XPATH_1_0.equals(version) ? new JaxenXPathRuleQuery() : new SaxonXPathRuleQuery();
-    }
-
     @Override
     public List<String> getRuleChainVisits() {
         if (xPathRuleQueryNeedsInitialization()) {
@@ -161,10 +212,11 @@ public class XPathRule extends AbstractRule {
 
     @Override
     public String dysfunctionReason() {
-        return hasXPathExpression() ? null : "Missing xPath expression";
+        if (getVersion() == null) {
-    }
+            return "Invalid XPath version '" + getProperty(VERSION_DESCRIPTOR) + "'";
-
+        } else if (StringUtils.isBlank(getXPathExpression())) {
-    private boolean hasXPathExpression() {
+            return "Missing XPath expression";
-        return StringUtils.isNotBlank(getProperty(XPATH_DESCRIPTOR));
+        }
+        return null;
     }
 }

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

@@ -9,12 +9,17 @@ import java.util.List;
 import java.util.Map;
 
 import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.annotation.InternalApi;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 
 /**
  * This implementation of XPathRuleQuery provides support for RuleChain visits.
+ *
+ * @deprecated Internal API
  */
+@Deprecated
+@InternalApi
 public abstract class AbstractXPathRuleQuery implements XPathRuleQuery {
 
     /**

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

@@ -31,12 +31,17 @@ import org.jaxen.expr.XPathFactory;
 import org.jaxen.saxpath.Axis;
 
 import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.annotation.InternalApi;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 
 /**
  * This is a Jaxen based XPathRule query.
+ *
+ * @deprecated Internal API
  */
+@Deprecated
+@InternalApi
 public class JaxenXPathRuleQuery extends AbstractXPathRuleQuery {
 
     private static final Logger LOG = Logger.getLogger(JaxenXPathRuleQuery.class.getName());

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

@@ -16,6 +16,7 @@ import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.annotation.InternalApi;
 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;
@@ -50,7 +51,11 @@ import net.sf.saxon.value.Value;
 
 /**
  * This is a Saxon based XPathRule query.
+ *
+ * @deprecated Internal API
  */
+@Deprecated
+@InternalApi
 public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
     /**
      * Special nodeName that references the root expression.

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

@@ -8,6 +8,7 @@ import java.util.List;
 import java.util.Map;
 
 import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.annotation.InternalApi;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 
@@ -22,24 +23,38 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
  * are recommended to manage internal state that is invariant over AST Nodes in
  * a fashion which facilities high performance (e.g. caching).
  * </p>
+ *
+ * @deprecated This will be internalized in 7.0.0.
  */
+@InternalApi
+@Deprecated
 public interface XPathRuleQuery {
 
     /**
      * XPath 1.0 version.
+     *
+     * @deprecated Use {@link XPathVersion}
      */
+    @Deprecated
     String XPATH_1_0 = "1.0";
 
     /**
      * XPath 1.0 compatibility version.
+     *
+     * @deprecated Use {@link XPathVersion}
      */
+    @Deprecated
     String XPATH_1_0_COMPATIBILITY = "1.0 compatibility";
 
     /**
      * XPath 2.0 version.
+     *
+     * @deprecated Use {@link XPathVersion}
      */
+    @Deprecated
     String XPATH_2_0 = "2.0";
 
+
     /**
      * Set the XPath query string to be used.
      *

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

@@ -0,0 +1,70 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.rule.xpath;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Constants for XPath language version used in XPath queries.
+ */
+public enum XPathVersion {
+    /**
+     * XPath 1.0.
+     *
+     * @deprecated Will become unsupported in 7.0.0
+     */
+    @Deprecated
+    XPATH_1_0(XPathRuleQuery.XPATH_1_0),
+
+    /**
+     * XPath 1.0 compatibility mode.
+     *
+     * @deprecated Will become unsupported in 7.0.0
+     */
+    @Deprecated
+    XPATH_1_0_COMPATIBILITY(XPathRuleQuery.XPATH_1_0_COMPATIBILITY),
+
+    /** XPath 2.0. */
+    XPATH_2_0(XPathRuleQuery.XPATH_2_0);
+
+    private static final Map<String, XPathVersion> BY_NAME = new HashMap<>();
+    private final String version;
+
+
+    static {
+        for (XPathVersion value : values()) {
+            BY_NAME.put(value.getXmlName(), value);
+        }
+    }
+
+
+    XPathVersion(String version) {
+        this.version = version;
+    }
+
+
+    /**
+     * Returns the string used to represent the version in the XML.
+     *
+     * @return A string representation
+     */
+    public String getXmlName() {
+        return version;
+    }
+
+
+    /**
+     * Gets an XPath version from the string used to represent
+     * it in the XML.
+     *
+     * @param version A version string
+     *
+     * @return An XPath version, or null if the argument is not a valid version
+     */
+    public static XPathVersion ofId(String version) {
+        return BY_NAME.get(version);
+    }
+}

pmd-doc/src/main/java/net/sourceforge/pmd/docs/RuleDocGenerator.java

@@ -405,10 +405,11 @@ public class RuleDocGenerator {
                     lines.addAll(EscapeUtils.escapeLines(toLines(stripIndentation(rule.getDescription()))));
                     lines.add("");
 
-                    if (rule instanceof XPathRule || rule instanceof RuleReference && ((RuleReference) rule).getRule() instanceof XPathRule) {
+                    XPathRule xpathRule = asXPathRule(rule);
+                    if (xpathRule != null) {
                         lines.add("**This rule is defined by the following XPath expression:**");
                         lines.add("``` xpath");
-                        lines.addAll(toLines(StringUtils.stripToEmpty(rule.getProperty(XPathRule.XPATH_DESCRIPTOR))));
+                        lines.addAll(toLines(StringUtils.stripToEmpty(xpathRule.getXPathExpression())));
                         lines.add("```");
                     } else {
                         lines.add("**This rule is defined by the following Java class:** "
@@ -502,6 +503,15 @@ public class RuleDocGenerator {
         }
     }
 
+    private XPathRule asXPathRule(Rule rule) {
+        if (rule instanceof XPathRule) {
+            return (XPathRule) rule;
+        } else if (rule instanceof RuleReference && ((RuleReference) rule).getRule() instanceof XPathRule) {
+            return (XPathRule) ((RuleReference) rule).getRule();
+        }
+        return null;
+    }
+
     private static boolean isDeprecated(PropertyDescriptor<?> propertyDescriptor) {
         return propertyDescriptor.description() != null
             && propertyDescriptor.description().toLowerCase(Locale.ROOT).startsWith(DEPRECATED_RULE_PROPERTY_MARKER);

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/xpath/XPathMetricFunctionTest.java

@@ -26,6 +26,7 @@ import net.sourceforge.pmd.lang.LanguageRegistry;
 import net.sourceforge.pmd.lang.java.JavaLanguageModule;
 import net.sourceforge.pmd.lang.java.xpath.MetricFunction;
 import net.sourceforge.pmd.lang.rule.XPathRule;
+import net.sourceforge.pmd.lang.rule.xpath.XPathVersion;
 
 /**
  * @author Clément Fournier
@@ -40,8 +41,7 @@ public class XPathMetricFunctionTest {
 
 
     private Rule makeXpathRuleFromXPath(String xpath) {
-        XPathRule rule = new XPathRule();
+        XPathRule rule = new XPathRule(XPathVersion.XPATH_1_0, xpath);
-        rule.setXPath(xpath);
         rule.setMessage(VIOLATION_MESSAGE);
         rule.setLanguage(LanguageRegistry.getLanguage(JavaLanguageModule.NAME));
         return rule;

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java

@@ -12,7 +12,6 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import net.sourceforge.pmd.PMD;
@@ -35,9 +34,9 @@ import net.sourceforge.pmd.lang.rule.XPathRule;
 import net.sourceforge.pmd.lang.rule.xpath.JaxenXPathRuleQuery;
 import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
 import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery;
+import net.sourceforge.pmd.lang.rule.xpath.XPathVersion;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
-import net.sourceforge.pmd.properties.StringMultiProperty;
+import net.sourceforge.pmd.properties.PropertyFactory;
-import net.sourceforge.pmd.properties.StringProperty;
 import net.sourceforge.pmd.testframework.RuleTst;
 
 /**
@@ -45,18 +44,16 @@ import net.sourceforge.pmd.testframework.RuleTst;
  */
 public class XPathRuleTest extends RuleTst {
 
-    XPathRule rule;
+    private XPathRule makeXPath(String expression) {
-
+        XPathRule rule = new XPathRule(XPathVersion.XPATH_2_0, expression);
-    @Before
-    public void setUp() {
-        rule = new XPathRule();
         rule.setLanguage(LanguageRegistry.getLanguage(JavaLanguageModule.NAME));
         rule.setMessage("XPath Rule Failed");
+        return rule;
     }
 
     @Test
     public void testPluginname() throws Exception {
-        rule.setXPath("//VariableDeclaratorId[string-length(@Image) < 3]");
+        XPathRule rule = makeXPath("//VariableDeclaratorId[string-length(@Image) < 3]");
         rule.setMessage("{0}");
         Report report = getReportForTestString(rule, TEST1);
         RuleViolation rv = report.iterator().next();
@@ -66,15 +63,14 @@ public class XPathRuleTest extends RuleTst {
 
     @Test
     public void testXPathMultiProperty() throws Exception {
-        rule.setXPath("//VariableDeclaratorId[@Image=$forbiddenNames]");
+        XPathRule rule = makeXPath("//VariableDeclaratorId[@Image=$forbiddenNames]");
         rule.setMessage("Avoid vars");
-        rule.setVersion(XPathRuleQuery.XPATH_2_0);
+        PropertyDescriptor<List<String>> varDescriptor
-        StringMultiProperty varDescriptor
+            = PropertyFactory.stringListProperty("forbiddenNames")
-            = StringMultiProperty.named("forbiddenNames")
+                             .desc("Forbidden names")
-                                 .desc("Forbidden names")
+                             .defaultValues("forbid1", "forbid2")
-                                 .defaultValues("forbid1", "forbid2")
+                             .delim('$')
-                                 .delim('$')
+                             .build();
-                                 .build();
 
         rule.definePropertyDescriptor(varDescriptor);
 
@@ -90,9 +86,10 @@ public class XPathRuleTest extends RuleTst {
 
     @Test
     public void testVariables() throws Exception {
-        rule.setXPath("//VariableDeclaratorId[@Image=$var]");
+        XPathRule rule = makeXPath("//VariableDeclaratorId[@Image=$var]");
         rule.setMessage("Avoid vars");
-        StringProperty varDescriptor = new StringProperty("var", "Test var", null, 1.0f);
+        PropertyDescriptor<String> varDescriptor =
+            PropertyFactory.stringProperty("var").desc("Test var").defaultValue("").build();
         rule.definePropertyDescriptor(varDescriptor);
         rule.setProperty(varDescriptor, "fiddle");
         Report report = getReportForTestString(rule, TEST2);
@@ -102,8 +99,7 @@ public class XPathRuleTest extends RuleTst {
 
     @Test
     public void testFnPrefixOnSaxon() throws Exception {
-        rule.setXPath("//VariableDeclaratorId[fn:matches(@Image, 'fiddle')]");
+        XPathRule rule = makeXPath("//VariableDeclaratorId[fn:matches(@Image, 'fiddle')]");
-        rule.setVersion(XPathRuleQuery.XPATH_2_0);
         Report report = getReportForTestString(rule, TEST2);
         RuleViolation rv = report.iterator().next();
         assertEquals(3, rv.getBeginLine());
@@ -111,8 +107,7 @@ public class XPathRuleTest extends RuleTst {
 
     @Test
     public void testNoFnPrefixOnSaxon() throws Exception {
-        rule.setXPath("//VariableDeclaratorId[matches(@Image, 'fiddle')]");
+        XPathRule rule = makeXPath("//VariableDeclaratorId[matches(@Image, 'fiddle')]");
-        rule.setVersion(XPathRuleQuery.XPATH_2_0);
         Report report = getReportForTestString(rule, TEST2);
         RuleViolation rv = report.iterator().next();
         assertEquals(3, rv.getBeginLine());

pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/Ecmascript3Handler.java

@@ -10,9 +10,7 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.ecmascript.ast.DumpFacade;
 import net.sourceforge.pmd.lang.ecmascript.ast.EcmascriptNode;
 import net.sourceforge.pmd.lang.ecmascript.rule.EcmascriptRuleViolationFactory;
@@ -23,11 +21,6 @@ import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
  */
 public class Ecmascript3Handler extends AbstractLanguageVersionHandler {
 
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return EcmascriptRuleViolationFactory.INSTANCE;

pmd-jsp/src/main/java/net/sourceforge/pmd/lang/jsp/JspHandler.java

@@ -10,9 +10,7 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.jsp.ast.DumpFacade;
 import net.sourceforge.pmd.lang.jsp.ast.JspNode;
 import net.sourceforge.pmd.lang.jsp.rule.JspRuleViolationFactory;
@@ -25,11 +23,6 @@ import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
  */
 public class JspHandler extends AbstractLanguageVersionHandler {
 
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return JspRuleViolationFactory.INSTANCE;

pmd-modelica/src/main/java/net/sourceforge/pmd/lang/modelica/ModelicaHandler.java

@@ -8,19 +8,13 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.modelica.ast.ASTStoredDefinition;
 import net.sourceforge.pmd.lang.modelica.resolver.ModelicaSymbolFacade;
 import net.sourceforge.pmd.lang.modelica.rule.ModelicaRuleViolationFactory;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
 
 public class ModelicaHandler extends AbstractLanguageVersionHandler {
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
 
     @Override
     public RuleViolationFactory getRuleViolationFactory() {

pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/PLSQLHandler.java

@@ -11,9 +11,7 @@ import net.sourceforge.pmd.lang.DataFlowHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.dfa.DFAGraphRule;
 import net.sourceforge.pmd.lang.plsql.ast.ASTInput;
 import net.sourceforge.pmd.lang.plsql.ast.DumpFacade;
@@ -83,11 +81,4 @@ public class PLSQLHandler extends AbstractLanguageVersionHandler {
         };
     }
 
-    /**
-     * Return minimal XPathHandler to cope with Jaxen XPath Rules.
-     */
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
 }

pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/PLSQLXPathRuleTest.java

@@ -4,7 +4,7 @@
 
 package net.sourceforge.pmd.lang.plsql;
 
-import java.util.Arrays;
+import static java.util.Collections.singletonList;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -13,21 +13,19 @@ import net.sourceforge.pmd.RuleContext;
 import net.sourceforge.pmd.lang.LanguageRegistry;
 import net.sourceforge.pmd.lang.plsql.ast.ASTInput;
 import net.sourceforge.pmd.lang.rule.XPathRule;
+import net.sourceforge.pmd.lang.rule.xpath.XPathVersion;
 
 /**
  * Tests to use XPath rules with PLSQL.
  */
 public class PLSQLXPathRuleTest extends AbstractPLSQLParserTst {
 
-    private ASTInput node = plsql.parse(
+    private final ASTInput node = plsql.parse(
         "create or replace\n" + "package pkg_xpath_problem\n" + "AS\n" + "    PROCEDURE pkg_minimal\n" + "    IS\n"
             + "        a_variable VARCHAR2(1);\n" + "    BEGIN \n" + "        --PRAGMA INLINE(output,'YES');\n"
             + "        a_variable := 'Y' ;\n" + "    END ;\n" + "end pkg_xpath_problem;\n" + "/\n" + "");
 
-    private RuleContext ctx = new RuleContext();
-
     public PLSQLXPathRuleTest() {
-        ctx.setLanguageVersion(plsql.getDefaultVersion());
     }
 
     /**
@@ -35,10 +33,7 @@ public class PLSQLXPathRuleTest extends AbstractPLSQLParserTst {
      */
     @Test
     public void testXPathRule1() {
-        XPathRule rule = createRule("1.0");
+        testOnVersion(XPathVersion.XPATH_1_0);
-
-        rule.apply(Arrays.asList(node), ctx);
-        Assert.assertEquals(2, ctx.getReport().treeSize());
     }
 
     /**
@@ -46,10 +41,7 @@ public class PLSQLXPathRuleTest extends AbstractPLSQLParserTst {
      */
     @Test
     public void testXPathRule1Compatibility() {
-        XPathRule rule = createRule("1.0 compatibility");
+        testOnVersion(XPathVersion.XPATH_1_0_COMPATIBILITY);
-
-        rule.apply(Arrays.asList(node), ctx);
-        Assert.assertEquals(2, ctx.getReport().treeSize());
     }
 
     /**
@@ -57,18 +49,21 @@ public class PLSQLXPathRuleTest extends AbstractPLSQLParserTst {
      */
     @Test
     public void testXPathRule2() {
-        XPathRule rule = createRule("2.0");
+        testOnVersion(XPathVersion.XPATH_2_0);
-
-        rule.apply(Arrays.asList(node), ctx);
-        Assert.assertEquals(2, ctx.getReport().treeSize());
     }
 
-    private XPathRule createRule(String version) {
+
-        XPathRule rule = new XPathRule("//PrimaryPrefix");
+    private void testOnVersion(XPathVersion xpath10) {
+        XPathRule rule = new XPathRule(xpath10, "//PrimaryPrefix");
         rule.setLanguage(LanguageRegistry.getLanguage(PLSQLLanguageModule.NAME));
-        rule.setVersion(version);
         rule.setMessage("Test Violation");
-        return rule;
+
+        RuleContext ctx = new RuleContext();
+        ctx.setLanguageVersion(plsql.getDefaultVersion());
+
+        rule.apply(singletonList(node), ctx);
+        Assert.assertEquals(2, ctx.getReport().size());
     }
 
+
 }

pmd-scala/src/test/java/net/sourceforge/pmd/lang/scala/rule/XPathRuleTest.java

@@ -6,14 +6,13 @@ package net.sourceforge.pmd.lang.scala.rule;
 
 import static org.junit.Assert.assertEquals;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import net.sourceforge.pmd.Report;
 import net.sourceforge.pmd.RuleViolation;
 import net.sourceforge.pmd.lang.LanguageRegistry;
 import net.sourceforge.pmd.lang.rule.XPathRule;
-import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery;
+import net.sourceforge.pmd.lang.rule.xpath.XPathVersion;
 import net.sourceforge.pmd.lang.scala.ScalaLanguageModule;
 import net.sourceforge.pmd.lang.scala.ast.BaseScalaTest;
 
@@ -21,22 +20,17 @@ public class XPathRuleTest extends BaseScalaTest {
 
     private static final String SCALA_TEST = "/parserFiles/helloworld.scala";
 
-    XPathRule rule;
-
-    @Before
-    public void setUp() {
-        rule = new XPathRule();
-        rule.setLanguage(LanguageRegistry.getLanguage(ScalaLanguageModule.NAME));
-        rule.setMessage("XPath Rule Failed");
-    }
-
     @Test
     public void testPrintHelloWorld() {
-        String xpath = "//TermApply/TermName[@Image=\"println\"]";
+        Report report = evaluate(SCALA_TEST, "//TermApply/TermName[@Image=\"println\"]");
-        rule.setXPath(xpath);
-        rule.setVersion(XPathRuleQuery.XPATH_2_0);
-        Report report = scala.getReportForResource(rule, SCALA_TEST);
         RuleViolation rv = report.iterator().next();
         assertEquals(2, rv.getBeginLine());
     }
+
+    private Report evaluate(String testSource, String xpath) {
+        XPathRule rule = new XPathRule(XPathVersion.XPATH_2_0, xpath);
+        rule.setLanguage(LanguageRegistry.getLanguage(ScalaLanguageModule.NAME));
+        rule.setMessage("XPath Rule Failed");
+        return scala.getReportForResource(rule, testSource);
+    }
 }

pmd-visualforce/src/main/java/net/sourceforge/pmd/lang/vf/VfHandler.java

@@ -10,9 +10,7 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
 import net.sourceforge.pmd.lang.vf.ast.DumpFacade;
 import net.sourceforge.pmd.lang.vf.ast.VfNode;
@@ -20,11 +18,6 @@ import net.sourceforge.pmd.lang.vf.rule.VfRuleViolationFactory;
 
 public class VfHandler extends AbstractLanguageVersionHandler {
 
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return VfRuleViolationFactory.INSTANCE;

pmd-vm/src/main/java/net/sourceforge/pmd/lang/vm/VmHandler.java

@@ -10,9 +10,7 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
 import net.sourceforge.pmd.lang.vm.ast.AbstractVmNode;
 import net.sourceforge.pmd.lang.vm.rule.VmRuleViolationFactory;
@@ -23,11 +21,6 @@ import net.sourceforge.pmd.lang.vm.rule.VmRuleViolationFactory;
  */
 public class VmHandler extends AbstractLanguageVersionHandler {
 
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return VmRuleViolationFactory.INSTANCE;

pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/XmlHandler.java

@@ -10,9 +10,7 @@ import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler;
 import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.VisitorStarter;
-import net.sourceforge.pmd.lang.XPathHandler;
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
 import net.sourceforge.pmd.lang.xml.ast.DumpFacade;
 import net.sourceforge.pmd.lang.xml.ast.XmlNode;
@@ -23,11 +21,6 @@ import net.sourceforge.pmd.lang.xml.rule.XmlRuleViolationFactory;
  */
 public class XmlHandler extends AbstractLanguageVersionHandler {
 
-    @Override
-    public XPathHandler getXPathHandler() {
-        return new DefaultASTXPathHandler();
-    }
-
     @Override
     public RuleViolationFactory getRuleViolationFactory() {
         return XmlRuleViolationFactory.INSTANCE;