Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
May 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/se/kth/spork/base3dm/ChangeSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/se/kth/spork/base3dm/Content.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/se/kth/spork/base3dm/ListNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
127 changes: 114 additions & 13 deletions src/main/java/se/kth/spork/spoon/Spoon3dmMerge.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,6 +19,7 @@

import java.nio.file.Path;
import java.util.*;
import java.util.function.BiFunction;

/**
* Spoon specialization of the 3DM merge algorithm.
Expand Down Expand Up @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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);

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
37 changes: 37 additions & 0 deletions src/main/java/se/kth/spork/spoon/wrappers/NodeFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 0 additions & 6 deletions src/main/java/se/kth/spork/spoon/wrappers/SpoonNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/test/java/se/kth/spork/cli/CliTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/test/resources/conflict/add_similar_fields/Base.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public class Main {
Integer a = 2;
Integer b = 3;
}
Loading