Merge pull request #1800 from oowekyala/xml-unimplement-dom

[xml] Unimplement org.w3c.dom.Node from the XmlNodeWrapper

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

@@ -1,21 +0,0 @@
-/**
- * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
- */
-
-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-xml/src/main/java/net/sourceforge/pmd/lang/xml/XmlParser.java

@@ -13,6 +13,7 @@ import net.sourceforge.pmd.lang.ParserOptions;
 import net.sourceforge.pmd.lang.TokenManager;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.ParseException;
+import net.sourceforge.pmd.lang.xml.ast.internal.XmlParserImpl;
 
 /**
  * Adapter for the XmlParser.
@@ -35,7 +36,7 @@ public class XmlParser extends AbstractParser {
 
     @Override
     public Node parse(String fileName, Reader source) throws ParseException {
-        return new net.sourceforge.pmd.lang.xml.ast.XmlParser((XmlParserOptions) parserOptions).parse(source);
+        return new XmlParserImpl((XmlParserOptions) parserOptions).parse(source);
     }
 
     @Override

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

@@ -1,257 +0,0 @@
-/**
- * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
- */
-
-package net.sourceforge.pmd.lang.xml.ast;
-
-import org.w3c.dom.DOMException;
-import org.w3c.dom.Document;
-import org.w3c.dom.NamedNodeMap;
-import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;
-import org.w3c.dom.UserDataHandler;
-
-import net.sourceforge.pmd.lang.ast.AbstractNode;
-
-
-/**
- * Moves boilerplate out of {@link XmlNodeWrapper}.
- *
- * @author Clément Fournier
- * @since 6.1.0
- */
-public abstract class AbstractDomNodeProxy extends AbstractNode implements org.w3c.dom.Node {
-
-    protected final org.w3c.dom.Node node;
-
-
-    protected AbstractDomNodeProxy(Node node) {
-        super(0);
-        this.node = node;
-    }
-
-
-    @Override
-    public String getNodeName() {
-        return node.getNodeName();
-    }
-
-
-    @Override
-    public String getNodeValue() throws DOMException {
-        return node.getNodeValue();
-    }
-
-
-    @Override
-    public void setNodeValue(String nodeValue) throws DOMException {
-        node.setNodeValue(nodeValue);
-    }
-
-
-    @SuppressWarnings("PMD.AvoidUsingShortType")
-    @Override
-    public short getNodeType() {
-        return node.getNodeType();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node getParentNode() {
-        return node.getParentNode();
-    }
-
-
-    @Override
-    public NodeList getChildNodes() {
-        return node.getChildNodes();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node getFirstChild() {
-        return node.getFirstChild();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node getLastChild() {
-        return node.getLastChild();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node getPreviousSibling() {
-        return node.getPreviousSibling();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node getNextSibling() {
-        return node.getNextSibling();
-    }
-
-
-    @Override
-    public NamedNodeMap getAttributes() {
-        return node.getAttributes();
-    }
-
-
-    @Override
-    public Document getOwnerDocument() {
-        return node.getOwnerDocument();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node insertBefore(org.w3c.dom.Node newChild, org.w3c.dom.Node refChild) throws DOMException {
-        return node.insertBefore(newChild, refChild);
-    }
-
-
-    @Override
-    public org.w3c.dom.Node replaceChild(org.w3c.dom.Node newChild, org.w3c.dom.Node oldChild) throws DOMException {
-        return node.replaceChild(newChild, oldChild);
-    }
-
-
-    @Override
-    public org.w3c.dom.Node removeChild(org.w3c.dom.Node oldChild) throws DOMException {
-        return node.removeChild(oldChild);
-    }
-
-
-    @Override
-    public org.w3c.dom.Node appendChild(org.w3c.dom.Node newChild) throws DOMException {
-        return node.appendChild(newChild);
-    }
-
-
-    @Override
-    public boolean hasChildNodes() {
-        return node.hasChildNodes();
-    }
-
-
-    @Override
-    public org.w3c.dom.Node cloneNode(boolean deep) {
-        return node.cloneNode(deep);
-    }
-
-
-    @Override
-    public void normalize() {
-        node.normalize();
-    }
-
-
-    @Override
-    public boolean isSupported(String feature, String version) {
-        return node.isSupported(feature, version);
-    }
-
-
-    @Override
-    public String getNamespaceURI() {
-        return node.getNamespaceURI();
-    }
-
-
-    @Override
-    public String getPrefix() {
-        return node.getPrefix();
-    }
-
-
-    @Override
-    public void setPrefix(String prefix) throws DOMException {
-        node.setPrefix(prefix);
-    }
-
-
-    @Override
-    public String getLocalName() {
-        return node.getLocalName();
-    }
-
-
-    @Override
-    public boolean hasAttributes() {
-        return node.hasAttributes();
-    }
-
-
-    @Override
-    public String getBaseURI() {
-        return node.getBaseURI();
-    }
-
-
-    @SuppressWarnings("PMD.AvoidUsingShortType")
-    @Override
-    public short compareDocumentPosition(org.w3c.dom.Node other) throws DOMException {
-        return node.compareDocumentPosition(other);
-    }
-
-
-    @Override
-    public String getTextContent() throws DOMException {
-        return node.getTextContent();
-    }
-
-
-    @Override
-    public void setTextContent(String textContent) throws DOMException {
-        node.setTextContent(textContent);
-    }
-
-
-    @Override
-    public boolean isSameNode(org.w3c.dom.Node other) {
-        return node.isSameNode(other);
-    }
-
-
-    @Override
-    public String lookupPrefix(String namespaceURI) {
-        return node.lookupPrefix(namespaceURI);
-    }
-
-
-    @Override
-    public boolean isDefaultNamespace(String namespaceURI) {
-        return node.isDefaultNamespace(namespaceURI);
-    }
-
-
-    @Override
-    public String lookupNamespaceURI(String prefix) {
-        return node.lookupNamespaceURI(prefix);
-    }
-
-
-    @Override
-    public boolean isEqualNode(org.w3c.dom.Node arg) {
-        return node.isEqualNode(arg);
-    }
-
-
-    @Override
-    public Object getFeature(String feature, String version) {
-        return node.getFeature(feature, version);
-    }
-
-
-    @Override
-    public Object setUserData(String key, Object data, UserDataHandler handler) {
-        return node.setUserData(key, data, handler);
-    }
-
-
-    @Override
-    public Object getUserData(String key) {
-        return node.getUserData(key);
-    }
-
-}

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

@@ -5,17 +5,12 @@
 package net.sourceforge.pmd.lang.xml.ast;
 
 import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.xpath.AttributeNode;
 
 /**
  * This interface represents all XML AST nodes. They are essentially thin
  * wrappers around the underlying DOM nodes.
  */
-public interface XmlNode extends Node, AttributeNode {
-    String BEGIN_LINE = "pmd:beginLine";
-    String BEGIN_COLUMN = "pmd:beginColumn";
-    String END_LINE = "pmd:endLine";
-    String END_COLUMN = "pmd:endColumn";
+public interface XmlNode extends Node {
 
     /**
      * Provide access to the underlying DOM node.

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

@@ -1,43 +1,43 @@
-/**
+/*
  * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
  */
 
-package net.sourceforge.pmd.lang.xml.ast;
+package net.sourceforge.pmd.lang.xml.ast.internal;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.w3c.dom.Document;
 import org.w3c.dom.DocumentType;
 import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;
 import org.w3c.dom.ProcessingInstruction;
 
 import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
+import net.sourceforge.pmd.lang.xml.ast.internal.XmlParserImpl.RootXmlNode;
 
 /**
  *
  */
 class DOMLineNumbers {
-    private final Document document;
+    private final RootXmlNode document;
     private String xmlString;
     private SourceCodePositioner sourceCodePositioner;
 
-    DOMLineNumbers(Document document, String xmlString) {
-        this.document = document;
+    DOMLineNumbers(RootXmlNode root, String xmlString) {
+        this.document = root;
         this.xmlString = xmlString;
         this.sourceCodePositioner = new SourceCodePositioner(xmlString);
     }
 
-    public void determine() {
+    void determine() {
         determineLocation(document, 0);
     }
 
-    private int determineLocation(Node n, int index) {
+    private int determineLocation(XmlNodeWrapper wrapper, int index) {
         int nextIndex = index;
         int nodeLength = 0;
         int textLength = 0;
+        org.w3c.dom.Node n = wrapper.getNode();
         if (n.getNodeType() == Node.DOCUMENT_TYPE_NODE) {
             nextIndex = xmlString.indexOf("<!DOCTYPE", nextIndex);
         } else if (n.getNodeType() == Node.COMMENT_NODE) {
@@ -65,14 +65,14 @@ class DOMLineNumbers {
         } else if (n.getNodeType() == Node.ENTITY_REFERENCE_NODE) {
             nextIndex = xmlString.indexOf("&" + n.getNodeName() + ";", nextIndex);
         }
-        setBeginLocation(n, nextIndex);
+        setBeginLocation(wrapper, nextIndex);
 
         nextIndex += nodeLength;
 
         if (n.hasChildNodes()) {
-            NodeList childs = n.getChildNodes();
-            for (int i = 0; i < childs.getLength(); i++) {
-                nextIndex = determineLocation(childs.item(i), nextIndex);
+            int numChildren = wrapper.jjtGetNumChildren();
+            for (int i = 0; i < numChildren; i++) {
+                nextIndex = determineLocation((XmlNodeWrapper) wrapper.jjtGetChild(i), nextIndex);
             }
         }
 
@@ -104,7 +104,7 @@ class DOMLineNumbers {
             ProcessingInstruction pi = (ProcessingInstruction) n;
             nextIndex += "<?".length() + pi.getTarget().length() + "?>".length() + pi.getData().length();
         }
-        setEndLocation(n, nextIndex - 1);
+        setEndLocation(wrapper, nextIndex - 1);
         return nextIndex;
     }
 
@@ -146,21 +146,21 @@ class DOMLineNumbers {
         return result;
     }
 
-    private void setBeginLocation(Node n, int index) {
+    private void setBeginLocation(XmlNodeWrapper n, int index) {
         if (n != null) {
             int line = sourceCodePositioner.lineNumberFromOffset(index);
             int column = sourceCodePositioner.columnFromOffset(line, index);
-            n.setUserData(XmlNode.BEGIN_LINE, line, null);
-            n.setUserData(XmlNode.BEGIN_COLUMN, column, null);
+            n.setBeginLine(line);
+            n.setBeginColumn(column);
         }
     }
 
-    private void setEndLocation(Node n, int index) {
+    private void setEndLocation(XmlNodeWrapper n, int index) {
         if (n != null) {
             int line = sourceCodePositioner.lineNumberFromOffset(index);
             int column = sourceCodePositioner.columnFromOffset(line, index);
-            n.setUserData(XmlNode.END_LINE, line, null);
-            n.setUserData(XmlNode.END_COLUMN, column, null);
+            n.setEndLine(line);
+            n.setEndColumn(column);
         }
     }
 }

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

@@ -1,8 +1,8 @@
-/**
+/*
  * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
  */
 
-package net.sourceforge.pmd.lang.xml.ast;
+package net.sourceforge.pmd.lang.xml.ast.internal;
 
 
 import java.util.ArrayList;
@@ -16,9 +16,11 @@ import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.NodeList;
 import org.w3c.dom.Text;
 
+import net.sourceforge.pmd.lang.ast.AbstractNode;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
 import net.sourceforge.pmd.lang.dfa.DataFlowNode;
+import net.sourceforge.pmd.lang.xml.ast.XmlNode;
 import net.sourceforge.pmd.util.CompoundIterator;
 
 
@@ -28,14 +30,16 @@ import net.sourceforge.pmd.util.CompoundIterator;
  * @author Clément Fournier
  * @since 6.1.0
  */
-public class XmlNodeWrapper extends AbstractDomNodeProxy implements XmlNode {
+class XmlNodeWrapper extends AbstractNode implements XmlNode {
 
-    private final XmlParser parser;
+    private final XmlParserImpl parser;
     private Object userData;
+    private final org.w3c.dom.Node node;
 
 
-    public XmlNodeWrapper(XmlParser parser, org.w3c.dom.Node domNode) {
-        super(domNode);
+    XmlNodeWrapper(XmlParserImpl parser, org.w3c.dom.Node domNode) {
+        super(0);
+        this.node = domNode;
         this.parser = parser;
     }
 
@@ -120,30 +124,6 @@ public class XmlNodeWrapper extends AbstractDomNodeProxy implements XmlNode {
     }
 
 
-    @Override
-    public int getBeginLine() {
-        return (int) getUserData(BEGIN_LINE);
-    }
-
-
-    @Override
-    public int getBeginColumn() {
-        return (int) getUserData(BEGIN_COLUMN);
-    }
-
-
-    @Override
-    public int getEndLine() {
-        return (int) getUserData(END_LINE);
-    }
-
-
-    @Override
-    public int getEndColumn() {
-        return (int) getUserData(END_COLUMN);
-    }
-
-
     @Override
     public DataFlowNode getDataFlowNode() {
         throw new UnsupportedOperationException();
@@ -249,18 +229,28 @@ 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;
     }
+
+
+    // package private, open only to DOMLineNumbers
+
+    void setBeginLine(int i) {
+        this.beginLine = i;
+    }
+
+    void setBeginColumn(int i) {
+        this.beginColumn = i;
+    }
+
+    void setEndLine(int i) {
+        this.endLine = i;
+    }
+
+    void setEndColumn(int i) {
+        this.endColumn = i;
+    }
+
 }

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

@@ -1,8 +1,8 @@
-/**
+/*
  * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
  */
 
-package net.sourceforge.pmd.lang.xml.ast;
+package net.sourceforge.pmd.lang.xml.ast.internal;
 
 import java.io.IOException;
 import java.io.Reader;
@@ -22,22 +22,23 @@ import org.xml.sax.SAXException;
 import net.sourceforge.pmd.lang.ast.ParseException;
 import net.sourceforge.pmd.lang.ast.RootNode;
 import net.sourceforge.pmd.lang.xml.XmlParserOptions;
+import net.sourceforge.pmd.lang.xml.ast.XmlNode;
 
 
-public class XmlParser {
-    protected final XmlParserOptions parserOptions;
-    protected Map<org.w3c.dom.Node, XmlNode> nodeCache = new HashMap<>();
+public class XmlParserImpl {
+
+    private final XmlParserOptions parserOptions;
+    private Map<org.w3c.dom.Node, XmlNode> nodeCache = new HashMap<>();
 
 
-    public XmlParser(XmlParserOptions parserOptions) {
+    public XmlParserImpl(XmlParserOptions parserOptions) {
         this.parserOptions = parserOptions;
     }
 
 
-    protected Document parseDocument(Reader reader) throws ParseException {
+    private Document parseDocument(String xmlData) throws ParseException {
         nodeCache.clear();
         try {
-            String xmlData = IOUtils.toString(reader);
 
             DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
             dbf.setNamespaceAware(parserOptions.isNamespaceAware());
@@ -51,10 +52,7 @@ public class XmlParser {
             dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
             DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
             documentBuilder.setEntityResolver(parserOptions.getEntityResolver());
-            Document document = documentBuilder.parse(new InputSource(new StringReader(xmlData)));
-            DOMLineNumbers lineNumbers = new DOMLineNumbers(document, xmlData);
-            lineNumbers.determine();
-            return document;
+            return documentBuilder.parse(new InputSource(new StringReader(xmlData)));
         } catch (ParserConfigurationException | SAXException | IOException e) {
             throw new ParseException(e);
         }
@@ -62,8 +60,16 @@ public class XmlParser {
 
 
     public XmlNode parse(Reader reader) {
-        Document document = parseDocument(reader);
-        XmlNode root = new RootXmlNode(this, document);
+        String xmlData;
+        try {
+            xmlData = IOUtils.toString(reader);
+        } catch (IOException e) {
+            throw new ParseException(e);
+        }
+        Document document = parseDocument(xmlData);
+        RootXmlNode root = new RootXmlNode(this, document);
+        DOMLineNumbers lineNumbers = new DOMLineNumbers(root, xmlData);
+        lineNumbers.determine();
         nodeCache.put(document, root);
         return root;
     }
@@ -89,8 +95,8 @@ public class XmlParser {
     /**
      * The root should implement {@link RootNode}.
      */
-    public static class RootXmlNode extends XmlNodeWrapper implements RootNode {
-        RootXmlNode(XmlParser parser, Node domNode) {
+    static class RootXmlNode extends XmlNodeWrapper implements RootNode {
+        RootXmlNode(XmlParserImpl parser, Node domNode) {
             super(parser, domNode);
         }
     }

pmd-xml/src/test/java/net/sourceforge/pmd/lang/xml/XmlParserTest.java

@@ -24,11 +24,11 @@ import net.sourceforge.pmd.lang.Parser;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.xpath.Attribute;
 import net.sourceforge.pmd.lang.xml.ast.XmlNode;
-import net.sourceforge.pmd.lang.xml.ast.XmlParser;
+import net.sourceforge.pmd.lang.xml.ast.internal.XmlParserImpl;
 import net.sourceforge.pmd.util.StringUtil;
 
 /**
- * Unit test for the {@link XmlParser}.
+ * Unit test for the {@link XmlParserImpl}.
  */
 public class XmlParserTest {