Merge branch 'pr-1179'

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

@@ -21,6 +21,7 @@ import org.w3c.dom.Element;
 
 import net.sourceforge.pmd.PMDVersion;
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
+import net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator;
 import net.sourceforge.pmd.lang.ast.xpath.DocumentNavigator;
 import net.sourceforge.pmd.lang.dfa.DataFlowNode;
 
@@ -515,4 +516,10 @@ public abstract class AbstractNode implements Node {
     public String toString() {
         return getXPathNodeName();
     }
+
+
+    @Override
+    public Iterator<Attribute> getXPathAttributesIterator() {
+        return new AttributeAxisIterator(this);
+    }
 }

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

@@ -5,11 +5,13 @@
 
 package net.sourceforge.pmd.lang.ast;
 
+import java.util.Iterator;
 import java.util.List;
 
 import org.jaxen.JaxenException;
 import org.w3c.dom.Document;
 
+import net.sourceforge.pmd.lang.ast.xpath.Attribute;
 import net.sourceforge.pmd.lang.dfa.DataFlowNode;
 
 /**
@@ -322,4 +324,14 @@ public interface Node {
      * @return The XPath node name
      */
     String getXPathNodeName();
+
+
+    /**
+     * Returns an iterator enumerating all the attributes that are available
+     * from XPath for this node.
+     *
+     * @return An attribute iterator for this node
+     */
+    Iterator<Attribute> getXPathAttributesIterator();
+
 }

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

