Merge branch 'pr-736'

docs/pages/release_notes.md

@@ -547,7 +547,9 @@ a warning will now be produced suggesting users to adopt it for better performan
 *   [#727](https://github.com/pmd/pmd/pull/727): \[core] Fix #725: numeric property descriptors now check their default value - [Clément Fournier](https://github.com/oowekyala)
 *   [#733](https://github.com/pmd/pmd/pull/733): \[java] Some improvements to CommentRequired - [Clément Fournier](https://github.com/oowekyala)
 *   [#734](https://github.com/pmd/pmd/pull/734): \[java] Move CyclomaticComplexity from metrics to design - [Clément Fournier](https://github.com/oowekyala)
+*   [#736](https://github.com/pmd/pmd/pull/736): \[core] Make Saxon support multi valued XPath properties - [Clément Fournier](https://github.com/oowekyala)
 *   [#737](https://github.com/pmd/pmd/pull/737): \[doc] Fix NPathComplexity documentation bad rendering - [Clément Fournier](https://github.com/oowekyala)
 *   [#744](https://github.com/pmd/pmd/pull/744): \[doc] Added Apex to supported languages - [Michał Kuliński](https://github.com/coola)
 *   [#746](https://github.com/pmd/pmd/pull/746): \[doc] Fix typo in incremental analysis log message - [Clément Fournier](https://github.com/oowekyala)
 *   [#749](https://github.com/pmd/pmd/pull/749): \[doc] Update the documentation for properties - [Clément Fournier](https://github.com/oowekyala)
+

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

@@ -6,8 +6,11 @@ package net.sourceforge.pmd.lang.ast.xpath;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -98,17 +101,20 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
         return new Attribute(node, m.name, m.method);
     }
 
+    private static final Set<Class<?>> CONSIDERED_RETURN_TYPES 
+        = new HashSet<>(Arrays.<Class<?>>asList(Integer.TYPE, Boolean.TYPE, Double.TYPE, String.class, Long.TYPE, Character.TYPE, Float.TYPE));
+    
+    private static final Set<String> FILTERED_OUT_NAMES 
+        = new HashSet<>(Arrays.asList("toString", "getClass", "getTypeNameNode", "hashCode", "getImportedNameNode", "getScope"));
+    
     protected boolean isAttributeAccessor(Method method) {
-
         String methodName = method.getName();
         boolean deprecated = method.getAnnotation(Deprecated.class) != null;
 
         return !deprecated
-                && (Integer.TYPE == method.getReturnType() || Boolean.TYPE == method.getReturnType()
+               && CONSIDERED_RETURN_TYPES.contains(method.getReturnType())
-                        || Double.TYPE == method.getReturnType() || String.class == method.getReturnType())
+               && method.getParameterTypes().length == 0
-                && method.getParameterTypes().length == 0 && Void.TYPE != method.getReturnType()
+               && !methodName.startsWith("jjt")
-                && !methodName.startsWith("jjt") && !"toString".equals(methodName) && !"getScope".equals(methodName)
+               && !FILTERED_OUT_NAMES.contains(methodName);
-                && !"getClass".equals(methodName) && !"getTypeNameNode".equals(methodName)
-                && !"getImportedNameNode".equals(methodName) && !"hashCode".equals(methodName);
     }
 }

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

@@ -5,15 +5,12 @@
 package net.sourceforge.pmd.lang.ast.xpath.saxon;
 
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
+import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
 
 import net.sf.saxon.om.NodeInfo;
 import net.sf.saxon.om.SequenceIterator;
 import net.sf.saxon.trans.XPathException;
 import net.sf.saxon.type.Type;
-import net.sf.saxon.value.BooleanValue;
-import net.sf.saxon.value.EmptySequence;
-import net.sf.saxon.value.Int64Value;
-import net.sf.saxon.value.StringValue;
 import net.sf.saxon.value.Value;
 
 /**
@@ -48,20 +45,7 @@ public class AttributeNode extends AbstractNodeInfo {
     public Value atomize() throws XPathException {
         if (value == null) {
             Object v = attribute.getValue();
-            // TODO Need to handle the full range of types, is there something
+            value = SaxonXPathRuleQuery.getAtomicRepresentation(v);
-            // Saxon can do to help?
-            if (v instanceof String) {
-                value = new StringValue((String) v);
-            } else if (v instanceof Boolean) {
-                value = BooleanValue.get(((Boolean) v).booleanValue());
-            } else if (v instanceof Integer) {
-                value = Int64Value.makeIntegerValue((Integer) v);
-            } else if (v == null) {
-                value = EmptySequence.getInstance();
-            } else {
-                throw new RuntimeException(
-                        "Unable to create ValueRepresentaton for attribute value: " + v + " of type " + v.getClass());
-            }
         }
         return value;
     }

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

@@ -14,12 +14,9 @@ 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.xpath.Initializer;
-import net.sourceforge.pmd.properties.BooleanProperty;
-import net.sourceforge.pmd.properties.EnumeratedProperty;
-import net.sourceforge.pmd.properties.IntegerProperty;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
-import net.sourceforge.pmd.properties.StringProperty;
 
+import net.sf.saxon.om.Item;
 import net.sf.saxon.om.ValueRepresentation;
 import net.sf.saxon.sxpath.AbstractStaticContext;
 import net.sf.saxon.sxpath.IndependentContext;
@@ -29,27 +26,36 @@ import net.sf.saxon.sxpath.XPathExpression;
 import net.sf.saxon.sxpath.XPathStaticContext;
 import net.sf.saxon.sxpath.XPathVariable;
 import net.sf.saxon.trans.XPathException;
+import net.sf.saxon.value.AtomicValue;
+import net.sf.saxon.value.BigIntegerValue;
 import net.sf.saxon.value.BooleanValue;
+import net.sf.saxon.value.DoubleValue;
+import net.sf.saxon.value.EmptySequence;
+import net.sf.saxon.value.FloatValue;
 import net.sf.saxon.value.Int64Value;
+import net.sf.saxon.value.SequenceExtent;
 import net.sf.saxon.value.StringValue;
+import net.sf.saxon.value.UntypedAtomicValue;
+
 
 /**
  * This is a Saxon based XPathRule query.
  */
 public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
 
-    // Mapping from Node name to applicable XPath queries
-    private XPathExpression xpathExpression;
-    private List<XPathVariable> xpathVariables;
-
     private static final int MAX_CACHE_SIZE = 20;
     private static final Map<Node, DocumentNode> CACHE = new LinkedHashMap<Node, DocumentNode>(MAX_CACHE_SIZE) {
         private static final long serialVersionUID = -7653916493967142443L;
 
+
         protected boolean removeEldestEntry(final Map.Entry<Node, DocumentNode> eldest) {
             return size() > MAX_CACHE_SIZE;
         }
     };
+    // Mapping from Node name to applicable XPath queries
+    private XPathExpression xpathExpression;
+    private List<XPathVariable> xpathVariables;
+
 
     /**
      * {@inheritDoc}
@@ -59,6 +65,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         return XPATH_1_0_COMPATIBILITY.equals(version) || XPATH_2_0.equals(version);
     }
 
+
     /**
      * {@inheritDoc}
      */
@@ -83,31 +90,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
                 String name = xpathVariable.getVariableQName().getLocalName();
                 for (Map.Entry<PropertyDescriptor<?>, Object> entry : super.properties.entrySet()) {
                     if (name.equals(entry.getKey().name())) {
-                        PropertyDescriptor<?> propertyDescriptor = entry.getKey();
+                        ValueRepresentation valueRepresentation = getRepresentation(entry.getKey(), entry.getValue());
-                        Object value = entry.getValue();
-                        ValueRepresentation valueRepresentation;
-
-                        // TODO Need to handle null values?
-                        // TODO Need to handle more PropertyDescriptors, is
-                        // there an easy factory in Saxon we can use for this?
-                        if (propertyDescriptor instanceof StringProperty) {
-                            valueRepresentation = new StringValue((String) value);
-                        } else if (propertyDescriptor instanceof BooleanProperty) {
-                            valueRepresentation = BooleanValue.get((Boolean) value);
-                        } else if (propertyDescriptor instanceof IntegerProperty) {
-                            valueRepresentation = Int64Value.makeIntegerValue((Integer) value);
-                        } else if (propertyDescriptor instanceof EnumeratedProperty) {
-                            if (value instanceof String) {
-                                valueRepresentation = new StringValue((String) value);
-                            } else {
-                                throw new RuntimeException(
-                                        "Unable to create ValueRepresentaton for non-String EnumeratedProperty value: "
-                                                + value);
-                            }
-                        } else {
-                            throw new RuntimeException("Unable to create ValueRepresentaton for PropertyDescriptor: "
-                                    + propertyDescriptor);
-                        }
                         xpathDynamicContext.setVariable(xpathVariable, valueRepresentation);
                     }
                 }
@@ -123,6 +106,24 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         return results;
     }
 
+
+    private ValueRepresentation getRepresentation(PropertyDescriptor<?> descriptor, Object value) {
+        if (descriptor.isMultiValue()) {
+            List<?> val = (List<?>) value;
+            if (val.isEmpty()) {
+                return EmptySequence.getInstance();
+            }
+            Item[] converted = new Item[val.size()];
+            for (int i = 0; i < val.size(); i++) {
+                converted[i] = getAtomicRepresentation(val.get(i));
+            }
+            return new SequenceExtent(converted);
+        } else {
+            return getAtomicRepresentation(value);
+        }
+    }
+    
+
     private DocumentNode getDocumentNode(Node node) {
         // Get the root AST node
         Node root = node;
@@ -142,6 +143,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
         return documentNode;
     }
 
+
     private void initializeXPathExpression() {
         if (xpathExpression != null) {
             return;
@@ -178,4 +180,36 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
             throw new RuntimeException(e);
         }
     }
+
+
+    /**
+     * Gets the Saxon representation of the parameter, if its type corresponds 
+     * to an XPath 2.0 atomic datatype.
+     *
+     * @param value The value to convert
+     *
+     * @return The converted AtomicValue
+     */
+    public static AtomicValue getAtomicRepresentation(Object value) {
+        if (value == null) {
+            return UntypedAtomicValue.ZERO_LENGTH_UNTYPED;
+        } else if (value instanceof String) {
+            return new StringValue((String) value);
+        } else if (value instanceof Boolean) {
+            return BooleanValue.get((Boolean) value);
+        } else if (value instanceof Integer) {
+            return Int64Value.makeIntegerValue((Integer) value);
+        } else if (value instanceof Long) {
+            return new BigIntegerValue((Long) value);
+        } else if (value instanceof Double) {
+            return new DoubleValue((Double) value);
+        } else if (value instanceof Character) {
+            return new StringValue(value.toString());
+        } else if (value instanceof Float) {
+            return new FloatValue((Float) value);
+        } else {
+            // We could maybe use UntypedAtomicValue
+            throw new RuntimeException("Unable to create ValueRepresentation for value of type: " + value.getClass());
+        }
+    }
 }

pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorUtil.java

@@ -23,29 +23,29 @@ public class PropertyDescriptorUtil {
     static {
         Map<String, PropertyDescriptorExternalBuilder<?>> temp = new HashMap<>(18);
         temp.put("Boolean", BooleanProperty.extractor());
-        temp.put("List<Boolean>", BooleanMultiProperty.extractor());
+        temp.put("List[Boolean]", BooleanMultiProperty.extractor());
 
         temp.put("String", StringProperty.extractor());
-        temp.put("List<String>", StringMultiProperty.extractor());
+        temp.put("List[String]", StringMultiProperty.extractor());
         temp.put("Character", CharacterProperty.extractor());
-        temp.put("List<Character>", CharacterMultiProperty.extractor());
+        temp.put("List[Character]", CharacterMultiProperty.extractor());
 
 
         temp.put("Integer", IntegerProperty.extractor());
-        temp.put("List<Integer>", IntegerMultiProperty.extractor());
+        temp.put("List[Integer]", IntegerMultiProperty.extractor());
         temp.put("Long", LongProperty.extractor());
-        temp.put("List<Long>", LongMultiProperty.extractor());
+        temp.put("List[Long]", LongMultiProperty.extractor());
         temp.put("Float", FloatProperty.extractor());
-        temp.put("List<Float>", FloatMultiProperty.extractor());
+        temp.put("List[Float]", FloatMultiProperty.extractor());
         temp.put("Double", DoubleProperty.extractor());
-        temp.put("List<Double>", DoubleMultiProperty.extractor());
+        temp.put("List[Double]", DoubleMultiProperty.extractor());
-        //    temp.put("Enum", EnumeratedProperty.FACTORY); // TODO:cf implement that
+        //    temp.put("Enum", EnumeratedProperty.FACTORY); // TODO:cf we need new syntax in the xml to support that
-        //    temp.put("List<Enum>", EnumeratedMultiProperty.FACTORY);
+        //    temp.put("List[Enum]", EnumeratedMultiProperty.FACTORY);
 
         temp.put("Class", TypeProperty.extractor());
-        temp.put("List<Class>", TypeMultiProperty.extractor());
+        temp.put("List[Class]", TypeMultiProperty.extractor());
         temp.put("Method", MethodProperty.extractor());
-        temp.put("List<Method>", MethodMultiProperty.extractor());
+        temp.put("List[Method]", MethodMultiProperty.extractor());
 
         temp.put("File", FileProperty.extractor());
 

pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java

@@ -179,7 +179,7 @@ public class RuleSetFactoryTest {
                                    + "class=\"net.sourceforge.pmd.lang.rule.XPathRule\" language=\"dummy\">\n"
                                    + "         <description>Please move your class to the right folder(rest \nfolder)</description>\n"
                                    + "         <priority>2</priority>\n         <properties>\n             <property name=\"packageRegEx\""
-                                   + " value=\"com.aptsssss|com.abc\" \ntype=\"List&lt;String&gt;\" "
+                                   + " value=\"com.aptsssss|com.abc\" \ntype=\"List[String]\" "
                                    + "description=\"valid packages\"/>\n         </properties></rule></ruleset>");
         PropertyDescriptor<List<String>> prop = (PropertyDescriptor<List<String>>) r.getPropertyDescriptor("packageRegEx");
         List<String> values = r.getProperty(prop);
@@ -194,7 +194,7 @@ public class RuleSetFactoryTest {
                                    + " instead.\" \n" + "class=\"net.sourceforge.pmd.lang.rule.XPathRule\" language=\"dummy\">\n"
                                    + "         <description>Please move your class to the right folder(rest \nfolder)</description>\n"
                                    + "         <priority>2</priority>\n         <properties>\n             <property name=\"packageRegEx\""
-                                   + " value=\"com.aptsssss,com.abc\" \ntype=\"List&lt;String&gt;\" delimiter=\",\" "
+                                   + " value=\"com.aptsssss,com.abc\" \ntype=\"List[String]\" delimiter=\",\" "
                                    + "description=\"valid packages\"/>\n"
                                    + "         </properties></rule>" + "</ruleset>");
         PropertyDescriptor<List<String>> prop = (PropertyDescriptor<List<String>>) r.getPropertyDescriptor("packageRegEx");

pmd-core/src/test/java/net/sourceforge/pmd/properties/AbstractPropertyDescriptorTester.java

@@ -95,7 +95,7 @@ public abstract class AbstractPropertyDescriptorTester<T> {
 
     @SuppressWarnings("unchecked")
     protected final PropertyDescriptorExternalBuilder<List<T>> getMultiFactory() {
-        return (PropertyDescriptorExternalBuilder<List<T>>) PropertyDescriptorUtil.factoryFor("List<" + typeName + ">");
+        return (PropertyDescriptorExternalBuilder<List<T>>) PropertyDescriptorUtil.factoryFor("List[" + typeName + "]");
     }
 
 

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

@@ -8,13 +8,16 @@ import static org.junit.Assert.assertEquals;
 
 import java.io.StringReader;
 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;
+import net.sourceforge.pmd.PMDException;
 import net.sourceforge.pmd.Report;
+import net.sourceforge.pmd.Rule;
 import net.sourceforge.pmd.RuleContext;
 import net.sourceforge.pmd.RuleSet;
 import net.sourceforge.pmd.RuleSetFactory;
@@ -32,6 +35,7 @@ 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.properties.PropertyDescriptor;
+import net.sourceforge.pmd.properties.StringMultiProperty;
 import net.sourceforge.pmd.properties.StringProperty;
 import net.sourceforge.pmd.testframework.RuleTst;
 
@@ -53,17 +57,36 @@ public class XPathRuleTest extends RuleTst {
     public void testPluginname() throws Exception {
         rule.setXPath("//VariableDeclaratorId[string-length(@Image) < 3]");
         rule.setMessage("{0}");
-        PMD p = new PMD();
+        Report report = getReportForTestString(rule, TEST1);
-        RuleContext ctx = new RuleContext();
-        Report report = new Report();
-        ctx.setReport(report);
-        ctx.setSourceCodeFilename("n/a");
-        RuleSet rules = new RuleSetFactory().createSingleRuleRuleSet(rule);
-        p.getSourceCodeProcessor().processSourceCode(new StringReader(TEST1), new RuleSets(rules), ctx);
         RuleViolation rv = report.iterator().next();
         assertEquals("a", rv.getDescription());
     }
 
+
+    @Test
+    public void testXPathMultiProperty() throws Exception {
+        rule.setXPath("//VariableDeclaratorId[@Image=$forbiddenNames]");
+        rule.setMessage("Avoid vars");
+        rule.setVersion(XPathRuleQuery.XPATH_2_0);
+        StringMultiProperty varDescriptor
+            = StringMultiProperty.named("forbiddenNames")
+                                 .desc("Forbidden names")
+                                 .defaultValues("forbid1", "forbid2")
+                                 .delim('$')
+                                 .build();
+
+        rule.definePropertyDescriptor(varDescriptor);
+
+        Report report = getReportForTestString(rule, TEST3);
+        Iterator<RuleViolation> rv = report.iterator();
+        int i = 0;
+        for (; rv.hasNext(); ++i) {
+            rv.next();
+        }
+        assertEquals(2, i);
+    }
+
+
     @Test
     public void testVariables() throws Exception {
         rule.setXPath("//VariableDeclaratorId[@Image=$var]");
@@ -71,17 +94,12 @@ public class XPathRuleTest extends RuleTst {
         StringProperty varDescriptor = new StringProperty("var", "Test var", null, 1.0f);
         rule.definePropertyDescriptor(varDescriptor);
         rule.setProperty(varDescriptor, "fiddle");
-        PMD p = new PMD();
+        Report report = getReportForTestString(rule, TEST2);
-        RuleContext ctx = new RuleContext();
-        Report report = new Report();
-        ctx.setReport(report);
-        ctx.setSourceCodeFilename("n/a");
-        RuleSet rules = new RuleSetFactory().createSingleRuleRuleSet(rule);
-        p.getSourceCodeProcessor().processSourceCode(new StringReader(TEST2), new RuleSets(rules), ctx);
         RuleViolation rv = report.iterator().next();
         assertEquals(3, rv.getBeginLine());
     }
 
+
     /**
      * Test for problem reported in bug #1219 PrimarySuffix/@Image does not work
      * in some cases in xpath 2.0
@@ -168,8 +186,24 @@ public class XPathRuleTest extends RuleTst {
         assertEquals(5, nodes.get(1).getBeginLine());
     }
 
+    private static Report getReportForTestString(Rule r, String test) throws PMDException {
+        PMD p = new PMD();
+        RuleContext ctx = new RuleContext();
+        Report report = new Report();
+        ctx.setReport(report);
+        ctx.setSourceCodeFilename("n/a");
+        RuleSet rules = new RuleSetFactory().createSingleRuleRuleSet(r);
+        p.getSourceCodeProcessor().processSourceCode(new StringReader(test), new RuleSets(rules), ctx);
+        return report;
+    }
+
+
     private static final String TEST1 = "public class Foo {" + PMD.EOL + " int a;" + PMD.EOL + "}";
 
     private static final String TEST2 = "public class Foo {" + PMD.EOL + " int faddle;" + PMD.EOL + " int fiddle;"
             + PMD.EOL + "}";
+
+
+    private static final String TEST3 = "public class Foo {" + PMD.EOL + " int forbid1; int forbid2; int forbid1$forbid2;" + PMD.EOL + "}";
+
 }