Refactor the DiffTreeParser and upgrade to JUnit5

pom.xml

@@ -36,6 +36,17 @@
                 </configuration>
             </plugin>
 
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.0.0-M7</version>
+                <configuration>
+                    <includes>
+                        <include>**/*.java</include>
+                    </includes>
+                </configuration>
+            </plugin>
+
             <!-- To build jar with dependencies-->
             <plugin>
                 <artifactId>maven-assembly-plugin</artifactId>
@@ -122,10 +133,16 @@
         </dependency>
 
         <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.13.2</version>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <version>5.9.1</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>5.9.1</version>
             <scope>test</scope>
         </dependency>
     </dependencies>
-</project>
+</project>

src/main/java/org/variantsync/diffdetective/analysis/AnalysisResult.java

@@ -239,9 +239,13 @@ public static AnalysisResult importFrom(final Path p, final Map<String, BiConsum
                         } else if (key.startsWith(ExplainedFilterSummary.FILTERED_MESSAGE_BEGIN)) {
                             filterHitsLines.add(line);
                         } else if (key.startsWith(ERROR_BEGIN)) {
-                            DiffError e = new DiffError(key.substring(ERROR_BEGIN.length(), key.length() - ERROR_END.length()));
+                            var errorId = key.substring(ERROR_BEGIN.length(), key.length() - ERROR_END.length());
+                            var e = DiffError.fromMessage(errorId);
+                            if (e.isEmpty()) {
+                                throw new RuntimeException("Invalid error id " + errorId + " while importing " + p);
+                            }
                             // add DiffError
-                            result.diffErrors.put(e, Integer.parseInt(value));
+                            result.diffErrors.put(e.get(), Integer.parseInt(value));
                         } else {
                             final BiConsumer<AnalysisResult, String> customParser = customParsers.get(key);
                             if (customParser == null) {

src/main/java/org/variantsync/diffdetective/datasets/ParseOptions.java

@@ -1,14 +1,14 @@
 package org.variantsync.diffdetective.datasets;
 
-import org.variantsync.diffdetective.diff.difftree.parse.DiffNodeParser;
+import org.variantsync.diffdetective.feature.CPPAnnotationParser;
 
 /**
  * Parse options that should be used when parsing commits and patches within a commit history.
  * @param diffStoragePolicy Decides if and how unix diffs should be remembered when parsing commits.
  * @param annotationParser A parser for parsing c preprocessor annotations.
  * @author Paul Bittner
  */
