Add the Errorprone Java Compiler

These changes let the errorprone compiler find problems when building Android. A global flag disabled Errorprone by default. When enabled, code problems will be shown with suggestions on how to fix them. BUG=485599 Review URL: https://codereview.chromium.org/1136573002 Cr-Commit-Position: refs/heads/master@{#335509}
Denger-Network · Jun 22, 2015 · 6ffb117 · 6ffb117
1 parent 50f6a89
commit 6ffb117
Show file tree

Hide file tree

Showing 17 changed files with 587 additions and 10 deletions.
diff --git a/DEPS b/DEPS
@@ -415,6 +415,9 @@ deps_os = {
     'src/third_party/cardboard-java/src':
      Var('chromium_git') + '/external/github.com/googlesamples/cardboard-java.git' + '@' + '08ad25a04f2801bd822c3f2cd28301b68d74aef6',
 
+    'src/third_party/errorprone/lib':
+      Var('chromium_git') + '/chromium/third_party/errorprone.git' + '@' + '6c66e56c0f9d750aef83190466df834f9d6af8ab',
+
     'src/third_party/findbugs':
      Var('chromium_git') + '/chromium/deps/findbugs.git' + '@' + '7f69fa78a6db6dc31866d09572a0e356e921bf12',
 

diff --git a/build/android/BUILD.gn b/build/android/BUILD.gn
@@ -0,0 +1,26 @@
+# Copyright 2014 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("//build/config/android/rules.gni")
+
+sun_tools_jar_path = "$root_gen_dir/sun_tools_jar/tools.jar"
+
+action("find_sun_tools_jar") {
+  script = "//build/android/gyp/find_sun_tools_jar.py"
+  depfile = "$target_gen_dir/$target_name.d"
+  outputs = [
+    depfile,
+    sun_tools_jar_path,
+  ]
+  args = [
+    "--depfile",
+    rebase_path(depfile, root_build_dir),
+    "--output",
+    rebase_path(sun_tools_jar_path, root_build_dir),
+  ]
+}
+
+java_prebuilt("sun_tools_java") {
+  jar_path = sun_tools_jar_path
+}
diff --git a/build/android/gyp/find_sun_tools_jar.py b/build/android/gyp/find_sun_tools_jar.py
@@ -0,0 +1,56 @@
+#!/usr/bin/env python
+#
+# Copyright 2014 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.
+
+"""This finds the java distribution's tools.jar and copies it somewhere.
+"""
+
+import argparse
+import os
+import re
+import shutil
+import sys
+
+from util import build_utils
+
+RT_JAR_FINDER = re.compile(r'\[Opened (.*)/jre/lib/rt.jar\]')
+
+def main():
+  parser = argparse.ArgumentParser(description='Find Sun Tools Jar')
+  parser.add_argument('--depfile',
+                      help='Path to depfile. This must be specified as the '
+                           'action\'s first output.')
+  parser.add_argument('--output', required=True)
+  args = parser.parse_args()
+
+  sun_tools_jar_path = FindSunToolsJarPath()
+
+  if sun_tools_jar_path is None:
+    raise Exception("Couldn\'t find tools.jar")
+
+  # Using copyfile instead of copy() because copy() calls copymode()
+  # We don't want the locked mode because we may copy over this file again
+  shutil.copyfile(sun_tools_jar_path, args.output)
+
+  if args.depfile:
+    build_utils.WriteDepfile(
+        args.depfile,
+        [sun_tools_jar_path] + build_utils.GetPythonDependencies())
+
+
+def FindSunToolsJarPath():
+  # This works with at least openjdk 1.6, 1.7 and sun java 1.6, 1.7
+  stdout = build_utils.CheckOutput(
+      ["java", "-verbose", "-version"], print_stderr=False)
+  for ln in stdout.splitlines():
+    match = RT_JAR_FINDER.match(ln)
+    if match:
+      return os.path.join(match.group(1), 'lib', 'tools.jar')
+
+  return None
+
+
+if __name__ == '__main__':
+  sys.exit(main())
diff --git a/build/android/gyp/javac.py b/build/android/gyp/javac.py
@@ -54,8 +54,20 @@ def ApplyColor(line):
   return '\n'.join(map(ApplyColor, output.split('\n')))
 
 
+ERRORPRONE_OPTIONS = [
+  '-Xepdisable:'
+  # Something in chrome_private_java makes this check crash.
+  'com.google.errorprone.bugpatterns.ClassCanBeStatic,'
+  # These crash on lots of targets.
+  'com.google.errorprone.bugpatterns.WrongParameterPackage,'
+  'com.google.errorprone.bugpatterns.GuiceOverridesGuiceInjectableMethod,'
+  'com.google.errorprone.bugpatterns.GuiceOverridesJavaxInjectableMethod,'
+  'com.google.errorprone.bugpatterns.ElementsCountedInLoop'
+]
+
 def DoJavac(
-    classpath, classes_dir, chromium_code, java_files):
+    classpath, classes_dir, chromium_code,
+    use_errorprone_path, java_files):
   """Runs javac.
 
   Builds |java_files| with the provided |classpath| and puts the generated
@@ -88,7 +100,12 @@ def DoJavac(
     # trigger a compile warning or error.
     javac_args.extend(['-XDignore.symbol.file'])
 
-  javac_cmd = ['javac'] + javac_args + java_files
+  if use_errorprone_path:
+    javac_cmd = [use_errorprone_path] + ERRORPRONE_OPTIONS
+  else:
+    javac_cmd = ['javac']
+
+  javac_cmd = javac_cmd + javac_args + java_files
 
   def Compile():
     build_utils.CheckOutput(
@@ -183,6 +200,10 @@ def main(argv):
       help='Whether code being compiled should be built with stricter '
       'warnings for chromium code.')
 
+  parser.add_option(
+      '--use-errorprone-path',
+      help='Use the Errorprone compiler at this path.')
+
   parser.add_option(
       '--classes-dir',
       help='Directory for compiled .class files.')
@@ -241,6 +262,7 @@ def main(argv):
           classpath,
           classes_dir,
           options.chromium_code,
+          options.use_errorprone_path,
           java_files)
 
     if options.jar_path:

diff --git a/build/android/setup.gyp b/build/android/setup.gyp
@@ -77,6 +77,35 @@
         },
       ],
     }, # build_output_dirs
+    {
+      'target_name': 'sun_tools_java',
+      'type': 'none',
+      'variables': {
+        'found_jar_path': '<(PRODUCT_DIR)/sun_tools_java/tools.jar',
+        'jar_path': '<(found_jar_path)',
+      },
+      'includes': [
+        '../../build/host_prebuilt_jar.gypi',
+      ],
+      'actions': [
+        {
+          'action_name': 'find_sun_tools_jar',
+          'variables' : {
+          },
+          'inputs' : [
+            'gyp/find_sun_tools_jar.py',
+            'gyp/util/build_utils.py',
+          ],
+          'outputs': [
+            '<(found_jar_path)',
+          ],
+          'action': [
+            'python', 'gyp/find_sun_tools_jar.py',
+            '--output', '<(found_jar_path)',
+          ],
+        },
+      ],
+    }, # sun_tools_java
   ]
 }
 
diff --git a/build/config/android/config.gni b/build/config/android/config.gni
@@ -42,6 +42,9 @@ if (is_android) {
 
     # Set to true to run findbugs on JAR targets.
     run_findbugs = false
+
+    # Set to true to enable the Errorprone compiler
+    use_errorprone_java_compiler = false
   }
 
   # Host stuff -----------------------------------------------------------------

diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni
@@ -909,6 +909,12 @@ template("compile_java") {
   if (defined(invoker.chromium_code)) {
     _chromium_code = invoker.chromium_code
   }
+
+  _enable_errorprone = use_errorprone_java_compiler
+  if (defined(invoker.enable_errorprone)) {
+    _enable_errorprone = invoker.enable_errorprone
+  }
+
   _manifest_entries = []
   if (defined(invoker.manifest_entries)) {
     _manifest_entries = invoker.manifest_entries
@@ -978,7 +984,13 @@ template("compile_java") {
     if (_chromium_code) {
       args += [ "--chromium-code=1" ]
     }
-
+    if (_enable_errorprone) {
+      deps += [ "//third_party/errorprone:chromium_errorprone" ]
+      args += [
+        "--use-errorprone-path",
+        "bin/chromium_errorprone",
+      ]
+    }
     args += rebase_path(_java_files, root_build_dir)
   }
 
@@ -1128,6 +1140,9 @@ template("java_library_impl") {
     chromium_code = _chromium_code
     android = _requires_android
 
+    if (defined(invoker.enable_errorprone)) {
+      _enable_errorprone = invoker.enable_errorprone
+    }
     if (defined(invoker.jar_excluded_patterns)) {
       jar_excluded_patterns = invoker.jar_excluded_patterns
     }

diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni
@@ -798,6 +798,7 @@ template("java_strings_grd_prebuilt") {
 #     android_library target, for example.
 #
 #   chromium_code: If true, extra analysis warning/errors will be enabled.
+#   enable_errorprone: If true, enables the errorprone compiler.
 #
 #   data_deps, testonly
 #
@@ -825,6 +826,9 @@ template("java_binary") {
     if (defined(invoker.deps)) {
       deps = invoker.deps
     }
+    if (defined(invoker.enable_errorprone)) {
+      enable_errorprone = invoker.enable_errorprone
+    }
     if (defined(invoker.java_files)) {
       java_files = invoker.java_files
     }
@@ -919,6 +923,8 @@ template("junit_binary") {
 #     ease the gyp->gn conversion and will be removed in the future.
 #
 #   chromium_code: If true, extra analysis warning/errors will be enabled.
+#   enable_errorprone: If true, enables the errorprone compiler.
+#
 #   jar_excluded_patterns: List of patterns of .class files to exclude from the
 #     final jar.
 #
@@ -967,6 +973,9 @@ template("java_library") {
     if (defined(invoker.deps)) {
       deps = invoker.deps
     }
+    if (defined(invoker.enable_errorprone)) {
+      enable_errorprone = invoker.enable_errorprone
+    }
     if (defined(invoker.jar_excluded_patterns)) {
       jar_excluded_patterns = invoker.jar_excluded_patterns
     }
@@ -1058,6 +1067,8 @@ template("java_prebuilt") {
 #     ease the gyp->gn conversion and will be removed in the future.
 #
 #   chromium_code: If true, extra analysis warning/errors will be enabled.
+#   enable_errorprone: If true, enables the errorprone compiler.
+#
 #   jar_excluded_patterns: List of patterns of .class files to exclude from the
 #     final jar.
 #
@@ -1103,6 +1114,9 @@ template("android_library") {
     if (defined(invoker.deps)) {
       deps = invoker.deps
     }
+    if (defined(invoker.enable_errorprone)) {
+      enable_errorprone = invoker.enable_errorprone
+    }
     if (defined(invoker.jar_excluded_patterns)) {
       jar_excluded_patterns = invoker.jar_excluded_patterns
     }

diff --git a/build/host_jar.gypi b/build/host_jar.gypi
@@ -53,6 +53,8 @@
     'jar_path': '<(jar_dir)/<(jar_name)',
     'main_class%': '',
     'stamp': '<(intermediate_dir)/jar.stamp',
+    'enable_errorprone%': '0',
+    'errorprone_exe_path': '<(PRODUCT_DIR)/bin.java/chromium_errorprone',
   },
   'all_dependent_settings': {
     'variables': {
@@ -64,25 +66,33 @@
       'action_name': 'javac_<(_target_name)',
       'message': 'Compiling <(_target_name) java sources',
       'variables': {
-        'extra_options': [],
+        'extra_args': [],
+        'extra_inputs': [],
         'java_sources': [ '<!@(find <@(src_paths) -name "*.java")' ],
         'conditions': [
           ['"<(excluded_src_paths)" != ""', {
             'java_sources!': ['<!@(find <@(excluded_src_paths) -name "*.java")']
           }],
           ['"<(jar_excluded_classes)" != ""', {
-            'extra_options': ['--jar-excluded-classes=<(jar_excluded_classes)']
+            'extra_args': ['--jar-excluded-classes=<(jar_excluded_classes)']
           }],
           ['main_class != ""', {
-            'extra_options': ['--main-class=>(main_class)']
-          }]
+            'extra_args': ['--main-class=>(main_class)']
+          }],
+          ['enable_errorprone == 1', {
+            'extra_inputs': [
+              '<(errorprone_exe_path)',
+            ],
+            'extra_args': [ '--use-errorprone-path=<(errorprone_exe_path)' ],
+          }],
         ],
       },
       'inputs': [
         '<(DEPTH)/build/android/gyp/util/build_utils.py',
         '<(DEPTH)/build/android/gyp/javac.py',
         '^@(java_sources)',
         '>@(input_jars_paths)',
+        '<@(extra_inputs)',
       ],
       'outputs': [
         '<(jar_path)',
@@ -95,7 +105,7 @@
         '--chromium-code=<(chromium_code)',
         '--stamp=<(stamp)',
         '--jar-path=<(jar_path)',
-        '<@(extra_options)',
+        '<@(extra_args)',
         '^@(java_sources)',
       ],
     },
@@ -125,7 +135,12 @@
           ]
         }
       ]
-    }]
+    }],
+    ['enable_errorprone == 1', {
+      'dependencies': [
+        '<(DEPTH)/third_party/errorprone/errorprone.gyp:chromium_errorprone',
+      ],
+    }],
   ]
 }