diff --git a/src/main/java/se/kth/spork/base3dm/ChangeSet.java b/src/main/java/se/kth/spork/base3dm/ChangeSet.java index 5e653f6c..d2cb4598 100644 --- a/src/main/java/se/kth/spork/base3dm/ChangeSet.java +++ b/src/main/java/se/kth/spork/base3dm/ChangeSet.java @@ -197,7 +197,7 @@ private void add(Set> tree, Function getContent) { addToLookupTable(classRepSucc, classRepPcs, successors); } if (!pred.isVirtual()) { - Content c = new Content(pcs, getContent.apply(pred)); + Content c = new Content(pcs, getContent.apply(pred), pred.getRevision()); addToLookupTable(classRepPred, c, content); } } diff --git a/src/main/java/se/kth/spork/base3dm/Content.java b/src/main/java/se/kth/spork/base3dm/Content.java index 0ade804e..efde8922 100644 --- a/src/main/java/se/kth/spork/base3dm/Content.java +++ b/src/main/java/se/kth/spork/base3dm/Content.java @@ -8,8 +8,9 @@ * @author Simon Larsén */ public class Content { - private Pcs context; - private V value; + private final Pcs context; + private final V value; + private final Revision revision; /** * Create a content container. @@ -17,15 +18,20 @@ public class Content { * @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 context, V value) { + public Content(Pcs context, V value, Revision revision) { this.context = context; this.value = value; + this.revision = revision; } public Pcs getContext() { return context; } + public Revision getRevision() { + return revision; + } + public V getValue() { return value; } diff --git a/src/main/java/se/kth/spork/base3dm/ListNode.java b/src/main/java/se/kth/spork/base3dm/ListNode.java index 16503259..de28a75d 100644 --- a/src/main/java/se/kth/spork/base3dm/ListNode.java +++ b/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(); } diff --git a/src/main/java/se/kth/spork/spoon/Spoon3dmMerge.java b/src/main/java/se/kth/spork/spoon/Spoon3dmMerge.java index 320eabff..5afc699f 100644 --- a/src/main/java/se/kth/spork/spoon/Spoon3dmMerge.java +++ b/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 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 Pair merge(T base, T left, T right) { + public static Pair merge( + T base, + T left, + T right, + BiFunction baseMatcher, + BiFunction leftRightMatcher) { long start = System.nanoTime(); // MATCHING PHASE @@ -74,9 +83,9 @@ public static Pair 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 Pair 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 mergedImports = mergeImportStatements(base, left, right); - mergeTree.putMetadata(Parser.IMPORT_STATEMENTS, mergedImports); - - LOGGER.info(() -> "Merging compilation unit comments"); - Pair 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 Pair 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 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 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> types = merge.getElements(e -> true); + int numConflicts = 0; + for (CtType type : types) { + numConflicts += eliminateDuplicateMembers(type); + } + return numConflicts; + } + + private static int eliminateDuplicateMembers(CtType type) { + List members = new ArrayList<>(type.getTypeMembers()); + Map 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 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; } /** diff --git a/src/main/java/se/kth/spork/spoon/matching/ClassRepresentatives.java b/src/main/java/se/kth/spork/spoon/matching/ClassRepresentatives.java index 1031bfd4..4c3262f1 100644 --- a/src/main/java/se/kth/spork/spoon/matching/ClassRepresentatives.java +++ b/src/main/java/se/kth/spork/spoon/matching/ClassRepresentatives.java @@ -69,7 +69,7 @@ private static Map initializeClassRepresentatives(CtElemen Iterator 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 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 classRepMap; + private Map 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); diff --git a/src/main/java/se/kth/spork/spoon/matching/SpoonMapping.java b/src/main/java/se/kth/spork/spoon/matching/SpoonMapping.java index 36244402..0049dd23 100644 --- a/src/main/java/se/kth/spork/spoon/matching/SpoonMapping.java +++ b/src/main/java/se/kth/spork/spoon/matching/SpoonMapping.java @@ -38,7 +38,6 @@ public class SpoonMapping { private Map dsts; - // SpoonMapping should only be instantiated with fromGumTreeMapping, which is why the default constructor is private private SpoonMapping() { srcs = new HashMap<>(); dsts = new HashMap<>(); diff --git a/src/main/java/se/kth/spork/spoon/pcsinterpreter/ContentMerger.java b/src/main/java/se/kth/spork/spoon/pcsinterpreter/ContentMerger.java index 77c28202..6bae23c0 100644 --- a/src/main/java/se/kth/spork/spoon/pcsinterpreter/ContentMerger.java +++ b/src/main/java/se/kth/spork/spoon/pcsinterpreter/ContentMerger.java @@ -277,7 +277,7 @@ private static _ContentTriple getContentRevisions(Set right = null; for (Content cnt : contents) { - switch (cnt.getContext().getRevision()) { + switch (cnt.getRevision()) { case BASE: base = cnt; break; diff --git a/src/main/java/se/kth/spork/spoon/pcsinterpreter/SpoonTreeBuilder.java b/src/main/java/se/kth/spork/spoon/pcsinterpreter/SpoonTreeBuilder.java index c7490271..cf10a810 100644 --- a/src/main/java/se/kth/spork/spoon/pcsinterpreter/SpoonTreeBuilder.java +++ b/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 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; } diff --git a/src/main/java/se/kth/spork/spoon/pcsinterpreter/SporkTree.java b/src/main/java/se/kth/spork/spoon/pcsinterpreter/SporkTree.java index f7dd6526..8c0adfe0 100644 --- a/src/main/java/se/kth/spork/spoon/pcsinterpreter/SporkTree.java +++ b/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) { diff --git a/src/main/java/se/kth/spork/spoon/wrappers/NodeFactory.java b/src/main/java/se/kth/spork/spoon/wrappers/NodeFactory.java index c5307a68..4d0f36d7 100644 --- a/src/main/java/se/kth/spork/spoon/wrappers/NodeFactory.java +++ b/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 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); diff --git a/src/main/java/se/kth/spork/spoon/wrappers/SpoonNode.java b/src/main/java/se/kth/spork/spoon/wrappers/SpoonNode.java index 3b659b6f..454ece98 100644 --- a/src/main/java/se/kth/spork/spoon/wrappers/SpoonNode.java +++ b/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. diff --git a/src/test/java/se/kth/spork/cli/CliTest.java b/src/test/java/se/kth/spork/cli/CliTest.java index 17f475a2..4d672079 100644 --- a/src/test/java/se/kth/spork/cli/CliTest.java +++ b/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; diff --git a/src/test/resources/conflict/add_similar_fields/Base.java b/src/test/resources/conflict/add_similar_fields/Base.java new file mode 100644 index 00000000..d41f9996 --- /dev/null +++ b/src/test/resources/conflict/add_similar_fields/Base.java @@ -0,0 +1,4 @@ +public class Main { + Integer a = 2; + Integer b = 3; +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_fields/Expected.java b/src/test/resources/conflict/add_similar_fields/Expected.java new file mode 100644 index 00000000..53f60744 --- /dev/null +++ b/src/test/resources/conflict/add_similar_fields/Expected.java @@ -0,0 +1,16 @@ +public class Main { + Integer a = 2; + Integer b = 3; +<<<<<<< LEFT +Object +======= +Integer +>>>>>>> RIGHT + field = Integer.valueOf( +<<<<<<< LEFT +2 +======= +3 +>>>>>>> RIGHT + ); +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_fields/Left.java b/src/test/resources/conflict/add_similar_fields/Left.java new file mode 100644 index 00000000..cac8d302 --- /dev/null +++ b/src/test/resources/conflict/add_similar_fields/Left.java @@ -0,0 +1,5 @@ +public class Main { + Object field = Integer.valueOf(2); + Integer a = 2; + Integer b = 3; +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_fields/Right.java b/src/test/resources/conflict/add_similar_fields/Right.java new file mode 100644 index 00000000..b42c4e93 --- /dev/null +++ b/src/test/resources/conflict/add_similar_fields/Right.java @@ -0,0 +1,5 @@ +public class Main { + Integer a = 2; + Integer b = 3; + Integer field = Integer.valueOf(3); +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_methods/Base.java b/src/test/resources/conflict/add_similar_methods/Base.java new file mode 100644 index 00000000..05271fc1 --- /dev/null +++ b/src/test/resources/conflict/add_similar_methods/Base.java @@ -0,0 +1,9 @@ +public class Arithmetic { + public int add(int a, int b) { + return a + b; + } + + public int sub(int a, int b) { + return a - b; + } +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_methods/Expected.java b/src/test/resources/conflict/add_similar_methods/Expected.java new file mode 100644 index 00000000..07792aed --- /dev/null +++ b/src/test/resources/conflict/add_similar_methods/Expected.java @@ -0,0 +1,19 @@ +public class Arithmetic { + public int add(int a, int b) { + return a + b; + } + + public int sub(int a, int b) { + return a - b; + } + + public int div(int a, int b) { +<<<<<<< LEFT + ======= + if (b == 0) { + throw new IllegalArgumentException("b must be non-zero"); + } +>>>>>>> RIGHT + return a / b; + } +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_methods/Left.java b/src/test/resources/conflict/add_similar_methods/Left.java new file mode 100644 index 00000000..1491dc33 --- /dev/null +++ b/src/test/resources/conflict/add_similar_methods/Left.java @@ -0,0 +1,13 @@ +public class Arithmetic { + public int add(int a, int b) { + return a + b; + } + + public int sub(int a, int b) { + return a - b; + } + + public int div(int a, int b) { + return a / b; + } +} \ No newline at end of file diff --git a/src/test/resources/conflict/add_similar_methods/Right.java b/src/test/resources/conflict/add_similar_methods/Right.java new file mode 100644 index 00000000..4f050370 --- /dev/null +++ b/src/test/resources/conflict/add_similar_methods/Right.java @@ -0,0 +1,16 @@ +public class Arithmetic { + public int div(int a, int b) { + if (b == 0) { + throw new IllegalArgumentException("b must be non-zero"); + } + return a / b; + } + + public int add(int a, int b) { + return a + b; + } + + public int sub(int a, int b) { + return a - b; + } +} \ No newline at end of file