[Android] Incorporate findbugs into android builds.

build/android/pylib/utils/findbugs.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import optparse
+import argparse
+import logging
 import os
 import re
 import shlex
-import subprocess
 import sys
+import xml.dom.minidom
 
 from pylib import cmd_helper
 from pylib import constants
 
 
-def _PrintMessage(warnings, title, action, known_bugs_file):
-  if warnings:
-    print
-    print '*' * 80
-    print '%s warnings.' % title
-    print '%s %s' % (action, known_bugs_file)
-    print '-' * 80
-    for warning in warnings:
-      print warning
-    print '-' * 80
-    print
+_FINDBUGS_HOME = os.path.join(constants.DIR_SOURCE_ROOT, 'third_party',
+                              'findbugs')
+_FINDBUGS_JAR = os.path.join(_FINDBUGS_HOME, 'lib', 'findbugs.jar')
+_FINDBUGS_MAX_HEAP = 768
+_FINDBUGS_PLUGIN_PATH = os.path.join(
+    constants.DIR_SOURCE_ROOT, 'tools', 'android', 'findbugs_plugin', 'lib',
+    'chromiumPlugin.jar')
 
 
-def _StripLineNumbers(current_warnings):
-  re_line = r':\[line.*?\]$'
-  return [re.sub(re_line, '', x) for x in current_warnings]
+def _ParseXmlResults(results_doc):
+  warnings = set()
+  for en in (n for n in results_doc.documentElement.childNodes
+             if n.nodeType == xml.dom.Node.ELEMENT_NODE):
+    if en.tagName == 'BugInstance':
+      warnings.add(_ParseBugInstance(en))
+  return warnings
 
 
-def _DiffKnownWarnings(current_warnings_set, known_bugs_file):
-  if os.path.exists(known_bugs_file):
-    with open(known_bugs_file, 'r') as known_bugs:
-      known_bugs_set = set(known_bugs.read().splitlines())
-  else:
-    known_bugs_set = set()
-
-  new_warnings = current_warnings_set - known_bugs_set
-  _PrintMessage(sorted(new_warnings), 'New', 'Please fix, or perhaps add to',
-                known_bugs_file)
-
-  obsolete_warnings = known_bugs_set - current_warnings_set
-  _PrintMessage(sorted(obsolete_warnings), 'Obsolete', 'Please remove from',
-                known_bugs_file)
-
-  count = len(new_warnings) + len(obsolete_warnings)
-  if count:
-    print '*** %d FindBugs warning%s! ***' % (count, 's' * (count > 1))
-    if len(new_warnings):
-      print '*** %d: new ***' % len(new_warnings)
-    if len(obsolete_warnings):
-      print '*** %d: obsolete ***' % len(obsolete_warnings)
-    print
-    print 'Alternatively,  rebaseline with --rebaseline command option'
-    print
-  else:
-    print 'No new FindBugs warnings.'
-  print
-  return count
+def _ParseBugInstance(node):
+  bug = FindBugsWarning(node.getAttribute('type'))
+  for c in (n for n in node.childNodes
+            if n.nodeType == xml.dom.Node.ELEMENT_NODE):
+    if c.tagName == 'Class':
+      bug.class_name = c.getAttribute('classname')
+    elif c.tagName == 'Method':
+      bug.method_name = c.getAttribute('name')
+    elif c.tagName == 'Field':
+      bug.field_name = c.getAttribute('name')
+    elif c.tagName == 'SourceLine':
+      bug.file_name = c.getAttribute('sourcefile')
+      bug.start_line = int(c.getAttribute('start'))
+      bug.end_line = int(c.getAttribute('end'))
+    elif (c.tagName == 'ShortMessage' and len(c.childNodes) == 1
+          and c.childNodes[0].nodeType == xml.dom.Node.TEXT_NODE):
+      bug.message = c.childNodes[0].data
+  return bug
 
 
-def _Rebaseline(current_warnings_set, known_bugs_file):
-  with file(known_bugs_file, 'w') as known_bugs:
-    for warning in sorted(current_warnings_set):
-      print >> known_bugs, warning
-  return 0
+class FindBugsWarning(object):
+
+  def __init__(self, bug_type='', class_name='', end_line=0, field_name='',
+               file_name='', message='', method_name='', start_line=0):
+    self.bug_type = bug_type
+    self.class_name = class_name
+    self.end_line = end_line
+    self.field_name = field_name
+    self.file_name = file_name
+    self.message = message
+    self.method_name = method_name
+    self.start_line = start_line
+
+  def __cmp__(self, other):

[cjhopman, 2015/03/12 19:22:43]

These functions could probably all just work on self.__dict__ and other.__dict__

def __cmp__(self, other):
  return cmp(self.__dict__, other.__dict__)

etc.

It'd be less likely to miss one of the fields in one of them that way.

[jbudorick, 2015/03/13 13:12:19]

