Issue checkstyle#344: add multi-thread support to CheckstyleReportsParser

Author: Pavel Bludov

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");

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 <T>
+     *        type of the enum.
      * @return compare mode.
      */
-    private static CompareMode getCompareMode(String optionName, CommandLine commandLine,
-            CompareMode defaultMode) {
-        final CompareMode result;
+    private static <T extends Enum<T>> T getEnumOption(String optionName, CommandLine commandLine,
+            T defaultValue) {
+        final T result;
         if (commandLine.hasOption(optionName)) {
-            result = CompareMode.valueOf(commandLine.getOptionValue(optionName).toUpperCase());
+            final Class<T> enumType = (Class<T>) defaultValue.getClass();
+            result = Enum.valueOf(enumType, commandLine.getOptionValue(optionName).toUpperCase());
         }
         else {
-            result = defaultMode;
+            result = defaultValue;
         }
         return result;
     }

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 = 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 = compareSeverity(this.severity, other.severity);
+        }
+        if (diff == 0) {
+            diff = this.message.compareTo(other.message);
+        }
+        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/CliPaths.java renamed to 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.
      *