Move the Error Prone plugin into Bazel

--
MOS_MIGRATED_REVID=96365813

13 files changed, 242 insertions, 82 deletions

diff --git a/examples/java-native/src/main/java/com/example/myproject/BUILD b/examples/java-native/src/main/java/com/example/myproject/BUILD
index 960877761f..72cbf76423 100644
--- a/examples/java-native/src/main/java/com/example/myproject/BUILD
+++ b/examples/java-native/src/main/java/com/example/myproject/BUILD
@@ -8,7 +8,10 @@ java_binary(
 
 java_library(
     name = "hello-lib",
-    srcs = glob(["*.java"]),
+    srcs = glob(
+        ["*.java"],
+        exclude = ["HelloErrorProne.java"],
+    ),
 )
 
 java_binary(
@@ -23,6 +26,11 @@ java_library(
     resources = ["//examples/java-native/src/main/resources:greeting"],
 )
 
+java_library(
+    name = "hello-error-prone",
+    srcs = ["HelloErrorProne.java"],
+)
+
 filegroup(
     name = "srcs",
     srcs = ["BUILD"] + glob(["**/*.java"]),
diff --git a/examples/java-native/src/main/java/com/example/myproject/HelloErrorProne.java b/examples/java-native/src/main/java/com/example/myproject/HelloErrorProne.java
new file mode 100644
index 0000000000..3f97adaac2
--- /dev/null
+++ b/examples/java-native/src/main/java/com/example/myproject/HelloErrorProne.java
@@ -0,0 +1,10 @@
+package com.example.myproject;
+
+/** Sanity check for Error Prone integration. */
+public class HelloErrorProne {
+  public static void main (String[] args) {
+    boolean result;
+    byte b = 0;
+    result = b == 255;
+  }
+}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
index 63d3cac13c..7a52e103ff 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
@@ -18,7 +18,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.buildjar.javac.JavacOptions;
 import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
 import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
-import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorProneOptionsPlugin;
+import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorPronePlugin;
 import com.google.devtools.build.buildjar.javac.plugins.filemanager.FileManagerInitializationPlugin;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
@@ -125,7 +125,7 @@ public abstract class BazelJavaBuilder {
     ImmutableList<BlazeJavaCompilerPlugin> plugins =
         ImmutableList.<BlazeJavaCompilerPlugin>of(
             new FileManagerInitializationPlugin(),
-            new ErrorProneOptionsPlugin());
+            new ErrorPronePlugin());
     JavaLibraryBuildRequest build =
         new JavaLibraryBuildRequest(args, plugins, new DependencyModule.Builder());
     build.setJavacOpts(JavacOptions.normalizeOptions(build.getJavacOpts()));
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorProneOptionsPlugin.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorProneOptionsPlugin.java
deleted file mode 100644
index 2172cd9f49..0000000000
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorProneOptionsPlugin.java
+++ /dev/null
@@ -1,64 +0,0 @@
-// Copyright 2014 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.buildjar.javac.plugins.errorprone;
-
-import com.google.devtools.build.buildjar.InvalidCommandLineException;
-import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
-import com.google.errorprone.ErrorProneOptions;
-import com.google.errorprone.InvalidCommandLineOptionException;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-/**
- * Process (and discard) Error Prone specific options.
- * 
- * <p>This is a stop-gap until full Error Prone support is added to Bazel.
- */
-public class ErrorProneOptionsPlugin extends BlazeJavaCompilerPlugin {
-
-  @Override
-  public List<String> processArgs(List<String> args) throws InvalidCommandLineException {
-    // TODO(cushon): add -XepIgnoreUnknownCheckNames once Error Prone is supported
-    return processEpOptions(processExtraChecksOption(args));
-  }
-
-  private List<String> processEpOptions(List<String> args) throws InvalidCommandLineException {
-    ErrorProneOptions epOptions;
-    try {
-      epOptions = ErrorProneOptions.processArgs(args);
-    } catch (InvalidCommandLineOptionException e) {
-      throw new InvalidCommandLineException(e.getMessage());
-    }
-    return Arrays.asList(epOptions.getRemainingArgs());
-  }
-
-  private List<String> processExtraChecksOption(List<String> args) {
-    List<String> arguments = new ArrayList<>();
-    for (String arg : args) {
-      switch (arg) {
-        case "-extra_checks":
-        case "-extra_checks:on":
-          break;
-        case "-extra_checks:off":
-          break;
-        default:
-          arguments.add(arg);
-      }
-    }
-    return arguments;
-  }
-}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorPronePlugin.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorPronePlugin.java
new file mode 100644
index 0000000000..35ff15354e
--- /dev/null
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/errorprone/ErrorPronePlugin.java
@@ -0,0 +1,175 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.buildjar.javac.plugins.errorprone;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.buildjar.InvalidCommandLineException;
+import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
+import com.google.errorprone.ErrorProneAnalyzer;
+import com.google.errorprone.ErrorProneOptions;
+import com.google.errorprone.InvalidCommandLineOptionException;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.scanner.BuiltInCheckerSuppliers;
+import com.google.errorprone.scanner.ScannerSupplier;
+
+import com.sun.source.util.TaskEvent;
+import com.sun.source.util.TaskEvent.Kind;
+import com.sun.tools.javac.comp.AttrContext;
+import com.sun.tools.javac.comp.Env;
+import com.sun.tools.javac.main.JavaCompiler;
+import com.sun.tools.javac.main.Main.Result;
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.JavacMessages;
+import com.sun.tools.javac.util.Log;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.ServiceLoader;
+
+import javax.tools.JavaFileManager;
+import javax.tools.StandardLocation;
+
+/**
+ * A plugin for BlazeJavaCompiler that performs Error Prone analysis.
+ * Error Prone is a static analysis framework that we use to perform
+ * some simple static checks on Java code.
+ */
+public final class ErrorPronePlugin extends BlazeJavaCompilerPlugin {
+
+  private final Optional<ScannerSupplier> extraChecks;
+
+  public ErrorPronePlugin(ScannerSupplier extraChecks) {
+    this.extraChecks = Optional.of(extraChecks);
+  }
+
+  public ErrorPronePlugin() {
+    this.extraChecks = Optional.absent();
+  }
+
+  private ErrorProneAnalyzer errorProneAnalyzer;
+  private ErrorProneOptions epOptions;
+  // error-prone is enabled by default
+  private boolean enabled = true;
+
+  /** Registers our message bundle. */
+  public static void setupMessageBundle(Context context) {
+    JavacMessages.instance(context).add("com.google.errorprone.errors");
+  }
+
+  @Override
+  public List<String> processArgs(List<String> args) throws InvalidCommandLineException {
+    // allow javacopts that reference unknown error-prone checks
+    args = ImmutableList.<String>builder().addAll(args).add("-XepIgnoreUnknownCheckNames").build();
+    return processEpOptions(processExtraChecksOption(args));
+  }
+
+  private List<String> processEpOptions(List<String> args) throws InvalidCommandLineException {
+    try {
+      epOptions = ErrorProneOptions.processArgs(args);
+    } catch (InvalidCommandLineOptionException e) {
+      throw new InvalidCommandLineException(e.getMessage());
+    }
+    return Arrays.asList(epOptions.getRemainingArgs());
+  }
+
+  private List<String> processExtraChecksOption(List<String> args) {
+    List<String> arguments = new ArrayList<>();
+    for (String arg : args) {
+      switch (arg) {
+        case "-extra_checks":
+        case "-extra_checks:on":
+          enabled = true;
+          break;
+        case "-extra_checks:off":
+          enabled = false;
+          break;
+        default:
+          arguments.add(arg);
+      }
+    }
+    return arguments;
+  }
+
+  private ScannerSupplier defaultScannerSupplier() {
+    // open-source checks that are errors
+    ScannerSupplier result = BuiltInCheckerSuppliers.errorChecks();
+    if (extraChecks.isPresent()) {
+      result = result.plus(extraChecks.get());
+    }
+    return result;
+  }
+
+  private static final Function<BugChecker, Class<? extends BugChecker>> GET_CLASS =
+      new Function<BugChecker, Class<? extends BugChecker>>() {
+        @Override
+        public Class<? extends BugChecker> apply(BugChecker input) {
+          return input.getClass();
+        }
+      };
+
+  @Override
+  public void init(Context context, Log log, JavaCompiler compiler) {
+    super.init(context, log, compiler);
+
+    if (!enabled) { // error-prone plugin is turned-off
+      return;
+    }
+
+    setupMessageBundle(context);
+
+    // TODO(cushon): Move this into error-prone proper
+    JavaFileManager fileManager = context.get(JavaFileManager.class);
+    // Search ANNOTATION_PROCESSOR_PATH if it's available, otherwise fallback to fileManager's
+    // own class loader.  Unlike in annotation processor discovery, we never search CLASS_PATH.
+    ClassLoader loader = fileManager.hasLocation(StandardLocation.ANNOTATION_PROCESSOR_PATH)
+        ? fileManager.getClassLoader(StandardLocation.ANNOTATION_PROCESSOR_PATH)
+        : fileManager.getClass().getClassLoader();
+    Iterable<BugChecker> extraBugCheckers = ServiceLoader.load(BugChecker.class, loader);
+    ScannerSupplier scannerSupplier =
+        defaultScannerSupplier().plus(
+            ScannerSupplier.fromBugCheckerClasses(
+                Iterables.transform(extraBugCheckers, GET_CLASS)));
+
+    if (epOptions != null) {
+      try {
+        scannerSupplier = scannerSupplier.applyOverrides(epOptions);
+      } catch (InvalidCommandLineOptionException e) {
+        throwError(Result.CMDERR, e.getMessage());
+      }
+    }
+
+    errorProneAnalyzer = ErrorProneAnalyzer.create(scannerSupplier.get()).init(context);
+  }
+
+  /**
+   * Run Error Prone analysis after performing dataflow checks.
+   */
+  @Override
+  public void postFlow(Env<AttrContext> env) {
+    if (enabled) {
+      errorProneAnalyzer.finished(new TaskEvent(Kind.ANALYZE, env.toplevel, env.enclClass.sym));
+    }
+  }
+
+  @VisibleForTesting
+  public boolean isEnabled() {
+    return enabled;
+  }
+}
diff --git a/src/main/tools/jdk.BUILD b/src/main/tools/jdk.BUILD
index 871fd9e32a..af75f3f1b0 100644
--- a/src/main/tools/jdk.BUILD
+++ b/src/main/tools/jdk.BUILD
@@ -52,14 +52,3 @@ filegroup(
     name = "jdk-default",
     srcs = glob(["bin/*"]),
 )
-
-filegroup(
-    name = "langtools",
-    srcs = ["lib/tools.jar"],
-)
-
-java_import(
-    name = "langtools-neverlink",
-    jars = ["lib/tools.jar"],
-    neverlink = 1,
-)
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index fa8441fa6f..33386143d3 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -25,6 +25,7 @@ filegroup(
         "//src/test/shell:bashunit",
         "//third_party:srcs",
         "//third_party/ijar",
+        "//third_party/java/jdk/langtools:srcs",
         "//tools:srcs",
     ],
 )
diff --git a/src/test/shell/bazel/bazel_example_test.sh b/src/test/shell/bazel/bazel_example_test.sh
index a204ba1874..9eae7dfba8 100755
--- a/src/test/shell/bazel/bazel_example_test.sh
+++ b/src/test/shell/bazel/bazel_example_test.sh
@@ -59,8 +59,11 @@ function test_java() {
 function test_java_test() {
   setup_javatest_support
   local java_native_tests=//examples/java-native/src/test/java/com/example/myproject
+  local java_native_main=//examples/java-native/src/main/java/com/example/myproject
 
-  assert_build //examples/java-native/...
+  assert_build "-- //examples/java-native/... -${java_native_main}:hello-error-prone"
+  assert_build_fails "${java_native_main}:hello-error-prone" \
+      "Did you mean 'result = b == -1;'?"
   assert_test_ok "${java_native_tests}:hello"
   assert_test_ok "${java_native_tests}:custom"
   assert_test_fails "${java_native_tests}:fail"
diff --git a/src/test/shell/bazel/test-setup.sh b/src/test/shell/bazel/test-setup.sh
index d71507d90d..f95f33c36c 100755
--- a/src/test/shell/bazel/test-setup.sh
+++ b/src/test/shell/bazel/test-setup.sh
@@ -110,9 +110,13 @@ function create_new_workspace() {
   workspaces+=(${new_workspace_dir})
   cd ${new_workspace_dir}
   mkdir tools
+  mkdir -p third_party/java/jdk/langtools
 
   copy_tools_directory
 
+  [ -e third_party/java/jdk/langtools/javac.jar ] \
+    || ln -s "${langtools_path}"  third_party/java/jdk/langtools/javac.jar
+
   ln -s "${javabuilder_path}" tools/jdk/JavaBuilder_deploy.jar
   ln -s "${singlejar_path}"  tools/jdk/SingleJar_deploy.jar
   ln -s "${ijar_path}" tools/jdk/ijar
@@ -179,6 +183,15 @@ function assert_build_output() {
   test -f "$OUTPUT" || fail "Output $OUTPUT not found for target $*"
 }
 
+function assert_build_fails() {
+  bazel build -s $1 >& $TEST_log \
+    && fail "Test $1 succeed while expecting failure" \
+    || true
+  if [ -n "${2:-}" ]; then
+    expect_log "$2"
+  fi
+}
+
 function assert_test_ok() {
   bazel test --test_output=errors $* \
     || fail "Test $1 failed while expecting success"
diff --git a/src/test/shell/bazel/testenv.sh b/src/test/shell/bazel/testenv.sh
index cd81537f79..c84e839197 100755
--- a/src/test/shell/bazel/testenv.sh
+++ b/src/test/shell/bazel/testenv.sh
@@ -34,10 +34,12 @@ langtools="${TEST_SRCDIR}/src/test/shell/bazel/langtools.jar"
 
 # Tools directory location
 tools_dir="${TEST_SRCDIR}/tools"
+langtools_dir="${TEST_SRCDIR}/third_party/java/jdk/langtools"
 EXTRA_BAZELRC="build --java_langtools=//tools/jdk:test-langtools"
 
 # Java tooling
 javabuilder_path="${TEST_SRCDIR}/src/java_tools/buildjar/JavaBuilder_deploy.jar"
+langtools_path="${TEST_SRCDIR}/third_party/java/jdk/langtools/javac.jar"
 singlejar_path="${TEST_SRCDIR}/src/java_tools/singlejar/SingleJar_deploy.jar"
 ijar_path="${TEST_SRCDIR}/third_party/ijar/ijar"
 
@@ -77,6 +79,9 @@ function copy_tools_directory() {
 filegroup(name = "test-langtools", srcs = ["langtools.jar"])
 EOF
 
+  mkdir -p third_party/java/jdk/langtools
+  cp -R ${langtools_dir}/* third_party/java/jdk/langtools
+
   chmod -R +w .
   mkdir -p tools/defaults
   touch tools/defaults/BUILD
@@ -85,7 +90,7 @@ EOF
 # Report whether a given directory name corresponds to a tools directory.
 function is_tools_directory() {
   case "$1" in
-    tools)
+    third_party|tools)
       true
       ;;
     *)
diff --git a/third_party/README.md b/third_party/README.md
index 497ed1637f..609e817d2e 100644
--- a/third_party/README.md
+++ b/third_party/README.md
@@ -114,6 +114,13 @@ a minimal set of extra dependencies.
 * License: Apache License 2.0
 
 
+[javac](https://github.com/google/error-prone-javac)
+-------
+
+* Version: 1.9.0-dev-r2644-1
+* License: GNU GPL v2 with Classpath exception
+
+
 [jarjar](https://code.google.com/p/jarjar/)
 -----------
 
diff --git a/third_party/java/jdk/langtools/BUILD b/third_party/java/jdk/langtools/BUILD
new file mode 100644
index 0000000000..5870b8cb32
--- /dev/null
+++ b/third_party/java/jdk/langtools/BUILD
@@ -0,0 +1,13 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["restricted"])  # GNU GPL v2 with Classpath exception
+
+filegroup(
+    name = "srcs",
+    srcs = glob(["**"]),
+)
+
+filegroup(
+    name = "javac_jar",
+    srcs = ["javac.jar"],
+)
diff --git a/tools/jdk/BUILD b/tools/jdk/BUILD
index 8e898c4a11..d17277b87d 100644
--- a/tools/jdk/BUILD
+++ b/tools/jdk/BUILD
@@ -50,12 +50,12 @@ filegroup(
 
 filegroup(
     name = "langtools",
-    srcs = ["//external:langtools"],
+    srcs = ["//third_party/java/jdk/langtools:javac_jar"],
 )
 
 java_import(
     name = "langtools-neverlink",
-    jars = ["//tools/defaults:java_langtools"],
+    jars = [":langtools"],
     neverlink = 1,
 )