Fine with that for __eq__ (especially since I missed the late additions,
field_name and file_name), but I've ordered the comparisons intentionally for
__cmp__ s.t. sorting warnings results in a reasonable (or at least, what I think
is reasonable) order:
 1 the file
 2 the start line
 3 the end line
 4 the bug type
 (the rest are there for completeness; sorting order between bugs for which 1-4
are equal doesn't seem too significant to me)

[cjhopman, 2015/03/17 01:31:16]

you could just explicitly compare 1-4 and then compare the __dict__s (don't
really think it matters much either way).

+    return (cmp(self.file_name, other.file_name)
+            or cmp(self.start_line, other.start_line)
+            or cmp(self.end_line, other.end_line)
+            or cmp(self.bug_type, other.bug_type)
+            or cmp(self.class_name, other.class_name)
+            or cmp(self.field_name, other.field_name)
+            or cmp(self.method_name, other.method_name)
+            or cmp(self.message, other.message))
+
+  def __eq__(self, other):
+    return (self.bug_type == other.bug_type
+            and self.class_name == other.class_name
+            and self.end_line == other.end_line
+            and self.message == other.message
+            and self.method_name == other.method_name
+            and self.start_line == other.start_line)
+
+  def __hash__(self):
+    return hash((self.bug_type, self.class_name, self.end_line, self.message,
+                 self.method_name, self.start_line))
+
+  def __ne__(self, other):
+    return not self == other
+
+  def __str__(self):
+    return '%s at %s: %s (%s)' % (
+        '%s.%s' % (self.class_name, self.method_name or self.field_name),
+        '%s:%s' % (
+            self.file_name,
+            str(self.start_line)
+                if self.start_line == self.end_line
+                else '%s-%s' % (self.start_line, self.end_line)),
+        self.message, self.bug_type)
 
 
-def _GetChromeJars(release_version):
-  version = 'Debug'
-  if release_version:
-    version = 'Release'
-  path = os.path.join(constants.DIR_SOURCE_ROOT,
-                      os.environ.get('CHROMIUM_OUT_DIR', 'out'),
-                      version,
-                      'lib.java')
-  cmd = 'find %s -name "*.jar"' % path
-  out = cmd_helper.GetCmdOutput(shlex.split(cmd))
-  out = [p for p in out.splitlines() if not p.endswith('.dex.jar')]
-  if not out:
-    print 'No classes found in %s' % path
-  return ' '.join(out)
-
-
-def _Run(exclude, known_bugs, classes_to_analyze, auxiliary_classes,
-        rebaseline, release_version, findbug_args):
-  """Run the FindBugs.
+def Run(exclude, classes_to_analyze, auxiliary_classes, output_file,
+        findbug_args, jars):
+  """Run FindBugs.
 
   Args:
     exclude: the exclude xml file, refer to FindBugs's -exclude command option.
-    known_bugs: the text file of known bugs. The bugs in it will not be
-                reported.
     classes_to_analyze: the list of classes need to analyze, refer to FindBug's
                         -onlyAnalyze command line option.
     auxiliary_classes: the classes help to analyze, refer to FindBug's
                        -auxclasspath command line option.
-    rebaseline: True if the known_bugs file needs rebaseline.
-    release_version: True if the release version needs check, otherwise check
-                     debug version.
-    findbug_args: addtional command line options needs pass to Findbugs.
+    output_file: An optional path to dump XML results to.
+    findbug_args: A list of addtional command line options to pass to Findbugs.
   """
+  system_classes = [
+    os.path.join(constants.ANDROID_SDK_ROOT, 'platforms',
+                 'android-%s' % constants.ANDROID_SDK_VERSION, 'android.jar')

[cjhopman, 2015/03/12 19:22:43]

This path should probably be gotten from the build system. A TODO to that effect
would be fine.

[jbudorick, 2015/03/13 13:12:19]

TODO added.

+  ]
+  system_classes.extend(os.path.abspath(classes)
+                        for classes in auxiliary_classes or [])
 
-  chrome_src = constants.DIR_SOURCE_ROOT
-  sdk_root = constants.ANDROID_SDK_ROOT
-  sdk_version = constants.ANDROID_SDK_VERSION
+  cmd = ['java',
+         '-classpath', '%s:' % _FINDBUGS_JAR,
+         '-Xmx%dm' % _FINDBUGS_MAX_HEAP,
+         '-Dfindbugs.home="%s"' % _FINDBUGS_HOME,
+         '-jar', _FINDBUGS_JAR,
+         '-textui', '-sortByClass',
+         '-pluginList', _FINDBUGS_PLUGIN_PATH, '-xml:withMessages',]
+  if system_classes:
+    cmd.extend(['-auxclasspath', ':'.join(system_classes)])
+  if classes_to_analyze:
+    cmd.extend(['-onlyAnalyze', classes_to_analyze])
+  if exclude:
+    cmd.extend(['-exclude', os.path.abspath(exclude)])
+  if output_file:
+    cmd.extend(['-output', output_file])
+  if findbug_args:
+    cmd.extend(findbug_args)
+  cmd.extend(os.path.abspath(j) for j in jars or [])
 
-  system_classes = []
-  system_classes.append(os.path.join(sdk_root, 'platforms',
-                                     'android-%s' % sdk_version, 'android.jar'))
-  if auxiliary_classes:
-    for classes in auxiliary_classes:
-      system_classes.append(os.path.abspath(classes))
+  if output_file:
+    cmd_helper.RunCmd(cmd)
+    results_doc = xml.dom.minidom.parse(output_file)
+  else:
+    raw_out = cmd_helper.GetCmdOutput(cmd)
+    results_doc = xml.dom.minidom.parseString(raw_out)
+  current_warnings_set = _ParseXmlResults(results_doc)
 
-  findbugs_javacmd = 'java'
-  findbugs_home = os.path.join(chrome_src, 'third_party', 'findbugs')
-  findbugs_jar = os.path.join(findbugs_home, 'lib', 'findbugs.jar')
-  findbugs_pathsep = ':'
-  findbugs_maxheap = '768'
+  return (' '.join(cmd), current_warnings_set)
 
-  cmd = '%s ' % findbugs_javacmd
-  cmd = '%s -classpath %s%s' % (cmd, findbugs_jar, findbugs_pathsep)
-  cmd = '%s -Xmx%sm ' % (cmd, findbugs_maxheap)
-  cmd = '%s -Dfindbugs.home="%s" ' % (cmd, findbugs_home)
-  cmd = '%s -jar %s ' % (cmd, findbugs_jar)
-
-  cmd = '%s -textui -sortByClass ' % cmd
-  cmd = '%s -pluginList %s' % (cmd, os.path.join(chrome_src, 'tools', 'android',
-                                                 'findbugs_plugin', 'lib',
-                                                 'chromiumPlugin.jar'))
-  if len(system_classes):
-    cmd = '%s -auxclasspath %s ' % (cmd, ':'.join(system_classes))
-
-  if classes_to_analyze:
-    cmd = '%s -onlyAnalyze %s ' % (cmd, classes_to_analyze)
-
-  if exclude:
-    cmd = '%s -exclude %s ' % (cmd, os.path.abspath(exclude))
-
-  if findbug_args:
-    cmd = '%s %s ' % (cmd, findbug_args)
-
-  chrome_classes = _GetChromeJars(release_version)
-  if not chrome_classes:
-    return 1
-  cmd = '%s %s ' % (cmd, chrome_classes)
-
-  print
-  print '*' * 80
-  print 'Command used to run findbugs:'
-  print cmd
-  print '*' * 80
-  print
-
-  proc = subprocess.Popen(shlex.split(cmd),
-                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-  out, _err = proc.communicate()
-  current_warnings_set = set(_StripLineNumbers(filter(None, out.splitlines())))
-
-  if rebaseline:
-    return _Rebaseline(current_warnings_set, known_bugs)
-  else:
-    return _DiffKnownWarnings(current_warnings_set, known_bugs)
-
-def Run(options):
-  exclude_file = None
-  known_bugs_file = None
-
-  if options.exclude:
-    exclude_file = options.exclude
-  elif options.base_dir:
-    exclude_file = os.path.join(options.base_dir, 'findbugs_exclude.xml')
-
-  if options.known_bugs:
-    known_bugs_file = options.known_bugs
-  elif options.base_dir:
-    known_bugs_file = os.path.join(options.base_dir, 'findbugs_known_bugs.txt')
-
-  auxclasspath = None
-  if options.auxclasspath:
-    auxclasspath = options.auxclasspath.split(':')
-  return _Run(exclude_file, known_bugs_file, options.only_analyze, auxclasspath,
-              options.rebaseline, options.release_build, options.findbug_args)
-
-
-def GetCommonParser():
-  parser = optparse.OptionParser()
-  parser.add_option('-r',
-                    '--rebaseline',
-                    action='store_true',
-                    dest='rebaseline',
-                    help='Rebaseline known findbugs issues.')
-
-  parser.add_option('-a',
-                    '--auxclasspath',
-                    action='store',
-                    default=None,
-                    dest='auxclasspath',
-                    help='Set aux classpath for analysis.')
-
-  parser.add_option('-o',
-                    '--only-analyze',
-                    action='store',
-                    default=None,
-                    dest='only_analyze',
-                    help='Only analyze the given classes and packages.')
-
-  parser.add_option('-e',
-                    '--exclude',
-                    action='store',
-                    default=None,
-                    dest='exclude',
-                    help='Exclude bugs matching given filter.')
-
-  parser.add_option('-k',
-                    '--known-bugs',
-                    action='store',
-                    default=None,
-                    dest='known_bugs',
-                    help='Not report the bugs in the given file.')
-
-  parser.add_option('-l',
-                    '--release-build',
-                    action='store_true',
-                    dest='release_build',
-                    help='Analyze release build instead of debug.')
-
-  parser.add_option('-f',
-                    '--findbug-args',
-                    action='store',
-                    default=None,
-                    dest='findbug_args',
-                    help='Additional findbug arguments.')
-
-  parser.add_option('-b',
-                    '--base-dir',
-                    action='store',
-                    default=None,
-                    dest='base_dir',
-                    help='Base directory for configuration file.')
-
-  return parser
-
-
-def main():
-  parser = GetCommonParser()
-  options, _ = parser.parse_args()
-
-  return Run(options)
-
-
-if __name__ == '__main__':
-  sys.exit(main())