Issue #532: optimize DiffReport

patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java

@@ -19,6 +19,8 @@
 
 package com.github.checkstyle.data;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -30,14 +32,21 @@
  * @author attatrol
  *
  */
-public final class CheckstyleRecord {
+public final class CheckstyleRecord implements Comparable<CheckstyleRecord> {
 
     /**
      * It is usual for sources of records to have name that
      * matches this pattern. It is used for shortening source names.
      */
     private static final Pattern CHECKSTYLE_CHECK_NAME = Pattern.compile(".+Check");
 
+    /**
+     * Predefined severities for sorting. All other severities has lower priority
+     * and will be arranged in the default order for strings.
+     */
+    private static final List<String> PREDEFINED_SEVERITIES =
+        Arrays.asList("info", "warning", "error");
+
     /**
      * Length of "Check" string.
      */
@@ -177,17 +186,67 @@ public String getSimpleCuttedSourceName() {
 
     /**
      * Compares CheckstyleRecord instances by their content.
-     * It is used in a single controlled occasion in the code.
+     * The order is source, line, column, severity, message.
+     * Properties index and xref are ignored.
      *
      * @param other
-     *        another ChechstyleRecord instance under comparison
+     *        another CheckstyleRecord instance under comparison
      *        with this instance.
-     * @return true if instances are equal.
+     * @return 0 if the objects are equal, a negative integer if this record is before the specified
+     *         record, or a positive integer if this record is after the specified record.
      */
-    public boolean specificEquals(final CheckstyleRecord other) {
-        return this.line == other.line && this.column == other.column
-                && this.source.equals(other.source)
-                && this.message.equals(other.message);
+    public int compareTo(final CheckstyleRecord other) {
+        int diff = Integer.compare(this.line, other.line);
+        if (diff == 0) {
+            diff = Integer.compare(this.column, other.column);
+        }
+        if (diff == 0) {
+            diff = compareSeverity(this.severity, other.severity);
+        }
+        if (diff == 0) {
+            diff = this.message.compareTo(other.message);
+        }
+        if (diff == 0) {
+            diff = this.source.compareTo(other.source);
+        }
+        return diff;
+    }
+
+    /**
+     * Compares record severities in the order "info", "warning", "error", all other.
+     *
+     * @param severity1 first severity
+     * @param severity2 second severity
+     * @return the value {@code 0} if both severities are the same
+     *         a value less than {@code 0} if severity1 should be first and
+     *         a value greater than {@code 0} if severity2 should be first
+     */
+    private int compareSeverity(String severity1, String severity2) {
+        final int result;
+        if (severity1.equals(severity2)) {
+            result = 0;
+        }
+        else {
+            final int index1 = PREDEFINED_SEVERITIES.indexOf(severity1);
+            final int index2 = PREDEFINED_SEVERITIES.indexOf(severity2);
+            if (index1 < 0 && index2 < 0) {
+                // Both severity levels are unknown, so use regular order for strings.
+                result = severity1.compareTo(severity2);
+            }
+            else if (index1 < 0) {
+                // First is unknown, second is known: second before
+                result = 1;
+            }
+            else if (index2 < 0) {
+                // First is known, second is unknown: first before
+                result = -1;
+            }
+            else {
+                // Both result are well-known, use predefined order
+                result = Integer.compare(index1, index2);
+            }
+        }
+        return result;
     }
 
 }

patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java

@@ -19,9 +19,7 @@
 
 package com.github.checkstyle.data;
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
@@ -76,111 +74,47 @@ public Statistics getStatistics() {
     public void addRecords(List<CheckstyleRecord> newRecords,
             String filename) {
         if (!newRecords.isEmpty()) {
+            Collections.sort(newRecords);
             final List<CheckstyleRecord> popped =
                 records.put(filename, newRecords);
             if (popped != null) {
                 final List<CheckstyleRecord> diff =
-                    produceDiff(popped, newRecords);
+                    DiffUtils.produceDiff(popped, newRecords);
                 if (diff.isEmpty()) {
                     records.remove(filename);
                 }
                 else {
-                    Collections.sort(diff, new PositionOrderComparator());
                     records.put(filename, diff);
                 }
             }
         }
     }
 
-    /**
-     * Creates difference between 2 lists of records.
-     *
-     * @param list1
-     *        the first list.
-     * @param list2
-     *        the second list.
-     * @return the difference list.
-     */
-    private static List<CheckstyleRecord> produceDiff(
-            List<CheckstyleRecord> list1, List<CheckstyleRecord> list2) {
-        final List<CheckstyleRecord> diff = new ArrayList<>();
-        for (CheckstyleRecord rec1 : list1) {
-            if (!isInList(list2, rec1)) {
-                diff.add(rec1);
-            }
-        }
-        for (CheckstyleRecord rec2 : list2) {
-            if (!isInList(list1, rec2)) {
-                diff.add(rec2);
-            }
-        }
-        return diff;
-    }
-
-    /**
-     * Compares the record against list of records.
-     *
-     * @param list
-     *        of records.
-     * @param testedRecord
-     *        the record.
-     * @return true, if has its copy in a list.
-     */
-    private static boolean isInList(List<CheckstyleRecord> list,
-            CheckstyleRecord testedRecord) {
-        boolean belongsToList = false;
-        for (CheckstyleRecord checkstyleRecord : list) {
-            if (testedRecord.specificEquals(checkstyleRecord)) {
-                belongsToList = true;
-                break;
-            }
-        }
-        return belongsToList;
-    }
-
     /**
      * Generates statistical information and puts in in the accumulator.
+     * This method will wait for completion of all asynchronous tasks.
      */
     public void getDiffStatistics() {
         statistics.setFileNumDiff(records.size());
-        for (Map.Entry<String, List<CheckstyleRecord>> entry
-                : records.entrySet()) {
-            final List<CheckstyleRecord> list = entry.getValue();
-            for (CheckstyleRecord rec : list) {
-                if (rec.getIndex() == CheckstyleReportsParser.BASE_REPORT_INDEX) {
-                    statistics.addSeverityRecordRemoved(rec.getSeverity());
-                    statistics.addModuleRecordRemoved(rec.getSource());
-                }
-                else {
-                    statistics.addSeverityRecordAdded(rec.getSeverity());
-                    statistics.addModuleRecordAdded(rec.getSource());
-                }
-                statistics.incrementUniqueMessageCount(rec.getIndex());
-            }
-        }
+        records.entrySet().stream()
+            .flatMap(entry -> entry.getValue().stream())
+            .forEach(this::addRecordStatistics);
     }
 
     /**
-     * Comparator used to sort lists of CheckstyleRecord objects
-     * by their position in code.
-     *
-     * @author atta_troll
+     * Generates statistical information for one CheckstyleRecord.
      *
+     * @param checkstyleRecord the checkstyleRecord to process
      */
-    private static class PositionOrderComparator
-        implements Comparator<CheckstyleRecord> {
-
-        @Override
-        public int compare(final CheckstyleRecord arg0,
-                final CheckstyleRecord arg1) {
-            final int difference = arg0.getLine() - arg1.getLine();
-            if (difference == 0) {
-                return arg0.getColumn() - arg1.getColumn();
-            }
-            else {
-                return difference;
-            }
+    private void addRecordStatistics(CheckstyleRecord checkstyleRecord) {
+        if (checkstyleRecord.getIndex() == CheckstyleReportsParser.BASE_REPORT_INDEX) {
+            statistics.addSeverityRecordRemoved(checkstyleRecord.getSeverity());
+            statistics.addModuleRecordRemoved(checkstyleRecord.getSource());
+        }
+        else {
+            statistics.addSeverityRecordAdded(checkstyleRecord.getSeverity());
+            statistics.addModuleRecordAdded(checkstyleRecord.getSource());
         }
+        statistics.incrementUniqueMessageCount(checkstyleRecord.getIndex());
     }
-
 }

patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffUtils.java

@@ -0,0 +1,112 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2021 the original author or authors.
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+////////////////////////////////////////////////////////////////////////////////
+
+package com.github.checkstyle.data;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Utility class to calculate difference between 2 sorted lists.
+ */
+public final class DiffUtils {
+
+    /** Private ctor. */
+    private DiffUtils() {
+    }
+
+    /**
+     * Creates difference between 2 sorted lists.
+     *
+     * @param firstList
+     *        the first list.
+     * @param secondList
+     *        the second list.
+     * @param <T> the type of elements.
+     * @return the difference list.
+     */
+    public static <T extends Comparable<T>> List<T> produceDiff(
+            List<T> firstList, List<T> secondList) {
+        final List<T> result;
+        if (firstList.isEmpty()) {
+            result = secondList;
+        }
+        else if (secondList.isEmpty()) {
+            result = firstList;
+        }
+        else {
+            result = produceDiff(firstList.iterator(), secondList.iterator());
+        }
+        return result;
+    }
+
+    /**
+     * Creates difference between 2 non-empty iterators.
+     *
+     * @param firstIterator
+     *        the first iterator.
+     * @param secondIterator
+     *        the second iterator.
+     * @param <T> the type of elements.
+     * @return the difference list (always sorted).
+     */
+    private static <T extends Comparable<T>> List<T> produceDiff(
+            Iterator<T> firstIterator, Iterator<T> secondIterator) {
+        T firstVal = firstIterator.next();
+        T secondVal = secondIterator.next();
+        final List<T> result = new ArrayList<>();
+        while (true) {
+            final int diff = firstVal.compareTo(secondVal);
+            if (diff < 0) {
+                result.add(firstVal);
+                if (!firstIterator.hasNext()) {
+                    result.add(secondVal);
+                    break;
+                }
+                firstVal = firstIterator.next();
+            }
+            else if (diff > 0) {
+                result.add(secondVal);
+                if (!secondIterator.hasNext()) {
+                    result.add(firstVal);
+                    break;
+                }
+                secondVal = secondIterator.next();
+            }
+            else {
+                if (!firstIterator.hasNext() || !secondIterator.hasNext()) {
+                    break;
+                }
+                firstVal = firstIterator.next();
+                secondVal = secondIterator.next();
+            }
+        }
+        // add tails
+        while (firstIterator.hasNext()) {
+            result.add(firstIterator.next());
+        }
+        while (secondIterator.hasNext()) {
+            result.add(secondIterator.next());
+        }
+        return Collections.unmodifiableList(result);
+    }
+
+}