Programming exercises: Fix inconsistencies between diff viewer and di…

…ff line stats (#9984)

src/main/java/de/tum/cit/aet/artemis/programming/service/CommitHistoryService.java

@@ -107,7 +107,7 @@ private ProgrammingExerciseGitDiffReport createReport(Repository repository, Rev
 
             diffs.append(out.toString(StandardCharsets.UTF_8));
         }
-        var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diffs.toString(), false);
+        var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diffs.toString(), false, false);
         var report = new ProgrammingExerciseGitDiffReport();
         for (ProgrammingExerciseGitDiffEntry gitDiffEntry : programmingExerciseGitDiffEntries) {
             gitDiffEntry.setGitDiffReport(report);

src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseGitDiffReportService.java

@@ -109,7 +109,7 @@ public ProgrammingExerciseGitDiffReport updateReport(ProgrammingExercise program
 
         var templateHash = templateSubmission.getCommitHash();
         var solutionHash = solutionSubmission.getCommitHash();
-        var existingReport = this.getReportOfExercise(programmingExercise);
+        var existingReport = getReportOfExercise(programmingExercise);
         if (existingReport != null && canUseExistingReport(existingReport, templateHash, solutionHash)) {
             return existingReport;
         }
@@ -164,7 +164,7 @@ else if (reports.size() == 1) {
      * @return The report or null if none can be generated
      */
     public ProgrammingExerciseGitDiffReport getOrCreateReportOfExercise(ProgrammingExercise programmingExercise) {
-        var report = this.getReportOfExercise(programmingExercise);
+        var report = getReportOfExercise(programmingExercise);
         if (report == null) {
             return updateReport(programmingExercise);
         }
@@ -215,7 +215,7 @@ public int calculateNumberOfDiffLinesBetweenRepos(VcsRepositoryUri urlRepoA, Pat
         try (var diffOutputStream = new ByteArrayOutputStream(); var git = Git.wrap(repoB)) {
             git.diff().setOldTree(treeParserRepoB).setNewTree(treeParserRepoA).setOutputStream(diffOutputStream).call();
             var diff = diffOutputStream.toString();
-            return gitDiffReportParserService.extractDiffEntries(diff, true).stream().mapToInt(ProgrammingExerciseGitDiffEntry::getLineCount).sum();
+            return gitDiffReportParserService.extractDiffEntries(diff, true, false).stream().mapToInt(ProgrammingExerciseGitDiffEntry::getLineCount).sum();
         }
         catch (IOException | GitAPIException e) {
             log.error("Error calculating number of diff lines between repositories: urlRepoA={}, urlRepoB={}.", urlRepoA, urlRepoB, e);
@@ -234,6 +234,7 @@ public int calculateNumberOfDiffLinesBetweenRepos(VcsRepositoryUri urlRepoA, Pat
      */
     private ProgrammingExerciseGitDiffReport generateReport(TemplateProgrammingExerciseParticipation templateParticipation,
             SolutionProgrammingExerciseParticipation solutionParticipation) throws GitAPIException, IOException {
+        // TODO: in case of LocalVC, we should calculate the diff in the bare origin repository
         Repository templateRepo = prepareTemplateRepository(templateParticipation);
         var solutionRepo = gitService.getOrCheckoutRepository(solutionParticipation.getVcsRepositoryUri(), true);
         gitService.resetToOriginHead(solutionRepo);
@@ -306,19 +307,20 @@ private ProgrammingExerciseGitDiffReport parseFilesAndCreateReport(Repository re
      * It parses all files of the repositories in their directories on the file system and creates a report containing the changes.
      * Both repositories have to be checked out at the commit that should be compared and be in different directories
      *
-     * @param repo1         The first repository
-     * @param oldTreeParser The tree parser for the first repository
-     * @param newTreeParser The tree parser for the second repository
+     * @param firstRepo            The first repository
+     * @param firstRepoTreeParser  The tree parser for the first repository
+     * @param secondRepoTreeParser The tree parser for the second repository
      * @return The report with the changes between the two repositories at their checked out state
      * @throws IOException     If an error occurs while accessing the file system
      * @throws GitAPIException If an error occurs while accessing the git repository
      */
     @NotNull
-    private ProgrammingExerciseGitDiffReport createReport(Repository repo1, FileTreeIterator oldTreeParser, FileTreeIterator newTreeParser) throws IOException, GitAPIException {
-        try (ByteArrayOutputStream diffOutputStream = new ByteArrayOutputStream(); Git git = Git.wrap(repo1)) {
-            git.diff().setOldTree(oldTreeParser).setNewTree(newTreeParser).setOutputStream(diffOutputStream).call();
+    private ProgrammingExerciseGitDiffReport createReport(Repository firstRepo, FileTreeIterator firstRepoTreeParser, FileTreeIterator secondRepoTreeParser)
+            throws IOException, GitAPIException {
+        try (ByteArrayOutputStream diffOutputStream = new ByteArrayOutputStream(); Git git = Git.wrap(firstRepo)) {
+            git.diff().setOldTree(firstRepoTreeParser).setNewTree(secondRepoTreeParser).setOutputStream(diffOutputStream).call();
             var diff = diffOutputStream.toString();
-            var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diff, false);
+            var programmingExerciseGitDiffEntries = gitDiffReportParserService.extractDiffEntries(diff, false, true);
             var report = new ProgrammingExerciseGitDiffReport();
             for (ProgrammingExerciseGitDiffEntry gitDiffEntry : programmingExerciseGitDiffEntries) {
                 gitDiffEntry.setGitDiffReport(report);

src/main/java/de/tum/cit/aet/artemis/programming/web/GitDiffReportParserService.java

@@ -23,16 +23,17 @@ public class GitDiffReportParserService {
 
     private static final String PREFIX_RENAME_TO = "rename to ";
 
-    private final Pattern gitDiffLinePattern = Pattern.compile("@@ -(?<previousLine>\\d+)(,(?<previousLineCount>\\d+))? \\+(?<newLine>\\d+)(,(?<newLineCount>\\d+))? @@");
+    private final Pattern gitDiffLinePattern = Pattern.compile("@@ -(?<previousLine>\\d+)(,(?<previousLineCount>\\d+))? \\+(?<newLine>\\d+)(,(?<newLineCount>\\d+))? @@.*");
 
     /**
      * Extracts the ProgrammingExerciseGitDiffEntry from the raw git-diff output
      *
      * @param diff                 The raw git-diff output
      * @param useAbsoluteLineCount Whether to use absolute line count or previous line count
+     * @param ignoreWhitespace     Whether to ignore entries where only leading and trailing whitespace differ
      * @return The extracted ProgrammingExerciseGitDiffEntries
      */
-    public List<ProgrammingExerciseGitDiffEntry> extractDiffEntries(String diff, boolean useAbsoluteLineCount) {
+    public List<ProgrammingExerciseGitDiffEntry> extractDiffEntries(String diff, boolean useAbsoluteLineCount, boolean ignoreWhitespace) {
         var lines = diff.split("\n");
         var parserState = new ParserState();
         Map<String, String> renamedFilePaths = new HashMap<>();
@@ -44,8 +45,7 @@ public List<ProgrammingExerciseGitDiffEntry> extractDiffEntries(String diff, boo
                 continue;
             }
 
-            // Files may be renamed without changes, in which case the lineMatcher will never match the entry
-            // We store this information separately so it is not lost
+            // Check for renamed files
             if (line.startsWith(PREFIX_RENAME_FROM) && i + 1 < lines.length) {
                 var nextLine = lines[i + 1];
                 if (nextLine.startsWith(PREFIX_RENAME_TO)) {
@@ -57,75 +57,81 @@ public List<ProgrammingExerciseGitDiffEntry> extractDiffEntries(String diff, boo
 
             var lineMatcher = gitDiffLinePattern.matcher(line);
             if (lineMatcher.matches()) {
-                handleNewDiffBlock(lines, i, parserState, lineMatcher);
+                handleNewDiffBlock(lines, i, parserState, lineMatcher, ignoreWhitespace);
             }
-            else if (!parserState.deactivateCodeReading) {
+            else if (!parserState.deactivateCodeReading && !line.isEmpty()) {
                 switch (line.charAt(0)) {
-                    case '+' -> handleAddition(parserState);
-                    case '-' -> handleRemoval(parserState, useAbsoluteLineCount);
-                    case ' ' -> handleUnchanged(parserState);
+                    case '+' -> handleAddition(parserState, line);
+                    case '-' -> handleRemoval(parserState, useAbsoluteLineCount, line);
+                    case ' ' -> handleUnchanged(parserState, ignoreWhitespace);
                     default -> parserState.deactivateCodeReading = true;
                 }
             }
         }
-        if (!parserState.currentEntry.isEmpty()) {
-            parserState.entries.add(parserState.currentEntry);
-        }
-        // Add an empty diff entry for renamed files without changes
+
+        // Check the last entry
+        finalizeEntry(parserState, ignoreWhitespace);
+
+        // Add empty entries for renamed files without changes
         for (var entry : renamedFilePaths.entrySet()) {
             var diffEntry = new ProgrammingExerciseGitDiffEntry();
             diffEntry.setFilePath(entry.getValue());
             diffEntry.setPreviousFilePath(entry.getKey());
             parserState.entries.add(diffEntry);
         }
+
         return parserState.entries;
     }
 
-    private void handleNewDiffBlock(String[] lines, int currentLine, ParserState parserState, Matcher lineMatcher) {
-        if (!parserState.currentEntry.isEmpty()) {
-            parserState.entries.add(parserState.currentEntry);
-        }
+    private void handleNewDiffBlock(String[] lines, int currentLine, ParserState parserState, Matcher lineMatcher, boolean ignoreWhitespace) {
+        finalizeEntry(parserState, ignoreWhitespace);
+
         // Start of a new file
         var newFilePath = getFilePath(lines, currentLine);
         var newPreviousFilePath = getPreviousFilePath(lines, currentLine);
         if (newFilePath != null || newPreviousFilePath != null) {
             parserState.currentFilePath = newFilePath;
             parserState.currentPreviousFilePath = newPreviousFilePath;
         }
+
         parserState.currentEntry = new ProgrammingExerciseGitDiffEntry();
         parserState.currentEntry.setFilePath(parserState.currentFilePath);
         parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath);
         parserState.currentLineCount = Integer.parseInt(lineMatcher.group("newLine"));
         parserState.currentPreviousLineCount = Integer.parseInt(lineMatcher.group("previousLine"));
         parserState.deactivateCodeReading = false;
+        parserState.addedLines.clear();
+        parserState.removedLines.clear();
     }
 
-    private void handleUnchanged(ParserState parserState) {
-        var entry = parserState.currentEntry;
-        if (!entry.isEmpty()) {
-            parserState.entries.add(entry);
-        }
-        entry = new ProgrammingExerciseGitDiffEntry();
-        entry.setFilePath(parserState.currentFilePath);
-        entry.setPreviousFilePath(parserState.currentPreviousFilePath);
+    private void handleUnchanged(ParserState parserState, boolean ignoreWhitespace) {
+        finalizeEntry(parserState, ignoreWhitespace);
+        parserState.currentEntry = new ProgrammingExerciseGitDiffEntry();
+        parserState.currentEntry.setFilePath(parserState.currentFilePath);
+        parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath);
 
-        parserState.currentEntry = entry;
         parserState.lastLineRemoveOperation = false;
         parserState.currentLineCount++;
         parserState.currentPreviousLineCount++;
+        parserState.addedLines.clear();
+        parserState.removedLines.clear();
     }
 
-    private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount) {
+    private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount, String line) {
         var entry = parserState.currentEntry;
         if (!parserState.lastLineRemoveOperation && !entry.isEmpty()) {
-            parserState.entries.add(entry);
-            entry = new ProgrammingExerciseGitDiffEntry();
-            entry.setFilePath(parserState.currentFilePath);
-            entry.setPreviousFilePath(parserState.currentPreviousFilePath);
+            finalizeEntry(parserState, false);
+            parserState.currentEntry = new ProgrammingExerciseGitDiffEntry();
+            parserState.currentEntry.setFilePath(parserState.currentFilePath);
+            parserState.currentEntry.setPreviousFilePath(parserState.currentPreviousFilePath);
         }
-        if (entry.getPreviousLineCount() == null) {
-            entry.setPreviousLineCount(0);
-            entry.setPreviousStartLine(parserState.currentPreviousLineCount);
+
+        // Store removed line
+        parserState.removedLines.add(line.substring(1));
+
+        if (parserState.currentEntry.getPreviousLineCount() == null) {
+            parserState.currentEntry.setPreviousLineCount(0);
+            parserState.currentEntry.setPreviousStartLine(parserState.currentPreviousLineCount);
         }
         if (useAbsoluteLineCount) {
             if (parserState.currentEntry.getLineCount() == null) {
@@ -135,15 +141,17 @@ private void handleRemoval(ParserState parserState, boolean useAbsoluteLineCount
             parserState.currentEntry.setLineCount(parserState.currentEntry.getLineCount() + 1);
         }
         else {
-            entry.setPreviousLineCount(entry.getPreviousLineCount() + 1);
+            parserState.currentEntry.setPreviousLineCount(parserState.currentEntry.getPreviousLineCount() + 1);
         }
 
-        parserState.currentEntry = entry;
         parserState.lastLineRemoveOperation = true;
         parserState.currentPreviousLineCount++;
     }
 
-    private void handleAddition(ParserState parserState) {
+    private void handleAddition(ParserState parserState, String line) {
+        // Store added line
+        parserState.addedLines.add(line.substring(1));
+
         if (parserState.currentEntry.getLineCount() == null) {
             parserState.currentEntry.setLineCount(0);
             parserState.currentEntry.setStartLine(parserState.currentLineCount);
@@ -154,6 +162,29 @@ private void handleAddition(ParserState parserState) {
         parserState.currentLineCount++;
     }
 
+    private void finalizeEntry(ParserState parserState, boolean ignoreWhitespace) {
+        if (!parserState.currentEntry.isEmpty()) {
+            if (!ignoreWhitespace || !isWhitespaceOnlyChange(parserState.addedLines, parserState.removedLines)) {
+                parserState.entries.add(parserState.currentEntry);
+            }
+        }
+    }
+
+    private boolean isWhitespaceOnlyChange(List<String> addedLines, List<String> removedLines) {
+        if (addedLines.size() != removedLines.size()) {
+            return false; // Different number of lines changed, definitely not whitespace only
+        }
+
+        for (int i = 0; i < addedLines.size(); i++) {
+            String added = addedLines.get(i).trim();
+            String removed = removedLines.get(i).trim();
+            if (!added.equals(removed)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * Extracts the file path from the raw git-diff for a specified diff block
      *
@@ -219,13 +250,19 @@ private static class ParserState {
 
         private int currentPreviousLineCount;
 
+        private final List<String> addedLines;
+
+        private final List<String> removedLines;
+
         public ParserState() {
             entries = new ArrayList<>();
             currentEntry = new ProgrammingExerciseGitDiffEntry();
             deactivateCodeReading = true;
             lastLineRemoveOperation = false;
             currentLineCount = 0;
             currentPreviousLineCount = 0;
+            addedLines = new ArrayList<>();
+            removedLines = new ArrayList<>();
         }
     }
 }

src/test/java/de/tum/cit/aet/artemis/UnionFindTest.java → src/test/java/de/tum/cit/aet/artemis/atlas/UnionFindTest.java

@@ -1,4 +1,4 @@
-package de.tum.cit.aet.artemis;
+package de.tum.cit.aet.artemis.atlas;
 
 import static org.assertj.core.api.Assertions.assertThat;