[fix] Introduce intermediate virtual nodes for select concrete nodes (#…

…133)

src/main/java/se/kth/spork/base3dm/ChangeSet.java

@@ -196,18 +196,18 @@ private void add(Set<Pcs<T>> tree, Function<T, V> getContent) {
                 addToLookupTable(classRepPred, classRepPcs, predecessors);
                 addToLookupTable(classRepSucc, classRepPcs, successors);
             }
-            if (!pred.isListEdge()) {
+            if (!pred.isVirtual()) {
                 Content<T,V> c = new Content<T,V>(pcs, getContent.apply(pred));
                 addToLookupTable(classRepPred, c, content);
             }
         }
     }
 
     private Pcs<T> addToStar(Pcs<T> pcs) {
-        T root = pcs.getRoot();
-        T pred = pcs.getPredecessor();
-        T succ = pcs.getSuccessor();
-        Pcs<T> classRepPcs = new Pcs<T>(classRepMap.get(root), classRepMap.get(pred), classRepMap.get(succ), pcs.getRevision());
+        T root = classRepMap.get(pcs.getRoot());
+        T pred = classRepMap.get(pcs.getPredecessor());
+        T succ = classRepMap.get(pcs.getSuccessor());
+        Pcs<T> classRepPcs = new Pcs<T>(root, pred, succ, pcs.getRevision());
         pcsSet.add(classRepPcs);
         return classRepPcs;
     }

src/main/java/se/kth/spork/base3dm/ListNode.java

@@ -27,4 +27,11 @@ default boolean isEndOfList() {
     default boolean isListEdge() {
         return isStartOfList() || isEndOfList();
     }
+
+    /**
+     * @return true iff this node is a virtual node.
+     */
+    default boolean isVirtual() {
+        return isListEdge();
+    }
 }

src/main/java/se/kth/spork/base3dm/Pcs.java

