HBASE-14859 Better checkstyle reporting. Reports file names, error names and old/new error counts. Fix some checks. (Apekshit)

Signed-off-by: stack <stack@apache.org>

dev-support/checkstyle_report.py

@@ -36,28 +36,45 @@ def path_key(x):
   path = x.attrib['name']
   return path[path.find('hbase-'):]
 
-def print_row(path, master_errors, patch_errors):
-    print '%s\t%s\t%s' % (path,master_errors,patch_errors)
+def error_name(x):
+  error_class = x.attrib['source']
+  return error_class[error_class.rfind(".") + 1:]
+
+def print_row(path, error, master_errors, patch_errors):
+    print '%s\t%s\t%s\t%s' % (path,error, master_errors,patch_errors)
 
 master = etree.parse(sys.argv[1])
 patch = etree.parse(sys.argv[2])
 
 master_dict = defaultdict(int)
+ret_value = 0
 
 for child in master.getroot().getchildren():
     if child.tag != 'file':
         continue
-    child_errors = len(child.getchildren())
-    if child_errors == 0:
-        continue
-    master_dict[path_key(child)] = child_errors
+    file = path_key(child)
+    for error_tag in child.getchildren():
+        error = error_name(error_tag)
+        if (file, error) in master_dict:
+            master_dict[(file, error)] += 1
+        else:
+            master_dict[(file, error)] = 1
 
 for child in patch.getroot().getchildren():
     if child.tag != 'file':
         continue
-    child_errors = len(child.getchildren())
-    if child_errors == 0:
-        continue
-    k = path_key(child)
-    if child_errors > master_dict[k]:
-        print_row(k, master_dict[k], child_errors)
+    temp_dict = defaultdict(int)
+    for error_tag in child.getchildren():
+        error = error_name(error_tag)
+        if error in temp_dict:
+            temp_dict[error] += 1
+        else:
+            temp_dict[error] = 1
+
+    file = path_key(child)
+    for error, count in temp_dict.iteritems():
+        if count > master_dict[(file, error)]:
+            print_row(file, error, master_dict[(file, error)], count)
+            ret_value = 1
+
+sys.exit(ret_value)

dev-support/test-patch.sh

@@ -659,19 +659,17 @@ checkCheckstyleErrors() {
     mv target/checkstyle-result.xml $PATCH_DIR/patchCheckstyle.xml
     mv target/site/checkstyle-aggregate.html $PATCH_DIR
     mv target/site/checkstyle.css $PATCH_DIR
-    trunkCheckstyleErrors=`$GREP '<error' $PATCH_DIR/trunkCheckstyle.xml | $AWK 'BEGIN {total = 0} {total += 1} END {print total}'`
-    patchCheckstyleErrors=`$GREP '<error' $PATCH_DIR/patchCheckstyle.xml | $AWK 'BEGIN {total = 0} {total += 1} END {print total}'`
-    if [[ $patchCheckstyleErrors -gt $trunkCheckstyleErrors ]] ; then
+    $BASEDIR/dev-support/checkstyle_report.py $PATCH_DIR/trunkCheckstyle.xml $PATCH_DIR/patchCheckstyle.xml
+    if [[ $? -eq 1 ]] ; then
                 JIRA_COMMENT_FOOTER="Checkstyle Errors: $BUILD_URL/artifact/patchprocess/checkstyle-aggregate.html
 
                 $JIRA_COMMENT_FOOTER"
 
                 JIRA_COMMENT="$JIRA_COMMENT
 
-                {color:red}-1 checkstyle{color}.  The applied patch generated $patchCheckstyleErrors checkstyle errors (more than the master's current $trunkCheckstyleErrors errors)."
+                {color:red}-1 checkstyle{color}.  The applied patch generated new checkstyle errors. Check build console for list of new errors."
         return 1
     fi
-    echo "There were $patchCheckstyleErrors checkstyle errors in this patch compared to $trunkCheckstyleErrors on master."
   fi
   JIRA_COMMENT_FOOTER="Checkstyle Errors: $BUILD_URL/artifact/patchprocess/checkstyle-aggregate.html
 
@@ -679,7 +677,7 @@ checkCheckstyleErrors() {
 
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 checkstyle{color}.  The applied patch does not increase the total number of checkstyle errors"
+    {color:green}+1 checkstyle{color}. The applied patch does not generate new checkstyle errors."
   return 0
 
 }

hbase-checkstyle/src/main/resources/hbase/checkstyle.xml

@@ -67,10 +67,7 @@
     http://checkstyle.sourceforge.net/config_imports.html -->
     <module name="AvoidStarImport"/>
     <module name="ImportOrder">
-      <property name="groups" value="*,javax,java"/>
       <property name="ordered" value="true"/>
-      <property name="separated" value="true"/>
-      <property name="option" value="bottom"/>
       <property name="sortStaticImportsAlphabetically" value="true"/>
     </module>
     <module name="RedundantImport"/>
@@ -89,6 +86,9 @@
     <module name="Indentation">
       <property name="basicOffset" value="2"/>
       <property name="caseIndent" value="2"/>
+      <property name="throwsIndent" value="2"/>
+      <property name="arrayInitIndent" value="2"/>
+      <property name="lineWrappingIndentation" value="2"/>
     </module>
 
     <!-- Size Violation Checks