-public record ParseOptions(DiffStoragePolicy diffStoragePolicy, DiffNodeParser annotationParser) {
+public record ParseOptions(DiffStoragePolicy diffStoragePolicy, CPPAnnotationParser annotationParser) {
     public enum DiffStoragePolicy {
         REMEMBER_DIFF,
         REMEMBER_STRIPPED_DIFF,
@@ -21,13 +21,13 @@ public enum DiffStoragePolicy {
      * @see ParseOptions#Default
      * @param annotationParser A parser for parsing c preprocessor annotations.
      */
-    public ParseOptions(DiffNodeParser annotationParser) {
+    public ParseOptions(CPPAnnotationParser annotationParser) {
         this(Default.diffStoragePolicy, annotationParser);
     }
 
     /**
      * Creates ParseOptions with the given policy for storing diffs.
-     * @see ParseOptions#ParseOptions(DiffStoragePolicy, DiffNodeParser)
+     * @see ParseOptions#ParseOptions(DiffStoragePolicy, CPPAnnotationParser)
      * @param diffStoragePolicy Decides if and how unix diffs should be remembered when parsing commits.
      */
     public ParseOptions withDiffStoragePolicy(DiffStoragePolicy diffStoragePolicy) {
@@ -36,10 +36,10 @@ public ParseOptions withDiffStoragePolicy(DiffStoragePolicy diffStoragePolicy) {
 
     /**
      * Default value for ParseOptions that does not remember parsed unix diffs
-     * and uses the default value for the parsing annotations ({@link DiffNodeParser#Default}).
+     * and uses the default value for the parsing annotations ({@link CPPAnnotationParser#Default}).
      */
     public static final ParseOptions Default = new ParseOptions(
             DiffStoragePolicy.DO_NOT_REMEMBER,
-            DiffNodeParser.Default
+            CPPAnnotationParser.Default
     );
 }

src/main/java/org/variantsync/diffdetective/datasets/predefined/Marlin.java

@@ -2,7 +2,6 @@
 
 import org.variantsync.diffdetective.datasets.ParseOptions;
 import org.variantsync.diffdetective.datasets.Repository;
-import org.variantsync.diffdetective.diff.difftree.parse.DiffNodeParser;
 import org.variantsync.diffdetective.feature.CPPAnnotationParser;
 import org.variantsync.diffdetective.feature.PropositionalFormulaParser;
 
@@ -14,9 +13,11 @@
  * @author Kevin Jedelhauser, Paul Maximilian Bittner
  */
 public class Marlin {
-    public static final DiffNodeParser ANNOTATION_PARSER = new DiffNodeParser(
-            new CPPAnnotationParser(PropositionalFormulaParser.Default, new MarlinCPPDiffLineFormulaExtractor())
-    );
+    public static final CPPAnnotationParser ANNOTATION_PARSER =
+        new CPPAnnotationParser(
+            PropositionalFormulaParser.Default,
+            new MarlinCPPDiffLineFormulaExtractor()
+        );
 
     /**
      * Clones Marlin from Github.

src/main/java/org/variantsync/diffdetective/diff/DiffLineNumber.java

@@ -12,12 +12,11 @@
  * - the corresponding line number after the edit
  * @author Paul Bittner
  */
-public class DiffLineNumber {
+public record DiffLineNumber(int inDiff, int beforeEdit, int afterEdit) {
     /**
      * Index for invalid line numbers.
      */
     public static final int InvalidLineNumber = -1;
-    public int inDiff, beforeEdit, afterEdit;
 
     /**
      * Creates a line number of a diff.
@@ -41,34 +40,26 @@ public static DiffLineNumber Invalid() {
     }
 
     /**
-     * Creates a copy of the given DiffLineNumber.
-     */
-    public static DiffLineNumber Copy(final DiffLineNumber other) {
-        return new DiffLineNumber(other.inDiff, other.beforeEdit, other.afterEdit);
-    }
-
-    /**
-     * Make this line number become a copy of the given line number.
-     * @param other Number to copy. Remains unchanged.
-     * @return this
+     * Shifts this line number by adding the given offset.
+     * @param offset value to add to this line number.
+     * @return a new {@code DiffLineNumber} shifted by {@code offset}
      */
-    public DiffLineNumber set(final DiffLineNumber other) {
-        this.inDiff = other.inDiff;
-        this.beforeEdit = other.beforeEdit;
-        this.afterEdit = other.afterEdit;
-        return this;
+    public DiffLineNumber add(int offset) {
+        return add(offset, DiffType.NON);
     }
 
     /**
      * Shifts this line number by adding the given offset.
-     * @param offset Value to add to this line number.
-     * @return this
+     * @param offset value to add to this line number.
+     * @param diffType specifies which components are shifted
+     * @return a new {@code DiffLineNumber} shifted by {@code offset}
      */
-    public DiffLineNumber add(int offset) {
-        this.inDiff += offset;
-        this.beforeEdit += offset;
-        this.afterEdit += offset;
-        return this;
+    public DiffLineNumber add(int offset, DiffType diffType) {
+        return new DiffLineNumber(
+            inDiff + offset,
+            beforeEdit + (diffType == DiffType.ADD ? 0 : offset),
+            afterEdit + (diffType == DiffType.REM ? 0 : offset)
+        );
     }
 
     /**
@@ -78,12 +69,12 @@ public DiffLineNumber add(int offset) {
      * Non-existing values will be set to {@link DiffLineNumber#InvalidLineNumber}.
      * @param diffType The diff type according to which this line number should be filtered.
      */
-    public void as(final DiffType diffType) {
-        if (diffType == DiffType.ADD) {
-            beforeEdit = InvalidLineNumber;
-        } else if (diffType == DiffType.REM) {
-            afterEdit = InvalidLineNumber;
-        }
+    public DiffLineNumber as(final DiffType diffType) {
+        return new DiffLineNumber(
+            inDiff,
+            diffType == DiffType.ADD ? InvalidLineNumber : beforeEdit,
+            diffType == DiffType.REM ? InvalidLineNumber : afterEdit
+        );
     }
 
     @Override

src/main/java/org/variantsync/diffdetective/diff/GitDiffer.java

@@ -22,10 +22,9 @@
 import org.variantsync.diffdetective.diff.difftree.parse.DiffTreeParser;
 import org.variantsync.diffdetective.diff.result.CommitDiffResult;
 import org.variantsync.diffdetective.diff.result.DiffError;
-import org.variantsync.diffdetective.diff.result.DiffResult;
+import org.variantsync.diffdetective.diff.result.DiffParseException;
 import org.variantsync.diffdetective.preliminary.GitDiff;
 import org.variantsync.diffdetective.util.StringUtils;
-import org.variantsync.functjonal.Result;
 import org.variantsync.functjonal.iteration.MappedIterator;
 import org.variantsync.functjonal.iteration.SideEffectIterator;
 import org.variantsync.functjonal.iteration.Yield;
@@ -308,18 +307,26 @@ private static CommitDiffResult getPatchDiffs(
 
                 diffFormatter.format(diffEntry);
                 final String gitDiff = outputStream.toString(StandardCharsets.UTF_8);
-                final Result<PatchDiff, DiffError> patchDiff =
-                        getBeforeFullFile(git, parentCommit, diffEntry.getOldPath()).unwrap()
-                        .bind(file -> createPatchDiff(
-                                commitDiff,
-                                diffEntry,
-                                gitDiff,
-                                file,
-                                parseOptions).unwrap()
+                final String filename = diffEntry.getOldPath();
+                try {
+                    final BufferedReader file = getBeforeFullFile(git, parentCommit, filename);
+                    final PatchDiff patchDiff =
+                        createPatchDiff(
+                            commitDiff,
+                            diffEntry,
+                            gitDiff,
+                            file,
+                            parseOptions
                         );
 
-                patchDiff.ifSuccess(commitDiff::addPatchDiff);
-                patchDiff.ifFailure(errors::add);
+                    commitDiff.addPatchDiff(patchDiff);
+                } catch (IOException e) {
+                    Logger.debug(e, "Could not obtain full diff of file " + filename + " before commit " + parentCommit + "!");
+                    errors.add(DiffError.COULD_NOT_OBTAIN_FULLDIFF);
+                } catch (DiffParseException e) {
+                    errors.add(e.getError());
+                }
+
                 outputStream.reset();
             }
         } catch (IOException e) {
@@ -337,13 +344,15 @@ private static CommitDiffResult getPatchDiffs(
      * @param gitDiff        The git diff of the file that was changed
      * @param beforeFullFile The full file before the change
      * @return The PatchDiff of the given DiffEntry
+     * @throws DiffParseException if {@code gitDiff} couldn't be parsed
      */
-    private static DiffResult<PatchDiff> createPatchDiff(
+    private static PatchDiff createPatchDiff(
             CommitDiff commitDiff,
             DiffEntry diffEntry,
             String gitDiff,
             BufferedReader beforeFullFile,
-            final ParseOptions parseOptions) {
+            final ParseOptions parseOptions
+    ) throws DiffParseException {
         final Matcher matcher = DIFF_HEADER_PATTERN.matcher(gitDiff);
         final String strippedDiff;
         if (matcher.find()) {
@@ -353,14 +362,9 @@ private static DiffResult<PatchDiff> createPatchDiff(
         }
 
         final String fullDiff = getFullDiff(beforeFullFile, new BufferedReader(new StringReader(strippedDiff)));
-        final DiffResult<DiffTree> diffTree = DiffTreeParser.createDiffTree(fullDiff, true, true, parseOptions.annotationParser());
-
-//        if (diffTree.isFailure()) {
-//            Logger.debug("Something went wrong parsing patch for file {} at commit {}!",
-//                    diffEntry.getOldPath(), commitDiff.getAbbreviatedCommitHash());
-//        }
+        try {
+            DiffTree diffTree = DiffTreeParser.createDiffTree(fullDiff, true, true, parseOptions.annotationParser());
 
-        return diffTree.map(t -> {
             // not storing the full diff reduces memory usage by around 40-50%
             final String diffToRemember = switch (parseOptions.diffStoragePolicy()) {
                 case DO_NOT_REMEMBER -> "";
@@ -369,8 +373,14 @@ private static DiffResult<PatchDiff> createPatchDiff(
                 case REMEMBER_STRIPPED_DIFF -> strippedDiff;
             };
 
-            return new PatchDiff(commitDiff, diffEntry, diffToRemember, t);
-        });
+            return new PatchDiff(commitDiff, diffEntry, diffToRemember, diffTree);
+        } catch (DiffParseException e) {
+            // if (diffTree.isFailure()) {
+            //     Logger.debug(e, "Something went wrong parsing patch for file {} at commit {}!",
+            //             diffEntry.getOldPath(), commitDiff.getAbbreviatedCommitHash());
+            // }
+            throw e;
+        }
     }
 
     /**
@@ -435,7 +445,7 @@ public static String getFullDiff(BufferedReader beforeFile, BufferedReader gitDi
      * @param filename The name of the file
      * @return The full content of the file before the commit
      */
-    public static DiffResult<BufferedReader> getBeforeFullFile(Git git, RevCommit commit, String filename) {
+    public static BufferedReader getBeforeFullFile(Git git, RevCommit commit, String filename) throws IOException {
         RevTree tree = commit.getTree();
 
         try (TreeWalk treeWalk = new TreeWalk(git.getRepository())) {
@@ -445,14 +455,12 @@ public static DiffResult<BufferedReader> getBeforeFullFile(Git git, RevCommit co
 
             // Look for the first file that matches filename.
             if (!treeWalk.next()) {
-                return DiffResult.Failure(DiffError.COULD_NOT_OBTAIN_FULLDIFF, "Could not obtain full diff of file " + filename + " before commit " + commit + "!");
+                throw new FileNotFoundException("Couldn't find " + filename + " in the commit " + commit);
             }
 
             ObjectId objectId = treeWalk.getObjectId(0);
             ObjectLoader loader = git.getRepository().open(objectId);
-            return DiffResult.Success(new BufferedReader(new InputStreamReader(loader.openStream())));
-        } catch (IOException e) {
-            return DiffResult.Failure(DiffError.COULD_NOT_OBTAIN_FULLDIFF, "Could not obtain full diff of file " + filename + " before commit " + commit + "!");
+            return new BufferedReader(new InputStreamReader(loader.openStream()));
         }
     }