Use the Errorprone Compiler

build/android/gyp/javac.py

 #!/usr/bin/env python
 #
 # Copyright 2013 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 fnmatch
 import optparse
 import os
 import shutil
@@ skipping 37 matching lines @@
     elif error_re.match(line):
       line = Colorize(line, error_re, error_color)
     elif marker_re.match(line):
       line = Colorize(line, marker_re, marker_color)
     return line
 
   return '\n'.join(map(ApplyColor, output.split('\n')))
 
 
 def DoJavac(
-    classpath, classes_dir, chromium_code, java_files):
+    classpath, classes_dir, chromium_code,
+    use_errorprone, errorprone_path, java_files):
   """Runs javac.
 
   Builds |java_files| with the provided |classpath| and puts the generated
   .class files into |classes_dir|. If |chromium_code| is true, extra lint
   checking will be enabled.
   """
 
   jar_inputs = []
   for path in classpath:
     if os.path.exists(path + '.TOC'):
@@ skipping 12 matching lines @@
       '-d', classes_dir]
   if chromium_code:
     # TODO(aurimas): re-enable '-Xlint:deprecation' checks once they are fixed.
     javac_args.extend(['-Xlint:unchecked'])
   else:
     # XDignore.symbol.file makes javac compile against rt.jar instead of
     # ct.sym. This means that using a java internal package/class will not
     # trigger a compile warning or error.
     javac_args.extend(['-XDignore.symbol.file'])
 
-  javac_cmd = ['javac'] + javac_args + java_files
+  if use_errorprone:
+    disabled_checks = [

[jbudorick, 2015/05/07 18:49:26]

The contents of this if block should probably be in its own function.

[raywilliams_chromium, 2015/05/11 19:52:24]

helper method extracted

[raywilliams_chromium, 2015/05/11 19:52:25]

Done.

+        # Something in chrome_private_java makes this check crash.
+        'ClassCanBeStatic',
+        # These crash on lots of targets.
+        'WrongParameterPackage',
+        'GuiceOverridesGuiceInjectableMethod',
+        'GuiceOverridesJavaxInjectableMethod',
+        'ElementsCountedInLoop',
+      ]
+    errorprone_options = [
+        '-Xepdisable:' + ','.join(
+          ['com.google.errorprone.bugpatterns.' + s for s in disabled_checks])
+      ]
+    assert errorprone_path != None

[jbudorick, 2015/05/07 18:49:26]

Do we use assertions in the gyp scripts? I know we've generally been shying away
from them in the test runner scripts.

[raywilliams_chromium, 2015/05/11 19:52:25]

removed

[raywilliams_chromium, 2015/05/11 19:52:25]

Done.

+    javac_cmd = [errorprone_path] + errorprone_options
+  else:
+    javac_cmd = ['javac']
+
+  javac_cmd = javac_cmd + javac_args + java_files
 
   def Compile():
     build_utils.CheckOutput(
         javac_cmd,
         print_stdout=chromium_code,
         stderr_filter=ColorJavacOutput)
 
   record_path = os.path.join(classes_dir, 'javac.md5.stamp')
   md5_check.CallAndRecordIfStale(
       Compile,
@@ skipping 75 matching lines @@
       default='',
       help='List of .class file patterns to exclude from the jar.')
 
   parser.add_option(
       '--chromium-code',
       type='int',
       help='Whether code being compiled should be built with stricter '
       'warnings for chromium code.')
 
   parser.add_option(
+      '--disable-errorprone',

[jbudorick, 2015/05/07 18:49:26]

Having this as a disable flag implies that errorprone is on by default. I'm not
sure that's the stance we want to take, especially at introduction.

Also, doing this as an enable flag makes your use of it below less awkward.

[raywilliams_chromium, 2015/05/11 19:52:25]

changed the disable flag to an enable flag

[raywilliams_chromium, 2015/05/11 19:52:25]

Done.

+      action='store_true',
+      help='Use this to disable use of the errorprone compiler. This is '
+      'typically only used if errorprone crashes on some target or for targets '
+      'used in building the errorpone compiler.')
+  parser.add_option(
+      '--errorprone-path',
+      help='Path to the errorprone compiler executable.')
+
+  parser.add_option(
       '--classes-dir',
       help='Directory for compiled .class files.')
   parser.add_option('--jar-path', help='Jar output path.')
   parser.add_option(
       '--main-class',
       help='The class containing the main method.')
   parser.add_option(
       '--manifest-entry',
       action='append',
       help='Key:value pairs to add to the .jar manifest.')
@@ skipping 36 matching lines @@
         for include in javac_includes:
           if fnmatch.fnmatch(f, include):
             filtered_java_files.append(f)
             break
       java_files = filtered_java_files
 
     DoJavac(
         classpath,
         classes_dir,
         options.chromium_code,
+        options.chromium_code and not options.disable_errorprone,

[jbudorick, 2015/05/07 18:49:26]

(this is the awkward double-negative part referred to above)

[raywilliams_chromium, 2015/05/11 19:52:25]

Done.

+        options.errorprone_path,
         java_files)
 
     if options.jar_path:
       if options.main_class or options.manifest_entry:
         if options.manifest_entry:
           entries = map(lambda e: e.split(":"), options.manifest_entry)
         else:
           entries = []
         manifest_file = os.path.join(temp_dir, 'manifest')
         CreateManifest(manifest_file, classpath, options.main_class, entries)
@@ skipping 17 matching lines @@
     build_utils.WriteDepfile(
         options.depfile,
         input_files + build_utils.GetPythonDependencies())
 
   if options.stamp:
     build_utils.Touch(options.stamp)
 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))
-
-