Merge pull request #1550 from jplag/feature/Merging

Changes to MatchMerging as its tests by nils.

core/src/main/java/de/jplag/merging/MatchMerging.java

@@ -1,10 +1,8 @@
 package de.jplag.merging;
 
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
+import java.util.Comparator;
 import java.util.List;
-import java.util.Map;
 
 import de.jplag.JPlagComparison;
 import de.jplag.JPlagResult;
@@ -24,7 +22,7 @@
  * {@link JPlagOptions} as {@link MergingOptions} and default to (2,6).
  */
 public class MatchMerging {
-    private JPlagOptions options;
+    private final JPlagOptions options;
 
     /**
      * Instantiates the match merging algorithm for a comparison result and a set of specific options.
@@ -68,25 +66,15 @@ public JPlagResult mergeMatchesOf(JPlagResult result) {
      */
     private List<Neighbor> computeNeighbors(List<Match> globalMatches) {
         List<Neighbor> neighbors = new ArrayList<>();
+        List<Match> sortedByLeft = new ArrayList<>(globalMatches);
+        List<Match> sortedByRight = new ArrayList<>(globalMatches);
 
-        Map<Integer, List<Match>> matchesByLeft = new HashMap<>();
-        Map<Integer, List<Match>> matchesByRight = new HashMap<>();
+        sortedByLeft.sort(Comparator.comparingInt(Match::startOfFirst));
+        sortedByRight.sort(Comparator.comparingInt(Match::startOfSecond));
 
-        // Group matches by their left and right positions
-        for (Match match : globalMatches) {
-            matchesByLeft.computeIfAbsent(match.startOfFirst(), key -> new ArrayList<>()).add(match);
-            matchesByRight.computeIfAbsent(match.startOfSecond(), key -> new ArrayList<>()).add(match);
-        }
-
-        // Iterate through the matches and find neighbors
-        for (List<Match> matches : matchesByLeft.values()) {
-            for (Match match : matches) {
-                List<Match> rightMatches = matchesByRight.getOrDefault(match.startOfSecond(), Collections.emptyList());
-                for (Match rightMatch : rightMatches) {
-                    if (rightMatch != match) {
-                        neighbors.add(new Neighbor(match, rightMatch));
-                    }
-                }
+        for (int i = 0; i < sortedByLeft.size() - 1; i++) {
+            if (sortedByRight.indexOf(sortedByLeft.get(i)) == (sortedByRight.indexOf(sortedByLeft.get(i + 1)) - 1)) {
+                neighbors.add(new Neighbor(sortedByLeft.get(i), sortedByLeft.get(i + 1)));
             }
         }
 

core/src/test/java/de/jplag/merging/MergingTest.java

@@ -4,7 +4,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.function.Function;
 
@@ -33,15 +33,14 @@
  * CC BY 4.0 license.
  */
 class MergingTest extends TestBase {
-    private JPlagOptions options;
-    private JPlagResult result;
+    private final JPlagOptions options;
     private List<Match> matches;
     private List<JPlagComparison> comparisonsBefore;
     private List<JPlagComparison> comparisonsAfter;
-    private ComparisonStrategy comparisonStrategy;
-    private SubmissionSet submissionSet;
-    private final int MINIMUM_NEIGHBOR_LENGTH = 1;
-    private final int MAXIMUM_GAP_SIZE = 10;
+    private final ComparisonStrategy comparisonStrategy;
+    private final SubmissionSet submissionSet;
+    private static final int MINIMUM_NEIGHBOR_LENGTH = 1;
+    private static final int MAXIMUM_GAP_SIZE = 10;
 
     MergingTest() throws ExitException {
         options = getDefaultOptions("merging").withMergingOptions(new MergingOptions(true, MINIMUM_NEIGHBOR_LENGTH, MAXIMUM_GAP_SIZE));
@@ -55,13 +54,16 @@ class MergingTest extends TestBase {
 
     @BeforeEach
     void prepareTestState() {
-        result = comparisonStrategy.compareSubmissions(submissionSet);
-        comparisonsBefore = result.getAllComparisons();
+        JPlagResult result = comparisonStrategy.compareSubmissions(submissionSet);
+        comparisonsBefore = new ArrayList<>(result.getAllComparisons());
 
         if (options.mergingOptions().enabled()) {
             result = new MatchMerging(options).mergeMatchesOf(result);
         }
-        comparisonsAfter = result.getAllComparisons();
+        comparisonsAfter = new ArrayList<>(result.getAllComparisons());
+
+        comparisonsBefore.sort(Comparator.comparing(Object::toString));
+        comparisonsAfter.sort(Comparator.comparing(Object::toString));
     }
 
     @Test
@@ -83,10 +85,10 @@ void testGSTIgnoredMatches() {
     }
 
     private void checkMatchLength(Function<JPlagComparison, List<Match>> matchFunction, int threshold, List<JPlagComparison> comparisons) {
-        for (int i = 0; i < comparisons.size(); i++) {
-            matches = matchFunction.apply(comparisons.get(i));
-            for (int j = 0; j < matches.size(); j++) {
-                assertTrue(matches.get(j).length() >= threshold);
+        for (JPlagComparison comparison : comparisons) {
+            matches = matchFunction.apply(comparison);
+            for (Match match : matches) {
+                assertTrue(match.length() >= threshold);
             }
         }
     }
@@ -169,11 +171,11 @@ void testCorrectMerges() {
             matches = comparisonsAfter.get(i).matches();
             List<Match> sortedByFirst = new ArrayList<>(comparisonsBefore.get(i).matches());
             sortedByFirst.addAll(comparisonsBefore.get(i).ignoredMatches());
-            Collections.sort(sortedByFirst, (m1, m2) -> m1.startOfFirst() - m2.startOfFirst());
-            for (int j = 0; j < matches.size(); j++) {
+            sortedByFirst.sort(Comparator.comparingInt(Match::startOfFirst));
+            for (Match match : matches) {
                 int begin = -1;
                 for (int k = 0; k < sortedByFirst.size(); k++) {
-                    if (sortedByFirst.get(k).startOfFirst() == matches.get(j).startOfFirst()) {
+                    if (sortedByFirst.get(k).startOfFirst() == match.startOfFirst()) {
                         begin = k;
                         break;
                     }
@@ -182,10 +184,10 @@ void testCorrectMerges() {
                     correctMerges = false;
                 } else {
                     int foundToken = 0;
-                    while (foundToken < matches.get(j).length()) {
+                    while (foundToken < match.length()) {
                         foundToken += sortedByFirst.get(begin).length();
                         begin++;
-                        if (foundToken > matches.get(j).length()) {
+                        if (foundToken > match.length()) {
                             correctMerges = false;
                         }
                     }
@@ -194,4 +196,36 @@ void testCorrectMerges() {
         }
         assertTrue(correctMerges);
     }
+
+    @Test
+    @DisplayName("Sanity check for match merging")
+    void testSanity() {
+
+        List<Match> matchesBefore = new ArrayList<>();
+        List<Match> matchesAfter = new ArrayList<>();
+
+        for (JPlagComparison comparison : comparisonsBefore) {
+            if (comparison.toString().equals("sanityA.java <-> sanityB.java")) {
+                matchesBefore = comparison.ignoredMatches();
+            }
+        }
+        for (JPlagComparison comparison : comparisonsAfter) {
+            if (comparison.toString().equals("sanityA.java <-> sanityB.java")) {
+                matchesAfter = comparison.matches();
+            }
+        }
+
+        List<Match> expectedBefore = new ArrayList<>();
+        expectedBefore.add(new Match(5, 3, 6));
+        expectedBefore.add(new Match(11, 12, 6));
+        expectedBefore.add(new Match(0, 0, 3));
+        expectedBefore.add(new Match(3, 18, 2));
+        expectedBefore.add(new Match(17, 20, 2));
+
+        List<Match> expectedAfter = new ArrayList<>();
+        expectedAfter.add(new Match(5, 3, 12));
+
+        assertEquals(expectedBefore, matchesBefore);
+        assertEquals(expectedAfter, matchesAfter);
+    }
 }

core/src/test/resources/de/jplag/samples/merging/sanityA.java

@@ -0,0 +1,18 @@
+public class Minimal {
+  public static void main (String [] Argv) {
+    System.out.println("Test");
+    System.out.println("Test");
+    int a = 1;
+    a = 1;
+    int b = 1;
+    a = 1;
+    int c = 1;
+    a = 1;
+    int d = 1;
+    a = 1;
+    int e = 1;
+    a = 1;
+    int f = 1;
+    a = 1;
+  }
+}

core/src/test/resources/de/jplag/samples/merging/sanityB.java

@@ -0,0 +1,21 @@
+public class Minimal {
+  public static void main (String [] Argv) {
+    int a = 1;
+    a = 1;
+    int b = 1;
+    a = 1;
+    int c = 1;
+    a = 1;
+    if(a==1){
+      a = 2;
+    }
+    int d = 1;
+    a = 1;
+    int e = 1;
+    a = 1;
+    int f = 1;
+    a = 1;
+    System.out.println("Test");
+    System.out.println("Test");
+  }
+}