@@ -14,6 +14,10 @@ import java.util.logging.Logger;
 import net.sourceforge.pmd.lang.ast.Node;
 
 /**
+ * Represents an XPath attribute of a specific node.
+ * Attributes know their name, the node they wrap,
+ * and have access to their value.
+ *
  * @author daniels
  */
 public class Attribute {
@@ -22,20 +26,23 @@ public class Attribute {
     private static final Logger LOG = Logger.getLogger(Attribute.class.getName());
     private static final ConcurrentMap<String, Boolean> DETECTED_DEPRECATED_ATTRIBUTES = new ConcurrentHashMap<>();
 
-
     private static final Object[] EMPTY_OBJ_ARRAY = new Object[0];
+
     private Node parent;
     private String name;
     private Method method;
     private Object value;
     private String stringValue;
 
+    /** Creates a new attribute belonging to the given node using its accessor. */
     public Attribute(Node parent, String name, Method m) {
         this.parent = parent;
         this.name = name;
         this.method = m;
     }
 
+
+    /** Creates a new attribute belonging to the given node using its string value. */
     public Attribute(Node parent, String name, String value) {
         this.parent = parent;
         this.name = name;
@@ -43,6 +50,16 @@ public class Attribute {
         this.stringValue = value;
     }
 
+
+    public String getName() {
+        return name;
+    }
+
+
+    public Node getParent() {
+        return parent;
+    }
+
     public Object getValue() {
         if (value != null) {
             return value;
@@ -71,11 +88,7 @@ public class Attribute {
         if (this.value == null) {
             v = getValue();
         }
-        if (v == null) {
+        stringValue = v == null ? "" : String.valueOf(v);
-            stringValue = "";
-        } else {
-            stringValue = String.valueOf(v);
-        }
         return stringValue;
     }
 
@@ -83,14 +96,6 @@ public class Attribute {
         return parent.getXPathNodeName() + "/@" + name;
     }
 
-    public String getName() {
-        return name;
-    }
-
-    public Node getParent() {
-        return parent;
-    }
-
     @Override
     public String toString() {
         return name + ':' + getValue() + ':' + parent;

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

@@ -16,17 +16,125 @@ import java.util.concurrent.ConcurrentMap;
 
 import net.sourceforge.pmd.lang.ast.Node;
 
+
+/**
+ * Explores an AST node reflectively to iterate over its XPath
+ * attributes. This is the default way the attributes of a node
+ * are made accessible to XPath rules, and defines an important
+ * piece of PMD's XPath support.
+ */
 public class AttributeAxisIterator implements Iterator<Attribute> {
 
+    /** Caches the precomputed attribute accessors of a given class. */
+    private static final ConcurrentMap<Class<?>, MethodWrapper[]> METHOD_CACHE = new ConcurrentHashMap<>();
+
+    /* Constants used to determine which methods are accessors */
+    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", "getXPathNodeName", "getTypeNameNode", "hashCode", "getImportedNameNode", "getScope"));
+
+    /* Iteration variables */
+    private Attribute currObj;
+    private MethodWrapper[] methodWrappers;
+    private int position;
+    private Node node;
+
+
+    /**
+     * Creates a new iterator that enumerates the attributes of the given node.
+     * Note: if you want to access the attributes of a node, don't use this directly,
+     * use instead the overridable {@link Node#getXPathAttributesIterator()}.
+     */
+    public AttributeAxisIterator(Node contextNode) {
+        this.node = contextNode;
+        if (!METHOD_CACHE.containsKey(contextNode.getClass())) {
+            Method[] preFilter = contextNode.getClass().getMethods();
+            List<MethodWrapper> postFilter = new ArrayList<>();
+            for (Method element : preFilter) {
+                if (isAttributeAccessor(element)) {
+                    postFilter.add(new MethodWrapper(element));
+                }
+            }
+            METHOD_CACHE.putIfAbsent(contextNode.getClass(), postFilter.toArray(new MethodWrapper[0]));
+        }
+        this.methodWrappers = METHOD_CACHE.get(contextNode.getClass());
+
+        this.position = 0;
+        this.currObj = getNextAttribute();
+    }
+
+
+    /**
+     * Returns whether the given method is an attribute accessor,
+     * in which case a corresponding Attribute will be added to
+     * the iterator.
+     *
+     * @param method The method to test
+     */
+    protected boolean isAttributeAccessor(Method method) {
+        String methodName = method.getName();
+
+        return CONSIDERED_RETURN_TYPES.contains(method.getReturnType())
+                && method.getParameterTypes().length == 0
+                && !methodName.startsWith("jjt")
+                && !FILTERED_OUT_NAMES.contains(methodName);
+    }
+
+
+    @Override
+    public Attribute next() {
+        if (!hasNext()) {
+            throw new IndexOutOfBoundsException();
+        }
+        Attribute ret = currObj;
+        currObj = getNextAttribute();
+        return ret;
+    }
+
+
+    @Override
+    public boolean hasNext() {
+        return currObj != null;
+    }
+
+
+    @Override
+    public void remove() {
+        throw new UnsupportedOperationException();
+    }
+
+
+    private Attribute getNextAttribute() {
+        if (methodWrappers == null || position == methodWrappers.length) {
+            return null;
+        }
+        MethodWrapper m = methodWrappers[position++];
+        return new Attribute(node, m.name, m.method);
+    }
+
+
+    /**
+     * Associates an attribute accessor with the XPath-accessible
+     * name of the attribute. This is used to avoid recomputing
+     * the name of the attribute for each attribute (it's only done
+     * once and put inside the {@link #METHOD_CACHE}).
+     */
     private static class MethodWrapper {
         public Method method;
         public String name;
 
+
         MethodWrapper(Method m) {
             this.method = m;
             this.name = truncateMethodName(m.getName());
         }
 
+
+        /**
+         * This method produces the actual XPath name of an attribute
+         * from the name of its accessor.
+         */
         private String truncateMethodName(String n) {
             // about 70% of the methods start with 'get', so this case goes
             // first
@@ -46,76 +154,4 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
             return n;
         }
     }
-
-    private Attribute currObj;
-    private MethodWrapper[] methodWrappers;
-    private int position;
-    private Node node;
-
-    private static ConcurrentMap<Class<?>, MethodWrapper[]> methodCache =
-            new ConcurrentHashMap<Class<?>, MethodWrapper[]>();
-
-    public AttributeAxisIterator(Node contextNode) {
-        this.node = contextNode;
-        if (!methodCache.containsKey(contextNode.getClass())) {
-            Method[] preFilter = contextNode.getClass().getMethods();
-            List<MethodWrapper> postFilter = new ArrayList<>();
-            for (Method element : preFilter) {
-                if (isAttributeAccessor(element)) {
-                    postFilter.add(new MethodWrapper(element));
-                }
-            }
-            methodCache.putIfAbsent(contextNode.getClass(), postFilter.toArray(new MethodWrapper[0]));
-        }
-        this.methodWrappers = methodCache.get(contextNode.getClass());
-
-        this.position = 0;
-        this.currObj = getNextAttribute();
-    }
-
-    @Override
-    public Attribute next() {
-        if (currObj == null) {
-            throw new IndexOutOfBoundsException();
-        }
-        Attribute ret = currObj;
-        currObj = getNextAttribute();
-        return ret;
-    }
-
-    @Override
-    public boolean hasNext() {
-        return currObj != null;
-    }
-
-    @Override
-    public void remove() {
-        throw new UnsupportedOperationException();
-    }
-
-    private Attribute getNextAttribute() {
-        if (methodWrappers == null || position == methodWrappers.length) {
-            return null;
-        }
-        MethodWrapper m = methodWrappers[position++];
-        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", "getXPathNodeName", "getTypeNameNode", "hashCode", "getImportedNameNode", "getScope"));
-    
-    protected boolean isAttributeAccessor(Method method) {
-        String methodName = method.getName();
-
-        return CONSIDERED_RETURN_TYPES.contains(method.getReturnType())
-               && method.getParameterTypes().length == 0
-               && !methodName.startsWith("jjt")
-               && !FILTERED_OUT_NAMES.contains(methodName);
-    }
 }

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

@@ -6,11 +6,16 @@ package net.sourceforge.pmd.lang.ast.xpath;
 
 import java.util.Iterator;
 
+
 /**
  * This interface can be used by an AST node to indicate it can directly provide
  * access to it's attributes, versus having them be determined via
  * introspection.
+ *
+ * @deprecated See {@link net.sourceforge.pmd.lang.ast.Node#getXPathAttributesIterator()}.
+ *             Will be removed in 7.0.0
  */
+@Deprecated
 public interface AttributeNode {
     Iterator<Attribute> getAttributeIterator();
 }

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

@@ -135,11 +135,7 @@ public class DocumentNavigator extends DefaultNavigator {
 
     @Override
     public Iterator<Attribute> getAttributeAxisIterator(Object arg0) {
-        if (arg0 instanceof AttributeNode) {
+        return ((Node) arg0).getXPathAttributesIterator();
-            return ((AttributeNode) arg0).getAttributeIterator();
-        } else {
-            return new AttributeAxisIterator((Node) arg0);
-        }
     }
 
     /**

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

@@ -10,13 +10,16 @@ import java.util.NoSuchElementException;
 import net.sourceforge.pmd.lang.ast.Node;
 
 /**
+ * Base class for node iterators used to implement XPath axis
+ * iterators for Jaxen.
+ *
  * @author daniels
  */
 public abstract class NodeIterator implements Iterator<Node> {
 
     private Node node;
 
-    public NodeIterator(Node contextNode) {
+    protected NodeIterator(Node contextNode) {
         this.node = getFirstNode(contextNode);
     }
 

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

@@ -4,18 +4,21 @@
 
 package net.sourceforge.pmd.lang.ast.xpath.saxon;
 
+import java.util.Iterator;
+
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
 
 import net.sf.saxon.om.Navigator;
 import net.sf.saxon.om.SequenceIterator;
 
 /**
- * This is an Attribute axis iterator.
+ * An adapter over our {@link net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator}
+ * for the Saxon model.
  */
 public class AttributeAxisIterator extends Navigator.BaseEnumeration {
 
     protected final ElementNode startNodeInfo;
-    protected final net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator iterator;
+    protected final Iterator<Attribute> iterator;
 
     /**
      * Create an iterator over the Attribute axis for the given ElementNode.
@@ -24,7 +27,7 @@ public class AttributeAxisIterator extends Navigator.BaseEnumeration {
      */
     public AttributeAxisIterator(ElementNode startNodeInfo) {
         this.startNodeInfo = startNodeInfo;
-        this.iterator = new net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator(startNodeInfo.node);
+        this.iterator = startNodeInfo.node.getXPathAttributesIterator();
     }
 
     @Override

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

@@ -15,12 +15,20 @@ import net.sf.saxon.value.Value;
 
 /**
  * A Saxon OM Attribute node for an AST Node Attribute.
+ * Belongs to an {@link ElementNode}, and wraps an
+ * {@link Attribute}.
  */
 public class AttributeNode extends AbstractNodeInfo {
     protected final Attribute attribute;
     protected final int id;
     protected Value value;
 
+
+    /**
+     * Creates a new AttributeNode from a PMD Attribute.
+     *
+     * @param id The index within the attribute order
+     */
     public AttributeNode(Attribute attribute, int id) {
         this.attribute = attribute;
         this.id = id;

pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java

@@ -6,6 +6,7 @@ package net.sourceforge.pmd.util.fxdesigner;
 
 import java.net.URL;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.Objects;
 import java.util.ResourceBundle;
 
@@ -13,7 +14,6 @@ import org.reactfx.EventStreams;
 
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
-import net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator;
 import net.sourceforge.pmd.lang.java.ast.TypeNode;
 import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
 import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator;
@@ -138,9 +138,10 @@ public class NodeInfoPanelController implements Initializable {
      */
     private static ObservableList<String> getAttributes(Node node) {
         ObservableList<String> result = FXCollections.observableArrayList();
-        AttributeAxisIterator attributeAxisIterator = new AttributeAxisIterator(node);
+        Iterator<Attribute> attributeAxisIterator = node.getXPathAttributesIterator();
         while (attributeAxisIterator.hasNext()) {
             Attribute attribute = attributeAxisIterator.next();
+            // TODO the display should be handled in a ListCell
             result.add(attribute.getName() + " = "
                                + ((attribute.getValue() != null) ? attribute.getStringValue() : "null"));
         }

pmd-xml/src/main/java/net/sourceforge/pmd/lang/xml/ast/XmlNodeWrapper.java

@@ -204,7 +204,7 @@ public class XmlNodeWrapper extends AbstractDomNodeProxy implements XmlNode {
 
 
     @Override
-    public Iterator<Attribute> getAttributeIterator() {
+    public Iterator<Attribute> getXPathAttributesIterator() {
         List<Iterator<Attribute>> iterators = new ArrayList<>();
 
         // Expose DOM Attributes
@@ -222,7 +222,7 @@ public class XmlNodeWrapper extends AbstractDomNodeProxy implements XmlNode {
             @Override
             public Attribute next() {
                 org.w3c.dom.Node attributeNode = attributes.item(index++);
-                return new Attribute(parser.wrapDomNode(node),
+                return new Attribute(XmlNodeWrapper.this,
                                      attributeNode.getNodeName(),
                                      attributeNode.getNodeValue());
             }
@@ -249,6 +249,16 @@ public class XmlNodeWrapper extends AbstractDomNodeProxy implements XmlNode {
     }
 
 
+    /**
+     * @deprecated use {@link #getXPathAttributesIterator()}
+     */
+    @Override
+    @Deprecated
+    public Iterator<Attribute> getAttributeIterator() {
+        return getXPathAttributesIterator();
+    }
+
+
     @Override
     public org.w3c.dom.Node getNode() {
         return node;