Skip to content

Commit

Permalink
[feat] Make Spork comply with git merge-file, fix #113
Browse files Browse the repository at this point in the history
Some conflicts are currently compounded. For example, multiple conflicts in a comment counts as one conflict. But it is accurate for the most part.
  • Loading branch information
slarse committed May 12, 2020
1 parent 0643d74 commit 14f733b
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 44 deletions.
16 changes: 8 additions & 8 deletions src/main/java/se/kth/spork/cli/Cli.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ public Integer call() throws IOException {
rightPath.toFile().deleteOnExit();
}

Pair<String, Boolean> merged = merge(basePath, leftPath, rightPath);
Pair<String, Integer> merged = merge(basePath, leftPath, rightPath);
String pretty = merged.first;
boolean hasConflicts = merged.second;
int numConflicts = merged.second;

if (out != null) {
LOGGER.info(() -> "Writing merge to " + out);
Expand All @@ -149,7 +149,7 @@ public Integer call() throws IOException {
}

LOGGER.info(() -> "Total time elapsed: " + (double) (System.nanoTime() - start) / 1e9 + " seconds");
return hasConflicts ? 1 : 0;
return numConflicts % 127;
}

}
Expand All @@ -160,22 +160,22 @@ public Integer call() throws IOException {
* @param base Path to base revision.
* @param left Path to left revision.
* @param right Path to right revision.
* @return A pair on the form (prettyPrint, hasConflicts)
* @return A pair on the form (prettyPrint, numConflicts)
*/
public static Pair<String, Boolean> merge(Path base, Path left, Path right) {
public static Pair<String, Integer> merge(Path base, Path left, Path right) {
LOGGER.info(() -> "Parsing input files");
CtModule baseModule = Parser.parse(base);
CtModule leftModule = Parser.parse(left);
CtModule rightModule = Parser.parse(right);

LOGGER.info(() -> "Initiating merge");
Pair<CtElement, Boolean> merge = Spoon3dmMerge.merge(baseModule, leftModule, rightModule);
Pair<CtElement, Integer> merge = Spoon3dmMerge.merge(baseModule, leftModule, rightModule);
CtModule mergeTree = (CtModule) merge.first;
boolean hasConflicts = merge.second;
int numConflicts = merge.second;

LOGGER.info(() -> "Pretty-printing");
if (containsTypes(mergeTree)) {
return Pair.of(prettyPrint(mergeTree), hasConflicts);
return Pair.of(prettyPrint(mergeTree), numConflicts);
} else {
LOGGER.warn(() -> "Merge contains no types (i.e. classes, interfaces, etc), reverting to line-based merge");
String baseStr = Parser.read(base);
Expand Down
15 changes: 8 additions & 7 deletions src/main/java/se/kth/spork/spoon/Spoon3dmMerge.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public class Spoon3dmMerge {
* @param base The base revision.
* @param left The left revision.
* @param right The right revision.
* @return A pair on the form (mergeTree, hasConflicts).
* @return A pair on the form (mergeTree, numConflicts).
*/
public static Pair<CtModule, Boolean> merge(Path base, Path left, Path right) {
public static Pair<CtModule, Integer> merge(Path base, Path left, Path right) {
long start = System.nanoTime();

// PARSING PHASE
Expand All @@ -57,9 +57,9 @@ public static Pair<CtModule, Boolean> merge(Path base, Path left, Path right) {
* @param base The base revision.
* @param left The left revision.
* @param right The right revision.
* @return A pair on the form (mergeTree, hasConflicts).
* @return A pair on the form (mergeTree, numConflicts).
*/
public static <T extends CtElement> Pair<T, Boolean> merge(T base, T left, T right) {
public static <T extends CtElement> Pair<T, Integer> merge(T base, T left, T right) {
long start = System.nanoTime();

// MATCHING PHASE
Expand Down Expand Up @@ -114,19 +114,20 @@ public static <T extends CtElement> Pair<T, Boolean> merge(T base, T left, T rig

// INTERPRETER PHASE
LOGGER.info(() -> "Interpreting resolved PCS merge");
Pair<CtElement, Boolean> merge = PcsInterpreter.fromMergedPcs(delta, baseLeft, baseRight);
Pair<CtElement, Integer> merge = PcsInterpreter.fromMergedPcs(delta, baseLeft, baseRight);
// we can be certain that the merge tree has the same root type as the three constituents, so this cast is safe
@SuppressWarnings("unchecked")
T mergeTree = (T) merge.first;
boolean hasConflicts = merge.second;
int numConflicts = merge.second;


LOGGER.info(() -> "Merging import statements");
List<CtImport> mergedImports = mergeImportStatements(base, left, right);
mergeTree.putMetadata(Parser.IMPORT_STATEMENTS, mergedImports);

LOGGER.info(() -> "Merged in " + (double) (System.nanoTime() - start) / 1e9 + " seconds");

return Pair.of(mergeTree, hasConflicts);
return Pair.of(mergeTree, numConflicts);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ private static Optional<?> mergeIsUpper(Optional<CtElement> baseElem, CtElement
}

private static Optional<?> mergeComments(Object base, Object left, Object right) {
Pair<String, Boolean> merge = LineBasedMerge.merge(base.toString(), left.toString(), right.toString());
Pair<String, Integer> merge = LineBasedMerge.merge(base.toString(), left.toString(), right.toString());

if (merge.second) {
if (merge.second > 0) {
return Optional.empty();
}
return Optional.of(merge.first);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ public class PcsInterpreter {
*
* @param baseLeft A tree matching between the base revision and the left revision.
* @param baseRight A tree matching between the base revision and the right revision.
* @return A pair on the form (tree, hasStructuralConflicts).
* @return A pair on the form (tree, numConflicts).
*/
public static Pair<CtElement, Boolean> fromMergedPcs(
public static Pair<CtElement, Integer> fromMergedPcs(
ChangeSet<SpoonNode, RoledValues> delta,
SpoonMapping baseLeft,
SpoonMapping baseRight) {
Expand All @@ -36,6 +36,6 @@ public static Pair<CtElement, Boolean> fromMergedPcs(
SpoonTreeBuilder spoonTreeBuilder = new SpoonTreeBuilder(baseLeft, baseRight, oldEnv);
CtElement spoonTreeRoot = spoonTreeBuilder.build(sporkTreeRoot);

return Pair.of(spoonTreeRoot, sporkTreeBuilder.hasStructuralConflict() || spoonTreeBuilder.hasContentConflict());
return Pair.of(spoonTreeRoot, sporkTreeBuilder.numStructuralConflicts() + spoonTreeBuilder.numContentConflicts());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class SpoonTreeBuilder {

private SpoonMapping baseLeft;
private SpoonMapping baseRight;
private boolean hasContentConflict = false;
private int numContentConflicts = 0;

private Factory factory;

Expand Down Expand Up @@ -129,10 +129,10 @@ public CtElement build(SporkTree root) {
}

/**
* @return true if a content conflict was encountered when building the tree.
* @return The amount of content conflicts.
*/
public boolean hasContentConflict() {
return hasContentConflict;
public int numContentConflicts() {
return numContentConflicts;
}

/**
Expand Down Expand Up @@ -161,7 +161,7 @@ private CtElement visit(SporkTree sporkParent, SporkTree sporkChild) {
if (!mergedContent.second.isEmpty()) {
// at least one conflict was not resolved
mergeTree.putMetadata(ContentConflict.METADATA_KEY, mergedContent.second);
hasContentConflict = true;
numContentConflicts += mergedContent.second.size();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SporkTreeBuilder {
private final Map<SpoonNode, Map<SpoonNode, Pcs<SpoonNode>>> rootToChildren;
private final Map<Pcs<SpoonNode>, Set<Pcs<SpoonNode>>> structuralConflicts;
private final Map<SpoonNode, Set<Content<SpoonNode, RoledValues>>> contents;
private boolean hasStructuralConflicts;
private int numStructuralConflicts;

// keeps track of which nodes have been added to the tree all ready
// if any node is added twice, there's an unresolved move conflict
Expand All @@ -36,7 +36,7 @@ public SporkTreeBuilder(ChangeSet<SpoonNode, RoledValues> delta) {
this.rootToChildren = buildRootToChildren(delta.getPcsSet());
structuralConflicts = delta.getStructuralConflicts();
contents = delta.getContents();
hasStructuralConflicts = false;
numStructuralConflicts = 0;
usedNodes = new HashSet<>();
}

Expand Down Expand Up @@ -69,10 +69,10 @@ private static Optional<List<SpoonNode>> tryResolveConflict(List<SpoonNode> left
}

/**
* @return true iff conflicts were encountered at some point when building a tree.
* @return The amount of structural conflicts.
*/
public boolean hasStructuralConflict() {
return hasStructuralConflicts;
public int numStructuralConflicts() {
return numStructuralConflicts;
}

/**
Expand Down Expand Up @@ -137,7 +137,7 @@ private SpoonNode traverseConflict(
addChild(tree, build(child))
);
} else {
hasStructuralConflicts = true;
numStructuralConflicts++;
StructuralConflict conflict = new StructuralConflict(
leftNodes.stream().map(SpoonNode::getElement).collect(Collectors.toList()),
rightNodes.stream().map(SpoonNode::getElement).collect(Collectors.toList())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ private void processConflict(ContentConflict conflict, CtElement element) {
String rawBase = conflict.getBase().isPresent() ?
(String) conflict.getBase().get().getMetadata(RoledValue.Key.RAW_CONTENT) : "";

Pair<String, Boolean> rawConflict = LineBasedMerge.merge(rawBase, rawLeft, rawRight);
assert rawConflict.second : "Comments without conflict should already have been merged";
Pair<String, Integer> rawConflict = LineBasedMerge.merge(rawBase, rawLeft, rawRight);
assert rawConflict.second > 0 : "Comments without conflict should already have been merged";

element.putMetadata(RAW_COMMENT_CONFLICT_KEY, rawConflict.first);
break;
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/se/kth/spork/util/LineBasedMerge.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ public class LineBasedMerge {
* @param base The base revision.
* @param left The left revision.
* @param right The right revision.
* @return A pair containing the merge and a boolean that, if true, indicates the presence of conflicts.
* @return A pair containing the merge and the amount of conflicts.
*/
public static Pair<String, Boolean> merge(String base, String left, String right) {
public static Pair<String, Integer> merge(String base, String left, String right) {
RawText baseRaw = new RawText(base.getBytes());
RawText leftRaw = new RawText(left.getBytes());
RawText rightRaw = new RawText(right.getBytes());
boolean hasConflict = false;

MergeAlgorithm merge = new MergeAlgorithm();
MergeResult<RawText> res = merge.merge(new SequenceComparator<RawText>() {
Expand All @@ -51,13 +50,14 @@ public int hash(RawText s, int i) {
List<RawText> seqs = res.getSequences();
List<String> lines = new ArrayList<>();
boolean inConflict = false;
int numConflicts = 0;

while (it.hasNext()) {
MergeChunk chunk = it.next();
RawText seq = seqs.get(chunk.getSequenceIndex());

if (chunk.getConflictState() == MergeChunk.ConflictState.FIRST_CONFLICTING_RANGE) {
hasConflict = true;
numConflicts++;
inConflict = true;
lines.add(SporkPrettyPrinter.START_CONFLICT);
} else if (chunk.getConflictState() == MergeChunk.ConflictState.NEXT_CONFLICTING_RANGE) {
Expand Down Expand Up @@ -86,6 +86,6 @@ public int hash(RawText s, int i) {
lines.add(SporkPrettyPrinter.END_CONFLICT);
}

return new Pair<>(String.join("\n", lines), hasConflict);
return new Pair<>(String.join("\n", lines), numConflicts);
}
}
10 changes: 5 additions & 5 deletions src/test/java/se/kth/spork/cli/CliTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ void mergeTreeShouldEqualReParsedPrettyPrent_whenBothRevisionsAreModified(Util.T
void prettyPrint_shouldContainConflict(Util.TestSources sources) throws IOException {
List<Util.Conflict> expectedConflicts = Util.parseConflicts(sources.expected);

Pair<CtModule, Boolean> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
Pair<CtModule, Integer> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
CtModule mergeTree = merged.first;
boolean hasConflicts = merged.second;
Integer numConflicts = merged.second;

String prettyPrint = Cli.prettyPrint(mergeTree);

Expand All @@ -77,15 +77,15 @@ void prettyPrint_shouldContainConflict(Util.TestSources sources) throws IOExcept
System.out.println(prettyPrint);

assertEquals(expectedConflicts, actualConflicts);
assertTrue(hasConflicts);
assertTrue(numConflicts > 0);
}

@ParameterizedTest
@ArgumentsSource(Util.ConflictSourceProvider.class)
void prettyPrint_shouldParseToExpectedTree_whenConflictHasBeenStrippedOut(Util.TestSources sources) throws IOException {
CtModule expected = Parser.parse(Util.keepLeftConflict(sources.expected));

Pair<CtModule, Boolean> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
Pair<CtModule, Integer> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
CtModule mergeTree = merged.first;

String prettyPrint = Cli.prettyPrint(mergeTree);
Expand All @@ -102,7 +102,7 @@ void prettyPrint_shouldParseToExpectedTree_whenConflictHasBeenStrippedOut(Util.T
* @param sources
*/
private static void runTestMerge(Util.TestSources sources, Path tempDir) throws IOException {
Pair<CtModule, Boolean> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
Pair<CtModule, Integer> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
CtModule mergeTree = merged.first;

Object expectedImports = mergeTree.getMetadata(Parser.IMPORT_STATEMENTS);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/se/kth/spork/spoon/Spoon3dmMergeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ private static void runTestMerge(Util.TestSources sources) {
Object expectedImports = expected.getMetadata(Parser.IMPORT_STATEMENTS);
assert expectedImports != null;

Pair<CtModule, Boolean> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
Pair<CtModule, Integer> merged = Spoon3dmMerge.merge(sources.base, sources.left, sources.right);
CtModule mergeTree = merged.first;
Object mergedImports = mergeTree.getMetadata(Parser.IMPORT_STATEMENTS);

Expand Down

0 comments on commit 14f733b

Please sign in to comment.