[fix] Apply post-processing to find duplicated type members (#138)

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

@@ -197,7 +197,7 @@ private void add(Set<Pcs<T>> tree, Function<T, V> getContent) {
                 addToLookupTable(classRepSucc, classRepPcs, successors);
             }
             if (!pred.isVirtual()) {
-                Content<T,V> c = new Content<T,V>(pcs, getContent.apply(pred));
+                Content<T,V> c = new Content<T,V>(pcs, getContent.apply(pred), pred.getRevision());
                 addToLookupTable(classRepPred, c, content);
             }
         }

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

@@ -8,24 +8,30 @@
  * @author Simon Larsén
  */
 public class Content<T extends ListNode,V> {
-    private Pcs<T> context;
-    private V value;
+    private final Pcs<T> context;
+    private final V value;
+    private final Revision revision;
 
     /**
      * Create a content container.
      *
      * @param context The context of this content. The value is associated with the predecessor of the context.
      * @param value The value of the this content.
      */
-    public Content(Pcs<T> context, V value) {
+    public Content(Pcs<T> context, V value, Revision revision) {
         this.context = context;
         this.value = value;
+        this.revision = revision;
     }
 
     public Pcs<T> getContext() {
         return context;
     }
 
+    public Revision getRevision() {
+        return revision;
+    }
+
     public V getValue() {
         return value;
     }

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

@@ -34,4 +34,10 @@ default boolean isListEdge() {
     default boolean isVirtual() {
         return isListEdge();
     }
+
+    /**
+     * @return The revision this node was created from.
+     * @throws UnsupportedOperationException If called on the virtual root.
+     */
+    Revision getRevision();
 }

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

@@ -9,6 +9,7 @@
 import se.kth.spork.spoon.matching.MappingRemover;
 import se.kth.spork.spoon.matching.SpoonMapping;
 import se.kth.spork.spoon.pcsinterpreter.PcsInterpreter;
+import se.kth.spork.spoon.wrappers.NodeFactory;
 import se.kth.spork.spoon.wrappers.RoledValues;
 import se.kth.spork.spoon.wrappers.SpoonNode;
 import se.kth.spork.util.LazyLogger;
@@ -18,6 +19,7 @@
 
 import java.nio.file.Path;
 import java.util.*;
+import java.util.function.BiFunction;
 
 /**
  * Spoon specialization of the 3DM merge algorithm.
@@ -62,9 +64,16 @@ public static Pair<CtModule, Integer> merge(Path base, Path left, Path right) {
      * @param base  The base revision.
      * @param left  The left revision.
      * @param right The right revision.
+     * @param baseMatcher Function that returns a matcher for the base-to-left and base-to-right matchings.
+     * @param leftRightMatcher Function that returns a matcher for the left-to-right matching.
      * @return A pair on the form (mergeTree, numConflicts).
      */
-    public static <T extends CtElement> Pair<T, Integer> merge(T base, T left, T right) {
+    public static <T extends CtElement> Pair<T, Integer> merge(
+            T base,
+            T left,
+            T right,
+            BiFunction<ITree, ITree, Matcher> baseMatcher,
+            BiFunction<ITree, ITree, Matcher> leftRightMatcher) {
         long start = System.nanoTime();
 
         // MATCHING PHASE
@@ -74,9 +83,9 @@ public static <T extends CtElement> Pair<T, Integer> merge(T base, T left, T rig
         ITree rightGumtree = new SpoonGumTreeBuilder().getTree(right);
 
         LOGGER.info(() -> "Matching trees with GumTree");
-        Matcher baseLeftGumtreeMatch = matchTrees(baseGumtree, leftGumtree);
-        Matcher baseRightGumtreeMatch = matchTrees(baseGumtree, rightGumtree);
-        Matcher leftRightGumtreeMatch = matchTreesXY(leftGumtree, rightGumtree);
+        Matcher baseLeftGumtreeMatch = baseMatcher.apply(baseGumtree, leftGumtree);
+        Matcher baseRightGumtreeMatch = baseMatcher.apply(baseGumtree, rightGumtree);
+        Matcher leftRightGumtreeMatch = leftRightMatcher.apply(leftGumtree, rightGumtree);
 
         LOGGER.info(() -> "Converting GumTree matches to Spoon matches");
         SpoonMapping baseLeft = SpoonMapping.fromGumTreeMapping(baseLeftGumtreeMatch.getMappings());
@@ -125,19 +134,111 @@ public static <T extends CtElement> Pair<T, Integer> merge(T base, T left, T rig
         T mergeTree = (T) merge.first;
         int numConflicts = merge.second;
 
+        int metadataElementConflicts = mergeMetadataElements(mergeTree, base, left, right);
 
-        LOGGER.info(() -> "Merging import statements");
-        List<CtImport> mergedImports = mergeImportStatements(base, left, right);
-        mergeTree.putMetadata(Parser.IMPORT_STATEMENTS, mergedImports);
-
-        LOGGER.info(() -> "Merging compilation unit comments");
-        Pair<String, Integer> cuCommentMerge = mergeCuComments(base, left, right);
-        int cuCommentConflicts = cuCommentMerge.second;
-        mergeTree.putMetadata(Parser.COMPILATION_UNIT_COMMENT, cuCommentMerge.first);
+        LOGGER.info(() -> "Checking for duplicated members");
+        int duplicateMemberConflicts = eliminateDuplicateMembers(mergeTree);
 
         LOGGER.info(() -> "Merged in " + (double) (System.nanoTime() - start) / 1e9 + " seconds");
 
-        return Pair.of(mergeTree, numConflicts + cuCommentConflicts);
+        return Pair.of(mergeTree, numConflicts + metadataElementConflicts + duplicateMemberConflicts);
+    }
+
+    /**
+     * Merge the left and right revisions. The base revision is used for computing edits, and should be the best common
+     * ancestor of left and right.
+     *
+     * Uses the full GumTree matcher for base-to-left and base-to-right, and the XY matcher for left-to-right matchings.
+     *
+     * @param base  The base revision.
+     * @param left  The left revision.
+     * @param right The right revision.
+     * @return A pair on the form (mergeTree, numConflicts).
+     */
+    public static <T extends CtElement> Pair<T, Integer> merge(T base, T left, T right) {
+        return merge(base, left, right, Spoon3dmMerge::matchTrees, Spoon3dmMerge::matchTreesXY);
+    }
+
+    private static int mergeMetadataElements(CtElement mergeTree, CtElement base, CtElement left, CtElement right) {
+        int numConflicts = 0;
+
+        if (base.getMetadata(Parser.IMPORT_STATEMENTS) != null) {
+            LOGGER.info(() -> "Merging import statements");
+            List<CtImport> mergedImports = mergeImportStatements(base, left, right);
+            mergeTree.putMetadata(Parser.IMPORT_STATEMENTS, mergedImports);
+        }
+
+        if (base.getMetadata(Parser.COMPILATION_UNIT_COMMENT) != null) {
+            LOGGER.info(() -> "Merging compilation unit comments");
+            Pair<String, Integer> cuCommentMerge = mergeCuComments(base, left, right);
+            numConflicts += cuCommentMerge.second;
+            mergeTree.putMetadata(Parser.COMPILATION_UNIT_COMMENT, cuCommentMerge.first);
+        }
+
+        return numConflicts;
+    }
+
+
+    private static int eliminateDuplicateMembers(CtElement merge) {
+        List<CtType<?>> types = merge.getElements(e -> true);
+        int numConflicts = 0;
+        for (CtType<?> type : types) {
+            numConflicts += eliminateDuplicateMembers(type);
+        }
+        return numConflicts;
+    }
+
+    private static int eliminateDuplicateMembers(CtType<?> type) {
+        List<CtTypeMember> members = new ArrayList<>(type.getTypeMembers());
+        Map<String, CtTypeMember> memberMap = new HashMap<>();
+        int numConflicts = 0;
+
+        for (CtTypeMember member : members) {
+            String key;
+            if (member instanceof CtMethod<?>) {
+                key = ((CtMethod<?>) member).getSignature();
+            } else if (member instanceof CtField<?>) {
+                key = member.getSimpleName();
+            } else if (member instanceof CtType<?>) {
+                key = ((CtType<?>) member).getQualifiedName();
+            } else {
+                continue;
+            }
+
+            CtTypeMember duplicate = memberMap.get(key);
+            if (duplicate == null) {
+                memberMap.put(key, member);
+            } else {
+                LOGGER.info(() -> "Merging duplicated member " + key);
+
+                // need to clear the metadata from these members to be able to re-run the merge
+                member.descendantIterator().forEachRemaining(NodeFactory::clearNonRevisionMetadata);
+                duplicate.descendantIterator().forEachRemaining(NodeFactory::clearNonRevisionMetadata);
+                CtTypeMember dummyBase = (CtTypeMember) member.clone();
+                dummyBase.setParent(type);
+                dummyBase.getDirectChildren().forEach(CtElement::delete);
+
+                // we forcibly set the virtual root as parent, as the real parent of these members is outside of the current scope
+                NodeFactory.clearNonRevisionMetadata(member);
+                NodeFactory.clearNonRevisionMetadata(duplicate);
+                NodeFactory.clearNonRevisionMetadata(dummyBase);
+                NodeFactory.forceWrap(member, NodeFactory.ROOT);
+                NodeFactory.forceWrap(duplicate, NodeFactory.ROOT);
+                NodeFactory.forceWrap(dummyBase, NodeFactory.ROOT);
+
+                // use the full gumtree matcher as both base matcher and left-to-right matcher
+                Pair<CtTypeMember, Integer> mergePair = merge(dummyBase, member, duplicate, Spoon3dmMerge::matchTrees, Spoon3dmMerge::matchTrees);
+                numConflicts += mergePair.second;
+                CtTypeMember mergedMember = mergePair.first;
+
+                member.delete();
+                duplicate.delete();
+
+                type.addTypeMember(mergedMember);
+            }
+        }
+
+        return numConflicts;
     }
 
     /**

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

@@ -69,7 +69,7 @@ private static Map<SpoonNode, SpoonNode> initializeClassRepresentatives(CtElemen
         Iterator<CtElement> descIt = base.descendantIterator();
         while (descIt.hasNext()) {
             CtElement tree = descIt.next();
-            tree.putMetadata(TdmMerge.REV, Revision.BASE);
+            NodeFactory.setRevisionIfUnset(tree, Revision.BASE);
             SpoonNode wrapped = NodeFactory.wrap(tree);
 
             mapNodes(wrapped, wrapped, classRepMap);
@@ -104,7 +104,7 @@ private static void mapToClassRepresentatives(CtElement tree, SpoonMapping mappi
     }
 
     private static void mapToClassRep(SpoonMapping mappings, Map<SpoonNode, SpoonNode> classRepMap, Revision rev, CtElement t) {
-        t.putMetadata(TdmMerge.REV, rev);
+        NodeFactory.setRevisionIfUnset(t, rev);
         SpoonNode wrapped = NodeFactory.wrap(t);
         SpoonNode classRep = mappings.getSrc(wrapped);
 
@@ -168,6 +168,7 @@ private static void mapNodes(SpoonNode from, SpoonNode to, Map<SpoonNode, SpoonN
     private static class ClassRepresentativeAugmenter extends CtScanner {
         private SpoonMapping leftRightMatch;
         private Map<SpoonNode, SpoonNode> classRepMap;
+        private Map<String, SpoonNode> forcedMappings;
 
         /**
          * @param classRepMap The class representatives map, initialized with left-to-base and right-to-base mappings.
@@ -188,8 +189,6 @@ public void scan(CtElement element) {
             if (element == null)
                 return;
 
-            assert element.getMetadata(TdmMerge.REV) == Revision.LEFT;
-
             SpoonNode left = NodeFactory.wrap(element);
             if (classRepMap.get(left) == left) {
                 SpoonNode right = leftRightMatch.getDst(left);

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

@@ -38,7 +38,6 @@ public class SpoonMapping {
     private Map<SpoonNode, SpoonNode> dsts;
 
 
-    // SpoonMapping should only be instantiated with fromGumTreeMapping, which is why the default constructor is private
     private SpoonMapping() {
         srcs = new HashMap<>();
         dsts = new HashMap<>();

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

@@ -277,7 +277,7 @@ private static _ContentTriple getContentRevisions(Set<Content<SpoonNode, RoledVa
         Content<SpoonNode, RoledValues> right = null;
 
         for (Content<SpoonNode, RoledValues> cnt : contents) {
-            switch (cnt.getContext().getRevision()) {
+            switch (cnt.getRevision()) {
                 case BASE:
                     base = cnt;
                     break;

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

@@ -146,7 +146,8 @@ private CtElement visit(SporkTree sporkParent, SporkTree sporkChild) {
         SpoonNode origRootNode = sporkParent.getNode();
         SpoonNode origTreeNode = sporkChild.getNode();
         CtElement originalTree = origTreeNode.getElement();
-        CtElement mergeParent = origRootNode == NodeFactory.ROOT ? null : nodes.get(origRootNode).getElement();
+        Optional<CtElement> mergeParentOpt = Optional.ofNullable(
+                origRootNode == NodeFactory.ROOT ? null : nodes.get(origRootNode).getElement());
         CtElement mergeTree;
 
         if (sporkChild.isSingleRevisionSubtree()) {
@@ -171,7 +172,8 @@ private CtElement visit(SporkTree sporkParent, SporkTree sporkChild) {
         metadata.put(ORIGINAL_NODE_KEY, originalTree);
         mergeTree.setAllMetadata(metadata);
 
-        if (mergeParent != null) {
+        if (mergeParentOpt.isPresent()) {
+            CtElement mergeParent = mergeParentOpt.get();
             CtRole mergeTreeRole = resolveRole(origTreeNode);
             Object inserted = withSiblings(originalTree, mergeParent, mergeTree, mergeTreeRole);
 
@@ -187,10 +189,17 @@ 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));
+        SpoonNode mergeNode;
+        if (mergeParentOpt.isPresent()) {
+            // 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(mergeParentOpt.get());
+            mergeNode = NodeFactory.wrap(mergeTree);
+        } else {
+            // if the merge tree has no parent, then its parent is the virtual root
+            mergeNode = NodeFactory.forceWrap(mergeTree, NodeFactory.ROOT);
+        }
+        nodes.put(origTreeNode, mergeNode);
 
         return mergeTree;
     }

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

@@ -41,7 +41,7 @@ public SporkTree(
         if (node != NodeFactory.ROOT) {
             revisions.add(node.getRevision());
         }
-        content.stream().map(Content::getContext).map(Pcs::getRevision).forEach(revisions::add);
+        content.stream().map(Content::getRevision).forEach(revisions::add);
     }
 
     public SporkTree(SpoonNode node, Set<Content<SpoonNode, RoledValues>> content) {

src/main/java/se/kth/spork/spoon/wrappers/NodeFactory.java

@@ -82,6 +82,43 @@ public static SpoonNode wrap(CtElement elem) {
         return wrapInternal(elem);
     }
 
+    /**
+     * Wrap the provided element and forcibly set its parent.
+     *
+     * This will replace any previous wrapper for this element.
+     *
+     * @param elem An element to wrap.
+     * @param parent The SpoonNode parent of the element.
+     * @return A wrapper around a CtElement.
+     */
+    public static SpoonNode forceWrap(CtElement elem, SpoonNode parent) {
+        return initializeWrapper(elem, parent);
+    }
+
+    /**
+     * Clear all non-revision metadata from this element.
+     *
+     * @param elem An element.
+     */
+    public static void clearNonRevisionMetadata(CtElement elem) {
+        Revision rev = (Revision) elem.getMetadata(TdmMerge.REV);
+        Map<String, Object> metadata = new TreeMap<>();
+        metadata.put(TdmMerge.REV, rev);
+        elem.setAllMetadata(metadata);
+    }
+
+    /**
+     * Set the revision of this element only if it is not all ready set.
+     *
+     * @param elem An element.
+     * @param revision A revision.
+     */
+    public static void setRevisionIfUnset(CtElement elem, Revision revision) {
+        if (elem.getMetadata(TdmMerge.REV) == null) {
+            elem.putMetadata(TdmMerge.REV, revision);
+        }
+    }
+
     private static Node wrapInternal(CtElement elem) {
         Object wrapper = elem.getMetadata(WRAPPER_METADATA);
 

src/main/java/se/kth/spork/spoon/wrappers/SpoonNode.java

@@ -26,12 +26,6 @@ public interface SpoonNode extends ListNode {
      */
     SpoonNode getParent();
 
-    /**
-     * @return The revision this node was created from.
-     * @throws UnsupportedOperationException If called on the virtual root.
-     */
-    Revision getRevision();
-
     /**
      * @return All virtual children belonging to this node.
      * @throws UnsupportedOperationException If called on a list edge.

src/test/java/se/kth/spork/cli/CliTest.java

@@ -7,6 +7,7 @@
 import picocli.CommandLine;
 import se.kth.spork.Util;
 import se.kth.spork.exception.MergeException;
+import se.kth.spork.spoon.Compare;
 import se.kth.spork.spoon.Parser;
 import se.kth.spork.spoon.Spoon3dmMerge;
 import se.kth.spork.util.Pair;

src/test/resources/conflict/add_similar_fields/Base.java

@@ -0,0 +1,4 @@
+public class Main {
+    Integer a = 2;
+    Integer b = 3;
+}