[maven-checkstyle-plugin] 01/01: [MCHECKSTYLE-389] Partial revert of MCHECKSTYLE-365 severity change back to 'null' default

[eo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch MCHECKSTYLE-389
in repository https://gitbox.apache.org/repos/asf/maven-checkstyle-plugin.git

commit 4ed1a5dfa16c2ba0b599528b033b841f539c41ac
Author: Jeremy Landis <je...@hotmail.com>
AuthorDate: Mon Jan 20 19:35:03 2020 -0500

    [MCHECKSTYLE-389] Partial revert of MCHECKSTYLE-365 severity change back to 'null' default
    
    The severity setting even on original logic is incorrect.  It must remain null if it is to work properly at all.  While counts in fact are off, the entire section will be lost if it doesn't contain issues in at least 'error' due to the change when using default configurations.  This has to be reverted as not originally fixed properly so that the rules aggregate will display.  Example case has no 'error' type, only 'warning'.  Changing this to 'warning' will pick that section up.  Usin [...]
---
 src/it/MCHECKSTYLE-365/verify.groovy                              | 8 ++++++--
 .../maven/plugins/checkstyle/CheckstyleReportGenerator.java       | 7 ++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/it/MCHECKSTYLE-365/verify.groovy b/src/it/MCHECKSTYLE-365/verify.groovy
index 82810c2..4e0b594 100644
--- a/src/it/MCHECKSTYLE-365/verify.groovy
+++ b/src/it/MCHECKSTYLE-365/verify.groovy
@@ -32,14 +32,18 @@ assert htmlReportFile.exists();
  *
  * A more robust solution would be to parse the HTML, but that's
  * a lot more effort than required to actually verify the behaviour.
+ *
+ * TODO: The original fix of MCHECKSTYLE-365 does not take into account when 'error'
+ * type doesn't exist and therefore breaks 'rules' aggregate reporting. As a result, counts
+ * set back to 3 and code is reverted. A better fix needs to be implemented.
  */
 
 // Case with no custom messages
 def numberOfOccurancesOfFileTabCharacter = htmlReportFile.text.count(">FileTabCharacter<")
-assert 2 == numberOfOccurancesOfFileTabCharacter;
+assert 3 == numberOfOccurancesOfFileTabCharacter;
 
 // Case with custom messages
 def numberOfOccurancesOfRegexpSingleline = htmlReportFile.text.count(">RegexpSingleline<");
-assert 2 == numberOfOccurancesOfRegexpSingleline;
+assert 3 == numberOfOccurancesOfRegexpSingleline;
 
 return true;
\ No newline at end of file
diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleReportGenerator.java b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleReportGenerator.java
index a443998..aae277d 100644
--- a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleReportGenerator.java
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleReportGenerator.java
@@ -807,9 +807,10 @@ public class CheckstyleReportGenerator
             else
             {
                 String fixedmessage = getConfigAttribute( childConfig, null, "message", null );
-                // Grab the severity from the rule configuration, use "error" as default value (as is 
-                // done where the rule severity is displayed otherwise we miscount error violations)
-                String configSeverity = getConfigAttribute( childConfig, null, "severity", "error" );
+                // Grab the severity from the rule configuration. Do not set default value here as
+                // it breaks our rule aggregate section entirely.  The counts are off but this is
+                // not appropriate fix location per MCHECKSTYLE-365.
+                String configSeverity = getConfigAttribute( childConfig, null, "severity", null );
 
                 // count rule violations
                 long violations = 0;