@@ -21,6 +21,10 @@ public class Pcs<T extends ListNode> {
      * @param revision The revision this PCS is related to.
      */
     public Pcs(T root, T predecessor, T successor, Revision revision) {
+        if (root == null || predecessor == null || successor == null) {
+            throw new IllegalArgumentException("nodes may not be null");
+        }
+
         this.root = root;
         this.predecessor = predecessor;
         this.successor = successor;

src/main/java/se/kth/spork/spoon/PcsBuilder.java

@@ -5,6 +5,7 @@
 import se.kth.spork.spoon.wrappers.NodeFactory;
 import se.kth.spork.spoon.wrappers.SpoonNode;
 import spoon.reflect.declaration.CtElement;
+import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.visitor.CtScanner;
 
 import java.util.*;
@@ -15,15 +16,15 @@
  * @author Simon Larsén
  */
 class PcsBuilder extends CtScanner {
-    private Map<SpoonNode, SpoonNode> rootTolastSibling = new HashMap<>();
+    private Map<SpoonNode, SpoonNode> parentToLastSibling = new HashMap<>();
     private Set<Pcs<SpoonNode>> pcses = new HashSet<>();
     private SpoonNode root = null;
     private Revision revision;
 
     public PcsBuilder(Revision revision) {
        super();
        this.revision = revision;
-       rootTolastSibling.put(NodeFactory.ROOT, NodeFactory.startOfChildList(NodeFactory.ROOT));
+       parentToLastSibling.put(NodeFactory.ROOT, NodeFactory.startOfChildList(NodeFactory.ROOT));
     }
 
     /**
@@ -36,36 +37,58 @@ public PcsBuilder(Revision revision) {
     public static Set<Pcs<SpoonNode>> fromSpoon(CtElement spoonClass, Revision revision) {
         PcsBuilder scanner = new PcsBuilder(revision);
         scanner.scan(spoonClass);
+        scanner.finishPcses();
         return scanner.getPcses();
     }
 
     @Override
     protected void enter(CtElement e) {
         SpoonNode wrapped = NodeFactory.wrap(e);
+        SpoonNode parent = wrapped.getParent();
+
         if (root == null)
             root = wrapped;
 
-        rootTolastSibling.put(wrapped, NodeFactory.startOfChildList(wrapped));
+        parentToLastSibling.put(wrapped, NodeFactory.startOfChildList(wrapped));
 
-        SpoonNode parent = wrapped.getParent();
-        SpoonNode predecessor = rootTolastSibling.get(parent);
+        SpoonNode predecessor = parentToLastSibling.getOrDefault(parent, NodeFactory.startOfChildList(parent));
         pcses.add(new Pcs<>(parent, predecessor, wrapped, revision));
-        rootTolastSibling.put(parent, wrapped);
+        parentToLastSibling.put(parent, wrapped);
     }
 
-    @Override
-    protected void exit(CtElement e) {
-        SpoonNode current = NodeFactory.wrap(e);
-        SpoonNode predecessor = rootTolastSibling.get(current);
-        SpoonNode successor = NodeFactory.endOfChildList(current);
-        pcses.add(new Pcs<>(current, predecessor, successor, revision));
+    private void finishPcses() {
+        for (Map.Entry<SpoonNode, SpoonNode> nodePair : parentToLastSibling.entrySet()) {
+            SpoonNode parent = nodePair.getKey();
+            SpoonNode lastSibling = nodePair.getValue();
+            if (parent.isVirtual()) {
+                // this is either the virtual root, or a RoleNode
+                // we just need to close their child lists
+                pcses.add(new Pcs<>(parent, lastSibling, NodeFactory.endOfChildList(parent), revision));
+            } else {
+                // this is a concrete node, we must add all of its virtual children to the PCS structure, except for
+                // the start of the child list as it has all ready been added
+                List<SpoonNode> virtualNodes = parent.getVirtualNodes();
+                SpoonNode pred = lastSibling;
+                for (SpoonNode succ : virtualNodes.subList(1, virtualNodes.size())) {
+                    pcses.add(new Pcs<>(parent, pred, succ, revision));
 
-        if (current.getParent() == NodeFactory.ROOT) {
-            // need to finish the virtual root's child list artificially as it is not a real node
-            pcses.add(new Pcs<>(NodeFactory.ROOT, current, NodeFactory.endOfChildList(NodeFactory.ROOT), revision));
+                    // also need to create "leaf child lists" for any non-list-edge virtual node that does not have any
+                    // children, or Spork will not discover removals that entirely empty the child list.
+                    // The problem is detailed in https://github.com/KTH/spork/issues/116
+                    if (!pred.isListEdge() && !parentToLastSibling.containsKey(pred)) {
+                        pcses.add(createLeafPcs(pred));
+                    }
+
+                    pred = succ;
+                }
+            }
         }
     }
 
+    private Pcs<SpoonNode> createLeafPcs(SpoonNode node) {
+        return new Pcs<>(node, NodeFactory.startOfChildList(node), NodeFactory.endOfChildList(node), revision);
+    }
+
     public Set<Pcs<SpoonNode>> getPcses() {
         return pcses;
     }

src/main/java/se/kth/spork/spoon/matching/ClassRepresentatives.java

@@ -5,10 +5,12 @@
 import se.kth.spork.spoon.wrappers.SpoonNode;
 import se.kth.spork.spoon.wrappers.NodeFactory;
 import spoon.reflect.declaration.CtElement;
+import spoon.reflect.reference.CtReference;
 import spoon.reflect.visitor.CtScanner;
 
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -126,13 +128,19 @@ private static void mapNodes(SpoonNode from, SpoonNode to, Map<SpoonNode, SpoonN
         classRepMap.put(from, to);
 
         // map the virtual nodes
-        SpoonNode fromSOL = NodeFactory.startOfChildList(from);
-        SpoonNode toSOL = NodeFactory.startOfChildList(to);
-        SpoonNode fromEOL = NodeFactory.endOfChildList(from);
-        SpoonNode toEOL = NodeFactory.endOfChildList(to);
+        List<SpoonNode> fromVirtualNodes = from.getVirtualNodes();
+        List<SpoonNode> toVirtualNodes = to.getVirtualNodes();
 
-        classRepMap.put(fromSOL, toSOL);
-        classRepMap.put(fromEOL, toEOL);
+        for (int i = 0; i < fromVirtualNodes.size(); i++) {
+            SpoonNode fromVirt = fromVirtualNodes.get(i);
+            SpoonNode toVirt = toVirtualNodes.get(i);
+
+            if (fromVirt.isListEdge()) {
+                classRepMap.put(fromVirt, toVirt);
+            } else {
+                mapNodes(fromVirt, toVirt, classRepMap);
+            }
+        }
     }
 
     /**

src/main/java/se/kth/spork/spoon/matching/SpoonMapping.java

@@ -4,6 +4,7 @@
 import com.github.gumtreediff.matchers.MappingStore;
 import com.github.gumtreediff.tree.ITree;
 import com.github.gumtreediff.utils.Pair;
+import gumtree.spoon.builder.CtWrapper;
 import gumtree.spoon.builder.SpoonGumTreeBuilder;
 import se.kth.spork.spoon.wrappers.SpoonNode;
 import se.kth.spork.spoon.wrappers.NodeFactory;
@@ -98,6 +99,9 @@ private static boolean ignoreMapping(CtElement src, CtElement dst) {
             return true;
         } else if (isPrimitiveType(src) != isPrimitiveType(dst)) {
             return true;
+        } else if (src instanceof CtWrapper || dst instanceof CtWrapper) {
+            // the CtWrapper elements do not represent real Spoon nodes, and so are just noise
+            return true;
         }
         return false;
     }

src/main/java/se/kth/spork/spoon/pcsinterpreter/SpoonTreeBuilder.java

@@ -187,6 +187,9 @@ private CtElement visit(SporkTree sporkParent, SporkTree sporkChild) {
             }
         }
 
+        // NOTE: Super important that the parent of the merge tree is set no matter what, as wrapping a spoon CtElement
+        // in a SpoonNode requires access to its parent.
+        mergeTree.setParent(mergeParent);
         nodes.put(origTreeNode, NodeFactory.wrap(mergeTree));
 
         return mergeTree;

src/main/java/se/kth/spork/spoon/pcsinterpreter/SporkTreeBuilder.java

@@ -107,37 +107,14 @@ public SporkTree buildTree() {
     public SporkTree build(SpoonNode currentRoot) {
         Map<SpoonNode, Pcs<SpoonNode>> children = rootToChildren.get(currentRoot);
 
-        SpoonNode next = NodeFactory.startOfChildList(currentRoot);
-
         Set<Content<SpoonNode, RoledValues>> currentContent = contents.getOrDefault(currentRoot, Collections.emptySet());
         SporkTree tree = new SporkTree(currentRoot, currentContent);
 
         if (children == null) // leaf node
             return tree;
 
         try {
-            while (true) {
-                Pcs<SpoonNode> nextPcs = children.get(next);
-                tree.addRevision(nextPcs.getRevision());
-
-                next = nextPcs.getSuccessor();
-                if (next.isEndOfList()) {
-                    break;
-                }
-
-                Set<Pcs<SpoonNode>> conflicts = structuralConflicts.get(nextPcs);
-                Optional<Pcs<SpoonNode>> successorConflict = conflicts == null ? Optional.empty() :
-                        conflicts.stream().filter(confPcs ->
-                                StructuralConflict.isSuccessorConflict(nextPcs, confPcs)).findFirst();
-
-                // successor conflicts mark the start of a conflict, any other conflict is to be ignored
-                if (successorConflict.isPresent()) {
-                    Pcs<SpoonNode> conflicting = successorConflict.get();
-                    next = traverseConflict(nextPcs, conflicting, children, tree);
-                } else {
-                    addChild(tree, build(next));
-                }
-            }
+            build(NodeFactory.startOfChildList(currentRoot), tree, children);
 
             for (Pcs<SpoonNode> inconsistent : remainingInconsistencies) {
                 if (inconsistent.getRoot().equals(tree.getNode())) {
@@ -158,6 +135,49 @@ public SporkTree build(SpoonNode currentRoot) {
         return tree;
     }
 
+    private void build(SpoonNode start, SporkTree tree, Map<SpoonNode, Pcs<SpoonNode>> children) {
+        if (children == null) // leaf node
+            return;
+
+
+        SpoonNode next = start;
+        while (true) {
+            Pcs<SpoonNode> nextPcs = children.get(next);
+            tree.addRevision(nextPcs.getRevision());
+
+            next = nextPcs.getSuccessor();
+
+            if (next.isListEdge()) {
+                // can still have a conflict at the end of the child list
+                getSuccessorConflict(nextPcs).map(conflicting -> traverseConflict(nextPcs, conflicting, children, tree));
+                break;
+            }
+
+            if (next.isVirtual() && !next.isListEdge()) {
+                build(NodeFactory.startOfChildList(next), tree, rootToChildren.get(next));
+            } else {
+                Optional<Pcs<SpoonNode>> successorConflict = getSuccessorConflict(nextPcs);
+                if (successorConflict.isPresent()) {
+                    Pcs<SpoonNode> conflicting = successorConflict.get();
+                    next = traverseConflict(nextPcs, conflicting, children, tree);
+                } else {
+                    addChild(tree, build(next));
+                }
+
+                if (next.isEndOfList()) {
+                    break;
+                }
+            }
+        }
+    }
+
+    private Optional<Pcs<SpoonNode>> getSuccessorConflict(Pcs<SpoonNode> pcs) {
+        Set<Pcs<SpoonNode>> conflicts = structuralConflicts.get(pcs);
+        return conflicts == null ? Optional.empty() :
+                conflicts.stream().filter(confPcs ->
+                        StructuralConflict.isSuccessorConflict(pcs, confPcs)).findFirst();
+    }
+
     /**
      * When a conflict in the child list of a node is not possible to resolve, we approximate the conflict by finding
      * the node's matches in the left and right revisions and have them make up the conflict instead. This is a rough
@@ -211,6 +231,9 @@ private SpoonNode traverseConflict(
         List<SpoonNode> rightNodes = extractConflictList(rightPcs, children);
 
         Optional<List<SpoonNode>> resolved = tryResolveConflict(leftNodes, rightNodes);
+
+        // if nextPcs happens to be the final PCS of a child list, next may be a virtual node
+        next = leftNodes.isEmpty() ? rightNodes.get(rightNodes.size() - 1) : leftNodes.get(leftNodes.size() - 1);
         if (resolved.isPresent()) {
             resolved.get().forEach(child ->
                     addChild(tree, build(child))
@@ -225,7 +248,7 @@ private SpoonNode traverseConflict(
             tree.addChild(new SporkTree(next, contents.get(next), conflict));
         }
         // by convention, build left tree
-        return leftNodes.isEmpty() ? next : leftNodes.get(leftNodes.size() - 1);
+        return next;
     }
 
     private void addChild(SporkTree tree, SporkTree child) {