Use the Errorprone Compiler

build/android/gyp/javac.py

Index: build/android/gyp/javac.py
diff --git a/build/android/gyp/javac.py b/build/android/gyp/javac.py
index a0fcda815635e0d19948ab394f459ff49c9a08bf..236f95fb6fd1554d6d7697d09d16a459d6095751 100755
--- a/build/android/gyp/javac.py
+++ b/build/android/gyp/javac.py
@@ -55,7 +55,8 @@ def ColorJavacOutput(output):
 
 
 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
@@ -88,7 +89,26 @@ def DoJavac(
     # 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(
@@ -184,6 +204,16 @@ def main(argv):
       '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.')
@@ -240,6 +270,8 @@ def main(argv):
         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:
@@ -277,5 +309,3 @@ def main(argv):
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))
-
-