Make NodeList NOT a Node. (#522)

* Make NodeList NOT a Node.
* Update branch and fix bad PR #527

javaparser-core/src/main/java/com/github/javaparser/CommentsInserter.java

@@ -60,7 +60,7 @@ private void insertComments(CompilationUnit cu, TreeSet<Comment> comments) {
         // so I could use some heuristics in these cases to distinguish the two
         // cases
 
-        List<Node> children = cu.getBackwardsCompatibleChildrenNodes();
+        List<Node> children = cu.getChildNodes();
 
         Comment firstComment = comments.iterator().next();
         if (cu.getPackage() != null
@@ -91,7 +91,7 @@ void insertComments(Node node, TreeSet<Comment> commentsToAttribute) {
         // if they preceed a child they are assigned to it, otherweise they
         // remain "orphans"
 
-        List<Node> children = node.getBackwardsCompatibleChildrenNodes();
+        List<Node> children = node.getChildNodes();
 
         for (Node child : children) {
             TreeSet<Comment> commentsInsideChild = new TreeSet<>(NODE_BY_BEGIN_POSITION);

javaparser-core/src/main/java/com/github/javaparser/HasParentNode.java

@@ -0,0 +1,49 @@
+package com.github.javaparser;
+
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.NodeList;
+
+import java.util.List;
+
+/**
+ * An object that has a parent node.
+ */
+public interface HasParentNode<T> {
+    // TODO nullable
+    Node getParentNode();
+
+    T setParentNode(Node parentNode);
+
+    /**
+     * "this" for everything except NodeLists. NodeLists use their parent as their childrens parent.
+     */
+    Node getParentNodeForChildren();
+
+    @SuppressWarnings("unchecked")
+    default <N> N getParentNodeOfType(Class<N> classType) {
+        Node parent = getParentNode();
+        while (parent != null) {
+            if (classType.isAssignableFrom(parent.getClass()))
+                return (N) parent;
+            parent = parent.getParentNode();
+        }
+        return null;
+    }
+
+    // TODO should become protected once Java lets us
+    default void setAsParentNodeOf(List<? extends Node> childNodes) {
+        if (childNodes != null) {
+            for (HasParentNode current : childNodes) {
+                current.setParentNode(getParentNodeForChildren());
+            }
+        }
+    }
+
+    // TODO should become protected once Java lets us
+    default void setAsParentNodeOf(Node childNode) {
+        if (childNode != null) {
+            childNode.setParentNode(getParentNodeForChildren());
+        }
+    }
+
+}

javaparser-core/src/main/java/com/github/javaparser/ast/Node.java

@@ -29,13 +29,13 @@
 import java.util.List;
 import java.util.Optional;
 
+import com.github.javaparser.HasParentNode;
 import com.github.javaparser.Position;
 import com.github.javaparser.Range;
 import com.github.javaparser.ast.comments.BlockComment;
 import com.github.javaparser.ast.comments.Comment;
 import com.github.javaparser.ast.comments.LineComment;
 import com.github.javaparser.ast.visitor.*;
-import com.github.javaparser.utils.PositionUtils;
 
 import java.util.*;
 
@@ -50,7 +50,8 @@
  * 
  * @author Julio Vilmar Gesser
  */
-public abstract class Node implements Cloneable {
+// Use <Node> to prevent Node from becoming generic.
+public abstract class Node implements Cloneable, HasParentNode<Node>, Visitable {
     /**
      * This can be used to sort nodes on position.
      */
@@ -70,34 +71,7 @@ public abstract class Node implements Cloneable {
     public Node(Range range) {
         this.range = range;
     }
-
-    /**
-     * Accept method for visitor support.
-     * 
-     * @param <R>
-     *            the type the return value of the visitor
-     * @param <A>
-     *            the type the argument passed to the visitor
-     * @param v
-     *            the visitor implementation
-     * @param arg
-     *            the argument passed to the visitor
-     * @return the result of the visit
-     */
-    public abstract <R, A> R accept(GenericVisitor<R, A> v, A arg);
-
-    /**
-     * Accept method for visitor support.
-     * 
-     * @param <A>
-     *            the type the argument passed for the visitor
-     * @param v
-     *            the visitor implementation
-     * @param arg
-     *            any value relevant for the visitor
-     */
-    public abstract <A> void accept(VoidVisitor<A> v, A arg);
-
+
     /**
      * This is a comment associated with this node.
      *
@@ -222,24 +196,14 @@ public boolean equals(final Object obj) {
 
     @Override
     public Node clone() {
-        return this.accept(new CloneVisitor(), null);
+        return (Node) accept(new CloneVisitor(), null);
     }
 
+    @Override
     public Node getParentNode() {
         return parentNode;
     }
 
-    @SuppressWarnings("unchecked")
-    public <T> T getParentNodeOfType(Class<T> classType) {
-        Node parent = parentNode;
-        while (parent != null) {
-            if (classType.isAssignableFrom(parent.getClass()))
-                return (T) parent;
-            parent = parent.parentNode;
-        }
-        return null;
-    }
-
     /**
      * Contains all nodes that have this node set as their parent.
      * You can add nodes to it by setting a node's parent to this node.
@@ -251,30 +215,6 @@ public List<Node> getChildNodes() {
         return unmodifiableList(childrenNodes);
     }
 
-    /**
-     * Before 3.0.0.alpha-5, if we had a list of nodes, those nodes would not have the list
-     * as its parent, but the node containing the list.
-     * This method returns the children in that way: there are no lists, and all nodes that are
-     * in lists are directly in this list.
-     * @deprecated this will be gone in 3.0.0 release.
-     */
-    @Deprecated
-    public List<Node> getBackwardsCompatibleChildrenNodes() {
-        List<Node> children = new ArrayList<>();
-        for (Node childNode : getChildNodes()) {
-            // Avoid attributing comments to NodeLists by pretending they don't exist.
-            if (childNode instanceof NodeList) {
-                for (Node subChildNode : ((NodeList<Node>) childNode)) {
-                    children.add(subChildNode);
-                }
-            } else {
-                children.add(childNode);
-            }
-        }
-        PositionUtils.sortByBeginPosition(children);
-        return children;
-    }
-
     public <N extends Node> boolean containsWithin(N other) {
         return range.contains(other.getRange());
     }
@@ -327,7 +267,8 @@ public List<Comment> getAllContainedComments() {
      *
      * @param parentNode node to be set as parent
      */
-    public void setParentNode(Node parentNode) {
+    @Override
+    public Node setParentNode(Node parentNode) {
         // remove from old parent, if any
         if (this.parentNode != null) {
             this.parentNode.childrenNodes.remove(this);
@@ -337,20 +278,7 @@ public void setParentNode(Node parentNode) {
         if (this.parentNode != null) {
             this.parentNode.childrenNodes.add(this);
         }
-    }
-
-    protected void setAsParentNodeOf(List<? extends Node> childNodes) {
-        if (childNodes != null) {
-            for (Node current : childNodes) {
-                current.setParentNode(this);
-            }
-        }
-    }
-
-    protected void setAsParentNodeOf(Node childNode) {
-        if (childNode != null) {
-            childNode.setParentNode(this);
-        }
+        return this;
     }
 
     public static final int ABSOLUTE_BEGIN_LINE = -1;
@@ -474,4 +402,15 @@ public boolean remove() {
         setParentNode(null);
         return success;
     }
+
+    @Override
+    public Node getParentNodeForChildren() {
+        return this;
+    }
+
+    protected void setAsParentNodeOf(NodeList<? extends Node> list) {
+        if (list != null) {
+            list.setParentNode(getParentNodeForChildren());
+        }
+    }
 }

javaparser-core/src/main/java/com/github/javaparser/ast/NodeList.java

@@ -1,32 +1,28 @@
 package com.github.javaparser.ast;
 
-import com.github.javaparser.Position;
-import com.github.javaparser.Range;
+import com.github.javaparser.HasParentNode;
+import com.github.javaparser.ast.visitor.Visitable;
 import com.github.javaparser.ast.visitor.GenericVisitor;
 import com.github.javaparser.ast.visitor.VoidVisitor;
 
 import java.util.*;
 import java.util.stream.Stream;
 
 /**
- * A node that is a list of nodes.
+ * A list of nodes.
  *
  * @param <N> the type of nodes contained.
  */
-public class NodeList<N extends Node> extends Node implements Iterable<N> {
-    // TODO we probably want to use the already existing childrenNodes list for this.
+public class NodeList<N extends Node> implements Iterable<N>, HasParentNode<NodeList<N>>, Visitable {
     private List<N> innerList = new ArrayList<>(0);
 
+    private Node parentNode;
+
     public NodeList() {
-        this(Range.UNKNOWN, null);
+        this(null);
     }
 
     public NodeList(Node parent) {
-        this(Range.UNKNOWN, parent);
-    }
-
-    public NodeList(Range range, Node parent) {
-        super(range);
         setParentNode(parent);
     }
 
@@ -41,23 +37,6 @@ private void own(N node) {
             return;
         }
         setAsParentNodeOf(node);
-        // Expand the NodeList's range to include the new node.
-        if (getRange().equals(Range.UNKNOWN)) {
-            setRange(node.getRange());
-        } else {
-            Position nodeBegin = node.getBegin();
-            if (nodeBegin.valid()) {
-                if(nodeBegin.isBefore(getBegin())){
-                    setBegin(nodeBegin);
-                }
-            }
-            Position nodeEnd = node.getEnd();
-            if (nodeEnd.valid()) {
-                if(nodeEnd.isAfter(getEnd())){
-                    setEnd(nodeEnd);
-                }
-            }
-        }
     }
 
     public boolean remove(Node node) {
@@ -98,16 +77,6 @@ public Stream<N> stream() {
         return innerList.stream();
     }
 
-    @Override
-    public <R, A> R accept(final GenericVisitor<R, A> v, final A arg) {
-        return v.visit(this, arg);
-    }
-
-    @Override
-    public <A> void accept(final VoidVisitor<A> v, final A arg) {
-        v.visit(this, arg);
-    }
-
     public int size() {
         return innerList.size();
     }
@@ -152,4 +121,30 @@ public NodeList<N> add(int index, N node) {
         innerList.add(index, node);
         return this;
     }
+
+    @Override
+    public Node getParentNode() {
+        return parentNode;
+    }
+
+    @Override
+    public NodeList<N> setParentNode(Node parentNode) {
+        this.parentNode = parentNode;
+        setAsParentNodeOf(innerList);
+        return this;
+    }
+
+    @Override
+    public Node getParentNodeForChildren() {
+        return parentNode;
+    }
+
+    public <R, A> R accept(final GenericVisitor<R, A> v, final A arg) {
+        return v.visit(this, arg);
+    }
+
+    public <A> void accept(final VoidVisitor<A> v, final A arg) {
+        v.visit(this, arg);
+    }
+
 }