diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java index b9fb50651..6cc05e886 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CheckstyleRecord.java @@ -30,7 +30,7 @@ * @author attatrol * */ -public final class CheckstyleRecord { +public final class CheckstyleRecord implements Comparable { /** * It is usual for sources of records to have name that @@ -177,17 +177,31 @@ 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 = this.source.compareTo(other.source); + if (diff == 0) { + diff = Integer.compare(this.line, other.line); + } + if (diff == 0) { + diff = Integer.compare(this.column, other.column); + } + if (diff == 0) { + diff = SeverityLevel.getInstance(this.severity) + .compareTo(SeverityLevel.getInstance(other.severity)); + } + if (diff == 0) { + diff = this.message.compareTo(other.message); + } + return diff; } } diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java index 57260be0d..86c10c340 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffReport.java @@ -21,10 +21,11 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.TreeMap; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentSkipListMap; import com.github.checkstyle.parser.CheckstyleReportsParser; @@ -42,13 +43,18 @@ public final class DiffReport { * Container for parsed data, * note it is a TreeMap for memory keeping purposes. */ - private Map> records = - new TreeMap<>(); + private final Map> records = + new ConcurrentSkipListMap<>(); /** * Container for statistical data. */ - private Statistics statistics = new Statistics(); + private final Statistics statistics = new Statistics(); + + /** + * Asynchronous tasks for difference calculation. + */ + private final List> asyncTasks = new ArrayList<>(); /** * Getter for data container. @@ -56,7 +62,7 @@ public final class DiffReport { * @return map containing parsed data. */ public Map> getRecords() { - return records; + return Collections.unmodifiableMap(records); } public Statistics getStatistics() { @@ -76,6 +82,7 @@ public Statistics getStatistics() { public void addRecords(List newRecords, String filename) { if (!newRecords.isEmpty()) { + Collections.sort(newRecords); final List popped = records.put(filename, newRecords); if (popped != null) { @@ -85,7 +92,6 @@ public void addRecords(List newRecords, records.remove(filename); } else { - Collections.sort(diff, new PositionOrderComparator()); records.put(filename, diff); } } @@ -93,49 +99,92 @@ public void addRecords(List newRecords, } /** - * Creates difference between 2 lists of records. + * Adds new records to the diff report asynchronously, + * when there are records with this filename, comparison + * between them and new record is performed and only difference is saved. * - * @param list1 + * @param newRecords + * a new records list. + * @param filename + * name of a file which is a cause of records generation. + */ + public void addRecordsAsync( + List newRecords, String filename) { + asyncTasks.add(CompletableFuture.runAsync(() -> addRecords(newRecords, filename))); + } + + /** + * Creates difference between 2 sorted lists of records. + * + * @param firstList * the first list. - * @param list2 + * @param secondList * the second list. * @return the difference list. */ private static List produceDiff( - List list1, List list2) { - final List diff = new ArrayList<>(); - for (CheckstyleRecord rec1 : list1) { - if (!isInList(list2, rec1)) { - diff.add(rec1); - } + List firstList, List secondList) { + final List result; + if (firstList.isEmpty()) { + result = secondList; } - for (CheckstyleRecord rec2 : list2) { - if (!isInList(list1, rec2)) { - diff.add(rec2); - } + else if (secondList.isEmpty()) { + result = firstList; + } + else { + result = produceDiff(firstList.iterator(), secondList.iterator()); } - return diff; + return result; } /** - * Compares the record against list of records. + * Creates difference between 2 non-empty iterators of records. * - * @param list - * of records. - * @param testedRecord - * the record. - * @return true, if has its copy in a list. + * @param firstIterator + * the first iterator. + * @param secondIterator + * the second iterator. + * @return the difference list (always sorted). */ - private static boolean isInList(List list, - CheckstyleRecord testedRecord) { - boolean belongsToList = false; - for (CheckstyleRecord checkstyleRecord : list) { - if (testedRecord.specificEquals(checkstyleRecord)) { - belongsToList = true; - break; + private static List produceDiff( + Iterator firstIterator, Iterator secondIterator) { + CheckstyleRecord firstVal = firstIterator.next(); + CheckstyleRecord secondVal = secondIterator.next(); + final List 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 belongsToList; + return result; } /** @@ -161,26 +210,9 @@ public void getDiffStatistics() { } /** - * Comparator used to sort lists of CheckstyleRecord objects - * by their position in code. - * - * @author atta_troll - * + * Await completion of all asynchronous tasks. */ - private static class PositionOrderComparator - implements Comparator { - - @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; - } - } + public void joinAsyncTasksAll() { + asyncTasks.forEach(CompletableFuture::join); } - } diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/SeverityLevel.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/SeverityLevel.java new file mode 100644 index 000000000..1a0fc2287 --- /dev/null +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/SeverityLevel.java @@ -0,0 +1,65 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2020 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; + +/** + * Severity level for a {@link CheckstyleRecord}. + * Used to compare two records in predefined order: "info", "warning", "error", all other. + */ +public enum SeverityLevel { + + /** Severity level info. */ + INFO, + + /** Severity level warning. */ + WARNING, + + /** Severity level error. */ + ERROR, + + /** All other severity levels. */ + OTHER; + + /** + * SeverityLevel factory method. + * + * @param securityLevelName level name, such as "info", "warning", "error", etc. + * @return the {@code SeverityLevel} associated with {@code securityLevelName} + */ + public static SeverityLevel getInstance(String securityLevelName) { + final SeverityLevel result; + switch (securityLevelName) { + case "info": + result = INFO; + break; + case "warning": + result = WARNING; + break; + case "error": + result = ERROR; + break; + default: + result = OTHER; + break; + } + return result; + } + +} diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java index c4a1cee38..2ef9b47f5 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleReportsParser.java @@ -133,12 +133,14 @@ public static DiffReport parse(Path baseXml, Path patchXml, int portionSize) parseXmlPortion(content, baseReader, portionSize, BASE_REPORT_INDEX); parseXmlPortion(content, patchReader, portionSize, PATCH_REPORT_INDEX); } + content.joinAsyncTasksAll(); content.getDiffStatistics(); return content; } /** * Parses portion of the XML report. + * Difference generation is performed asynchronously for efficient CPU usage. * * @param diffReport * container for parsed data. @@ -187,7 +189,7 @@ else if (startElementName.equals(ERROR_TAG)) { if (event.isEndElement()) { final EndElement endElement = event.asEndElement(); if (endElement.getName().getLocalPart().equals(FILE_TAG)) { - diffReport.addRecords(records, filename); + diffReport.addRecordsAsync(records, filename); if (counter == 0) { break; }