From f48cea117fb1f88140952e2708756f8a0b48ed38 Mon Sep 17 00:00:00 2001 From: Pavel Bludov Date: Sat, 31 Oct 2020 20:14:41 +0300 Subject: [PATCH] Issue #344: add multi-thread support to CheckstyleReportsParser --- .../github/checkstyle/CliArgsValidator.java | 4 +- .../main/java/com/github/checkstyle/Main.java | 69 ++++--- .../checkstyle/data/CheckstyleRecord.java | 75 +++++++- .../data/{CliPaths.java => CliOptions.java} | 35 ++-- .../github/checkstyle/data/DiffReport.java | 169 ++++++++---------- .../com/github/checkstyle/data/DiffUtils.java | 112 ++++++++++++ .../github/checkstyle/data/ThreadingMode.java | 34 ++++ .../parser/CheckstyleReportsParser.java | 8 +- .../parser/CheckstyleTextParser.java | 8 +- .../github/checkstyle/site/SiteGenerator.java | 14 +- .../com/github/checkstyle/DiffUtilsTest.java | 134 ++++++++++++++ .../java/com/github/checkstyle/MainTest.java | 4 +- 12 files changed, 516 insertions(+), 150 deletions(-) rename patch-diff-report-tool/src/main/java/com/github/checkstyle/data/{CliPaths.java => CliOptions.java} (80%) create mode 100644 patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffUtils.java create mode 100644 patch-diff-report-tool/src/main/java/com/github/checkstyle/data/ThreadingMode.java create mode 100644 patch-diff-report-tool/src/test/java/com/github/checkstyle/DiffUtilsTest.java diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/CliArgsValidator.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/CliArgsValidator.java index 232871d5..d147aef1 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/CliArgsValidator.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/CliArgsValidator.java @@ -21,7 +21,7 @@ import java.nio.file.Files; -import com.github.checkstyle.data.CliPaths; +import com.github.checkstyle.data.CliOptions; import com.github.checkstyle.data.CompareMode; /** @@ -52,7 +52,7 @@ private CliArgsValidator() { * @throws IllegalArgumentException * on failure of any check. */ - public static void checkPaths(CliPaths paths) throws IllegalArgumentException { + public static void checkPaths(CliOptions paths) throws IllegalArgumentException { if (paths.getPatchReportPath() == null) { throw new IllegalArgumentException("obligatory argument --patchReportPath " + "not present, -h for help"); diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/Main.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/Main.java index 04f10a48..8beff069 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/Main.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/Main.java @@ -32,10 +32,11 @@ import org.apache.commons.cli.Options; import org.apache.commons.cli.ParseException; -import com.github.checkstyle.data.CliPaths; +import com.github.checkstyle.data.CliOptions; import com.github.checkstyle.data.CompareMode; import com.github.checkstyle.data.DiffReport; import com.github.checkstyle.data.MergedConfigurationModule; +import com.github.checkstyle.data.ThreadingMode; import com.github.checkstyle.parser.CheckstyleConfigurationsParser; import com.github.checkstyle.parser.CheckstyleReportsParser; import com.github.checkstyle.parser.CheckstyleTextParser; @@ -65,6 +66,7 @@ public final class Main { + "\t--output - path to store the resulting diff report (optional, if absent then " + "default path will be used: ~/XMLDiffGen_report_yyyy.mm.dd_hh_mm_ss), remember, " + "if this folder exists its content will be purged;\n" + + "\t--threadingMode - which type of threading mode to use: SINGLE or MULTI;\n" + "\t-h - simply shows help message."; /** @@ -107,6 +109,11 @@ public final class Main { */ private static final String OPTION_REFFILES_PATH = "refFiles"; + /** + * Name for command line option "threadingMode". + */ + private static final String OPTION_THREADING_MODE = "threadingMode"; + /** * Name for command line option "outputPath". */ @@ -155,39 +162,40 @@ public static void main(final String... args) throws Exception { System.out.println(MSG_HELP); } else { - final CliPaths paths = getCliPaths(commandLine); + final CliOptions options = getCliOptions(commandLine); final DiffReport diffReport; - if (paths.getCompareMode() == CompareMode.XML) { + if (options.getCompareMode() == CompareMode.XML) { // XML parsing stage System.out.println("XML parsing is started."); - diffReport = CheckstyleReportsParser.parse(paths.getBaseReportPath(), - paths.getPatchReportPath(), XML_PARSE_PORTION_SIZE); + diffReport = CheckstyleReportsParser.parse(options.getBaseReportPath(), + options.getPatchReportPath(), options.getThreadingMode(), + XML_PARSE_PORTION_SIZE); } else { // file parsing stage System.out.println("File parsing is started."); - diffReport = CheckstyleTextParser.parse(paths.getBaseReportPath(), - paths.getPatchReportPath()); + diffReport = CheckstyleTextParser.parse(options.getBaseReportPath(), + options.getPatchReportPath(), options.getThreadingMode()); } // Configuration processing stage. MergedConfigurationModule diffConfiguration = null; - if (paths.configurationPresent()) { + if (options.configurationPresent()) { System.out.println("Creation of configuration report is started."); - diffConfiguration = CheckstyleConfigurationsParser.parse(paths.getBaseConfigPath(), - paths.getPatchConfigPath()); + diffConfiguration = CheckstyleConfigurationsParser.parse( + options.getBaseConfigPath(), options.getPatchConfigPath()); } else { - System.out.println( - "Configuration processing skipped: " + "no configuration paths provided."); + System.out.println("Configuration processing skipped: " + + "no configuration options provided."); } // Site and XREF generation stage System.out.println("Creation of diff html site is started."); try { - exportResources(paths); - SiteGenerator.generate(diffReport, diffConfiguration, paths); + exportResources(options); + SiteGenerator.generate(diffReport, diffConfiguration, options); } finally { for (String message : JxrDummyLog.getLogs()) { @@ -223,8 +231,8 @@ private static CommandLine parseCli(String... args) throws ParseException { * CLI arguments. * @return CliPaths instance. */ - private static CliPaths getCliPaths(CommandLine commandLine) { - final CliPaths paths = parseCliToPojo(commandLine); + private static CliOptions getCliOptions(CommandLine commandLine) { + final CliOptions paths = parseCliToPojo(commandLine); CliArgsValidator.checkPaths(paths); return paths; } @@ -237,7 +245,7 @@ private static CliPaths getCliPaths(CommandLine commandLine) { * @throws IOException * thrown on failure to perform checks. */ - private static void exportResources(CliPaths paths) throws IOException { + private static void exportResources(CliOptions paths) throws IOException { final Path outputPath = paths.getOutputPath(); Files.createDirectories(outputPath); FilesystemUtils.createOverwriteDirectory(outputPath.resolve(CSS_FILEPATH)); @@ -265,6 +273,8 @@ private static Options buildOptions() { "Path to the patch checkstyle-report.xml"); options.addOption(null, OPTION_REFFILES_PATH, true, "Path to the directory containing source under checkstyle check, optional."); + options.addOption(null, OPTION_THREADING_MODE, true, + "Option to control which type of threading mode to use."); options.addOption(null, OPTION_OUTPUT_PATH, true, "Path to directory where diff results will be stored."); options.addOption(null, OPTION_BASE_CONFIG_PATH, true, @@ -286,9 +296,9 @@ private static Options buildOptions() { * @throws IllegalArgumentException * on failure to find necessary arguments. */ - private static CliPaths parseCliToPojo(CommandLine commandLine) + private static CliOptions parseCliToPojo(CommandLine commandLine) throws IllegalArgumentException { - final CompareMode compareMode = getCompareMode(OPTION_COMPARE_MODE, commandLine, + final CompareMode compareMode = getEnumOption(OPTION_COMPARE_MODE, commandLine, CompareMode.XML); final Path xmlBasePath = getPath(OPTION_BASE_REPORT_PATH, commandLine, null); final Path xmlPatchPath = getPath(OPTION_PATCH_REPORT_PATH, commandLine, null); @@ -300,8 +310,10 @@ private static CliPaths parseCliToPojo(CommandLine commandLine) final Path configBasePath = getPath(OPTION_BASE_CONFIG_PATH, commandLine, null); final Path configPatchPath = getPath(OPTION_PATCH_CONFIG_PATH, commandLine, null); final boolean shortFilePaths = commandLine.hasOption(OPTION_SHORT_PATHS); - return new CliPaths(compareMode, xmlBasePath, xmlPatchPath, refFilesPath, outputPath, - configBasePath, configPatchPath, shortFilePaths); + final ThreadingMode threadingMode = getEnumOption(OPTION_THREADING_MODE, commandLine, + ThreadingMode.MULTI); + return new CliOptions(compareMode, xmlBasePath, xmlPatchPath, refFilesPath, outputPath, + configBasePath, configPatchPath, shortFilePaths, threadingMode); } /** @@ -311,18 +323,21 @@ private static CliPaths parseCliToPojo(CommandLine commandLine) * name of the option. * @param commandLine * parsed CLI. - * @param defaultMode + * @param defaultValue * mode which is used if CLI option is absent. + * @param + * type of the enum. * @return compare mode. */ - private static CompareMode getCompareMode(String optionName, CommandLine commandLine, - CompareMode defaultMode) { - final CompareMode result; + private static > T getEnumOption(String optionName, CommandLine commandLine, + T defaultValue) { + final T result; if (commandLine.hasOption(optionName)) { - result = CompareMode.valueOf(commandLine.getOptionValue(optionName).toUpperCase()); + final Class enumType = (Class) defaultValue.getClass(); + result = Enum.valueOf(enumType, commandLine.getOptionValue(optionName).toUpperCase()); } else { - result = defaultMode; + result = defaultValue; } return result; } 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 b9fb5065..a89953cf 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 @@ -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,7 +32,7 @@ * @author attatrol * */ -public final class CheckstyleRecord { +public final class CheckstyleRecord implements Comparable { /** * It is usual for sources of records to have name that @@ -38,6 +40,13 @@ public final class CheckstyleRecord { */ 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 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; } } diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliPaths.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliOptions.java similarity index 80% rename from patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliPaths.java rename to patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliOptions.java index df0ec389..3d53f74d 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliPaths.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/CliOptions.java @@ -22,11 +22,11 @@ import java.nio.file.Path; /** - * POJO class that hold input paths. + * POJO class that hold input options. * * @author attatrol */ -public final class CliPaths { +public final class CliOptions { /** * Option to control which type of diff comparison to do. */ @@ -67,6 +67,12 @@ public final class CliPaths { */ private final boolean shortFilePaths; + /** + * Switch specifying if report generation should be done in a single-threaded + * or multi-threaded mode. + */ + private final ThreadingMode threadingMode; + /** * POJO ctor. * @@ -75,22 +81,24 @@ public final class CliPaths { * @param baseReportPath * path to the base checkstyle-report.xml. * @param patchReportPath - * path to the patch checkstyle-report.xml. + * path to the patch checkstyle-report.xml. * @param refFilesPath - * path to the data, tested by checkstyle. +* path to the data, tested by checkstyle. * @param outputPath - * path to the result site. - * @param patchConfigPath - * path to the configuration of the base report. +* path to the result site. * @param baseConfigPath - * path to the configuration of the patch report. +* path to the configuration of the patch report. + * @param patchConfigPath +* path to the configuration of the base report. * @param shortFilePaths - * {@code true} if only short file names should be used with no paths. +* {@code true} if only short file names should be used with no paths. + * @param threadingMode +* type of threading mode to use. */ // -@cs[ParameterNumber] Helper class to pass all CLI attributes around. - public CliPaths(CompareMode compareMode, Path baseReportPath, Path patchReportPath, + public CliOptions(CompareMode compareMode, Path baseReportPath, Path patchReportPath, Path refFilesPath, Path outputPath, Path baseConfigPath, Path patchConfigPath, - boolean shortFilePaths) { + boolean shortFilePaths, ThreadingMode threadingMode) { this.compareMode = compareMode; this.baseReportPath = baseReportPath; this.patchReportPath = patchReportPath; @@ -99,6 +107,7 @@ public CliPaths(CompareMode compareMode, Path baseReportPath, Path patchReportPa this.baseConfigPath = baseConfigPath; this.patchConfigPath = patchConfigPath; this.shortFilePaths = shortFilePaths; + this.threadingMode = threadingMode; } public CompareMode getCompareMode() { @@ -133,6 +142,10 @@ public boolean isShortFilePaths() { return shortFilePaths; } + public ThreadingMode getThreadingMode() { + return threadingMode; + } + /** * Checks if the necessary configuration paths are present to display them on the reports. * 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 57260be0..5bfaa0a3 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,10 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; 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 +42,34 @@ 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<>(); + + /** + * Switch specifying if report generation should be done in a single-threaded + * or multi-threaded mode. + */ + private final ThreadingMode threadingMode; + + /** + * Creates instance of {@code DiffReport} class. + * + * @param threadingMode + * type of threading mode to use. + */ + public DiffReport(ThreadingMode threadingMode) { + this.threadingMode = threadingMode; + } /** * Getter for data container. @@ -56,7 +77,7 @@ public final class DiffReport { * @return map containing parsed data. */ public Map> getRecords() { - return records; + return Collections.unmodifiableMap(records); } public Statistics getStatistics() { @@ -67,120 +88,88 @@ public Statistics getStatistics() { * Adds new records to the diff report, * when there are records with this filename, comparison * between them and new record is performed and only difference is saved. + * If the report is generated in single-threaded mode, the difference will + * be calculated in-place. Otherwise, an asynchronous task will be scheduled. * * @param newRecords * a new records list. * @param filename * name of a file which is a cause of records generation. */ - public void addRecords(List newRecords, - String filename) { + public void addRecords(List newRecords, String filename) { if (!newRecords.isEmpty()) { - final List popped = - records.put(filename, newRecords); - if (popped != null) { - final List diff = - produceDiff(popped, newRecords); - if (diff.isEmpty()) { - records.remove(filename); - } - else { - Collections.sort(diff, new PositionOrderComparator()); - records.put(filename, diff); - } + if (threadingMode == ThreadingMode.SINGLE) { + addNewRecords(newRecords, filename); + } + else { + asyncTasks.add(CompletableFuture.runAsync(() -> { + addNewRecords(newRecords, filename); + })); } } } /** - * Creates difference between 2 lists of records. - * - * @param list1 - * the first list. - * @param list2 - * the second list. - * @return the difference list. + * Generates statistical information and puts in in the accumulator. + * This method will wait for completion of all asynchronous tasks. */ - private static List produceDiff( - List list1, List list2) { - final List 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); - } + public void getDiffStatistics() { + if (threadingMode == ThreadingMode.MULTI) { + joinAsyncTasksAll(); } - return diff; + statistics.setFileNumDiff(records.size()); + records.entrySet().stream() + .flatMap(entry -> entry.getValue().stream()) + .forEach(this::addRecordStatistics); } /** - * Compares the record against list of records. + * Adds new records to the diff report, + * when there are records with this filename, comparison + * between them and new record is performed and only difference is saved. * - * @param list - * of records. - * @param testedRecord - * the record. - * @return true, if has its copy in a list. + * @param newRecords + * a new records list. + * @param filename + * name of a file which is a cause of records generation. */ - private static boolean isInList(List list, - CheckstyleRecord testedRecord) { - boolean belongsToList = false; - for (CheckstyleRecord checkstyleRecord : list) { - if (testedRecord.specificEquals(checkstyleRecord)) { - belongsToList = true; - break; + private void addNewRecords(List newRecords, String filename) { + Collections.sort(newRecords); + final List popped = + records.put(filename, newRecords); + if (popped != null) { + final List diff = + DiffUtils.produceDiff(popped, newRecords); + if (diff.isEmpty()) { + records.remove(filename); + } + else { + records.put(filename, diff); } } - return belongsToList; } /** - * Generates statistical information and puts in in the accumulator. + * Await completion of all asynchronous tasks. */ - public void getDiffStatistics() { - statistics.setFileNumDiff(records.size()); - for (Map.Entry> entry - : records.entrySet()) { - final List 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()); - } - } + private void joinAsyncTasksAll() { + asyncTasks.forEach(CompletableFuture::join); } /** - * 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 { - - @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()); } - } diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffUtils.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/DiffUtils.java new file mode 100644 index 00000000..f10b504d --- /dev/null +++ b/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-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; + +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 the type of elements. + * @return the difference list. + */ + public static > List produceDiff( + List firstList, List secondList) { + final List 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 the type of elements. + * @return the difference list (always sorted). + */ + private static > List produceDiff( + Iterator firstIterator, Iterator secondIterator) { + T firstVal = firstIterator.next(); + T 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 Collections.unmodifiableList(result); + } + +} diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/ThreadingMode.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/ThreadingMode.java new file mode 100644 index 00000000..622b7254 --- /dev/null +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/data/ThreadingMode.java @@ -0,0 +1,34 @@ +//////////////////////////////////////////////////////////////////////////////// +// 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; + +/** + * Different threading modes. + */ +public enum ThreadingMode { + /** + * Process files in a single thread. + */ + SINGLE, + /** + * Process files in multiple threads. + */ + MULTI +} 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 c4a1cee3..9ef818cd 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 @@ -35,6 +35,7 @@ import com.github.checkstyle.data.CheckstyleRecord; import com.github.checkstyle.data.DiffReport; import com.github.checkstyle.data.Statistics; +import com.github.checkstyle.data.ThreadingMode; /** * Contains logics of the StaX parser for the checkstyle xml reports. @@ -116,6 +117,8 @@ private CheckstyleReportsParser() { * path to base XML file. * @param patchXml * path to patch XML file. + * @param threadingMode + * type of threading mode to use. * @param portionSize * single portion of XML file processed at once by any parser. * @return parsed content. @@ -124,9 +127,10 @@ private CheckstyleReportsParser() { * @throws XMLStreamException * on internal parser error. */ - public static DiffReport parse(Path baseXml, Path patchXml, int portionSize) + public static DiffReport parse(Path baseXml, Path patchXml, + ThreadingMode threadingMode, int portionSize) throws FileNotFoundException, XMLStreamException { - final DiffReport content = new DiffReport(); + final DiffReport content = new DiffReport(threadingMode); final XMLEventReader baseReader = StaxUtils.createReader(baseXml); final XMLEventReader patchReader = StaxUtils.createReader(patchXml); while (baseReader.hasNext() || patchReader.hasNext()) { diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleTextParser.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleTextParser.java index cb597195..91b14df1 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleTextParser.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/parser/CheckstyleTextParser.java @@ -30,6 +30,7 @@ import com.github.checkstyle.data.CheckstyleRecord; import com.github.checkstyle.data.DiffReport; import com.github.checkstyle.data.Statistics; +import com.github.checkstyle.data.ThreadingMode; import com.github.checkstyle.parser.JgitUtils.JgitDifference; /** @@ -72,12 +73,15 @@ private CheckstyleTextParser() { * path to base directory. * @param patchReport * path to patch directory. + * @param threadingMode + * type of threading mode to use. * @return parsed content. * @throws IOException * if there is a problem accessing a file. */ - public static DiffReport parse(Path baseReport, Path patchReport) throws IOException { - final DiffReport content = new DiffReport(); + public static DiffReport parse(Path baseReport, Path patchReport, ThreadingMode threadingMode) + throws IOException { + final DiffReport content = new DiffReport(threadingMode); final StringListIterator baseReader = getFiles(baseReport); final StringListIterator patchReader = getFiles(patchReport); while (true) { diff --git a/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/SiteGenerator.java b/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/SiteGenerator.java index 829b523f..6fbca04e 100644 --- a/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/SiteGenerator.java +++ b/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/SiteGenerator.java @@ -33,7 +33,7 @@ import com.github.checkstyle.Main; import com.github.checkstyle.data.CheckstyleRecord; -import com.github.checkstyle.data.CliPaths; +import com.github.checkstyle.data.CliOptions; import com.github.checkstyle.data.DiffReport; import com.github.checkstyle.data.MergedConfigurationModule; import com.github.checkstyle.data.Statistics; @@ -81,7 +81,7 @@ private SiteGenerator() { * on failure to write site to disc. */ public static void generate(DiffReport diffReport, MergedConfigurationModule diffConfiguration, - CliPaths paths) throws IOException { + CliOptions paths) throws IOException { // setup thymeleaf engine final TemplateEngine tplEngine = getTemplateEngine(); // setup xreference generator @@ -147,16 +147,16 @@ private static void generateHeader(TemplateEngine tplEngine, FileWriter writer, * file writer. * @param diffReport * difference between two checkstyle reports. - * @param paths - * CLI paths. + * @param options + * CLI options. * @param xrefGenerator * xReference generator. */ private static void generateBody(TemplateEngine tplEngine, FileWriter writer, - DiffReport diffReport, CliPaths paths, XrefGenerator xrefGenerator) { + DiffReport diffReport, CliOptions options, XrefGenerator xrefGenerator) { final AnchorCounter anchorCounter = new AnchorCounter(); - final Path refFilesPath = paths.getRefFilesPath(); + final Path refFilesPath = options.getRefFilesPath(); for (Map.Entry> entry : diffReport.getRecords().entrySet()) { final List records = entry.getValue(); String filename = entry.getKey(); @@ -165,7 +165,7 @@ private static void generateBody(TemplateEngine tplEngine, FileWriter writer, for (CheckstyleRecord checkstyleRecord : records) { final String xreference = xrefGenerator.generateXref(checkstyleRecord.getXref(), - paths.isShortFilePaths()); + options.isShortFilePaths()); checkstyleRecord.setXref(xreference); } diff --git a/patch-diff-report-tool/src/test/java/com/github/checkstyle/DiffUtilsTest.java b/patch-diff-report-tool/src/test/java/com/github/checkstyle/DiffUtilsTest.java new file mode 100644 index 00000000..83070bf6 --- /dev/null +++ b/patch-diff-report-tool/src/test/java/com/github/checkstyle/DiffUtilsTest.java @@ -0,0 +1,134 @@ +//////////////////////////////////////////////////////////////////////////////// +// 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; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import com.github.checkstyle.data.DiffUtils; +import com.github.checkstyle.internal.AbstractTest; + +public class DiffUtilsTest extends AbstractTest { + + @Test + public void testConstructor() throws Exception { + assertUtilsClassHasPrivateConstructor(DiffUtils.class); + } + + @Test + public void testEmptyLists() { + final List list1 = Collections.emptyList(); + final List list2 = Collections.emptyList(); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected empty list", Collections.emptyList(), actual); + } + + @Test + public void testMatch() { + final List list1 = Arrays.asList(1, 2, 3); + final List list2 = Arrays.asList(1, 2, 3); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected empty list", Collections.emptyList(), actual); + } + + @Test + public void testOddEven() { + final List list1 = Arrays.asList(1, 3, 5); + final List list2 = Arrays.asList(2, 4, 6); + final List expected = Arrays.asList(1, 2, 3, 4, 5, 6); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [1, 2, 3, 4, 5, 6]", expected, actual); + } + + @Test + public void testEmptyLeft() { + final List list1 = Collections.emptyList(); + final List list2 = Arrays.asList(1, 2, 3); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected list2", list2, actual); + } + + @Test + public void testEmptyRight() { + final List list1 = Arrays.asList(1, 2, 3); + final List list2 = Collections.emptyList(); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected list1", list1, actual); + } + + @Test + public void testLeftTail() { + final List list1 = Arrays.asList(1, 2, 3, 4); + final List list2 = Arrays.asList(1, 2); + final List expected = Arrays.asList(3, 4); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [3, 4]", expected, actual); + } + + @Test + public void testRightTail() { + final List list1 = Arrays.asList(1, 2); + final List list2 = Arrays.asList(1, 2, 4, 5); + final List expected = Arrays.asList(4, 5); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [4, 5]", expected, actual); + } + + @Test + public void testLeftLower() { + final List list1 = Arrays.asList(1, 2); + final List list2 = Arrays.asList(4, 5); + final List expected = Arrays.asList(1, 2, 4, 5); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [1, 2, 4, 5]", expected, actual); + } + + @Test + public void testRightLower() { + final List list1 = Arrays.asList(4, 5); + final List list2 = Arrays.asList(1, 2); + final List expected = Arrays.asList(1, 2, 4, 5); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [1, 2, 4, 5]", expected, actual); + } + + @Test + public void testLeftHeadTail() { + final List list1 = Arrays.asList(1, 2, 3, 5, 6, 7); + final List list2 = Arrays.asList(3, 4, 5); + final List expected = Arrays.asList(1, 2, 4, 6, 7); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [1, 2, 4, 6, 7]", expected, actual); + } + + @Test + public void testRightHeadTail() { + final List list1 = Arrays.asList(3, 4, 5); + final List list2 = Arrays.asList(1, 2, 3, 5, 6, 7); + final List expected = Arrays.asList(1, 2, 4, 6, 7); + final List actual = DiffUtils.produceDiff(list1, list2); + Assert.assertEquals("Expected [1, 2, 4, 6, 7]", expected, actual); + } + +} diff --git a/patch-diff-report-tool/src/test/java/com/github/checkstyle/MainTest.java b/patch-diff-report-tool/src/test/java/com/github/checkstyle/MainTest.java index c49e1e5a..53c9129b 100644 --- a/patch-diff-report-tool/src/test/java/com/github/checkstyle/MainTest.java +++ b/patch-diff-report-tool/src/test/java/com/github/checkstyle/MainTest.java @@ -48,7 +48,9 @@ public void testHelp() throws Exception { + "\t--output - path to store the resulting diff report (optional," + " if absent then default path will be used:" + " ~/XMLDiffGen_report_yyyy.mm.dd_hh_mm_ss), remember, if this folder" - + " exists its content will be purged;\n" + "\t-h - simply shows help message.\n" + + " exists its content will be purged;\n" + + "\t--threadingMode - which type of threading mode to use: SINGLE or MULTI;\n" + + "\t-h - simply shows help message.\n" + "patch-diff-report-tool execution finished.\n", getSystemOut()); }