JavaBuilder: Reintroduce the -extra_checks flag.

Fixes #1570.

--
MOS_MIGRATED_REVID=128585415

4 files changed, 163 insertions, 18 deletions

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 076088cd37..c15f49a723 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
@@ -23,7 +23,6 @@ import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyMod
 import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorPronePlugin;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
-
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
@@ -31,6 +30,7 @@ import java.io.StringWriter;
 import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.List;
+import java.util.ListIterator;
 
 /**
  * The JavaBuilder main called by bazel.
@@ -99,6 +99,28 @@ public abstract class BazelJavaBuilder {
     }
   }
 
+  private static boolean processAndRemoveExtraChecksOptions(List<String> args) {
+    // error-prone is enabled by default for Bazel.
+    boolean errorProneEnabled = true;
+
+    ListIterator<String> arg = args.listIterator();
+    while (arg.hasNext()) {
+      switch (arg.next()) {
+        case "-extra_checks":
+        case "-extra_checks:on":
+          errorProneEnabled = true;
+          arg.remove();
+          break;
+        case "-extra_checks:off":
+          errorProneEnabled = false;
+          arg.remove();
+          break;
+      }
+    }
+
+    return errorProneEnabled;
+  }
+
   /**
    * Parses the list of arguments into a {@link JavaLibraryBuildRequest}. The returned
    * {@link JavaLibraryBuildRequest} object can be then used to configure the compilation itself.
@@ -110,12 +132,20 @@ public abstract class BazelJavaBuilder {
   @VisibleForTesting
   public static JavaLibraryBuildRequest parse(List<String> args) throws IOException,
       InvalidCommandLineException {
-    ImmutableList<BlazeJavaCompilerPlugin> plugins =
-        ImmutableList.<BlazeJavaCompilerPlugin>of(
-            new ClassLoaderMaskingPlugin(),
-            new ErrorPronePlugin());
+    OptionsParser optionsParser = new OptionsParser(args);
+    ImmutableList.Builder<BlazeJavaCompilerPlugin> plugins = ImmutableList.builder();
+    plugins.add(new ClassLoaderMaskingPlugin());
+
+    // Support for -extra_checks:off was removed from ErrorPronePlugin, but Bazel still needs it,
+    // so we'll emulate support for this here by handling the flag ourselves and not loading the
+    // plug-in when it is specified.
+    boolean errorProneEnabled = processAndRemoveExtraChecksOptions(optionsParser.getJavacOpts());
+    if (errorProneEnabled) {
+      plugins.add(new ErrorPronePlugin());
+    }
+
     JavaLibraryBuildRequest build =
-        new JavaLibraryBuildRequest(args, plugins, new DependencyModule.Builder());
+        new JavaLibraryBuildRequest(optionsParser, plugins.build(), new DependencyModule.Builder());
     build.setJavacOpts(JavacOptions.normalizeOptions(build.getJavacOpts()));
     return build;
   }
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
index b619e940a9..052b23fbe2 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList;
 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.processing.AnnotationProcessingModule;
-
 import java.io.IOException;
 import java.nio.file.Paths;
 import java.util.List;
@@ -86,34 +85,33 @@ public final class JavaLibraryBuildRequest {
   private final ImmutableList<BlazeJavaCompilerPlugin> plugins;
 
   /**
-   * Constructs a build from a list of command args. Sets the same JavacRunner
-   * for both compilation and annotation processing.
+   * Constructs a build from a list of command args. Sets the same JavacRunner for both compilation
+   * and annotation processing.
    *
-   * @param args the list of command line args
+   * @param optionsParser the parsed command line args.
    * @param extraPlugins extraneous plugins to use in addition to the strict dependency module.
    * @throws InvalidCommandLineException on any command line error
    */
-  public JavaLibraryBuildRequest(List<String> args, List<BlazeJavaCompilerPlugin> extraPlugins)
+  public JavaLibraryBuildRequest(
+      OptionsParser optionsParser, List<BlazeJavaCompilerPlugin> extraPlugins)
       throws InvalidCommandLineException, IOException {
-    this(args, extraPlugins, new DependencyModule.Builder());
+    this(optionsParser, extraPlugins, new DependencyModule.Builder());
   }
 
   /**
-   * Constructs a build from a list of command args. Sets the same JavacRunner
-   * for both compilation and annotation processing.
+   * Constructs a build from a list of command args. Sets the same JavacRunner for both compilation
+   * and annotation processing.
    *
-   * @param args the list of command line args
+   * @param optionsParser the parsed command line args.
    * @param extraPlugins extraneous plugins to use in addition to the strict dependency module.
    * @param depsBuilder a preconstructed dependency module builder.
    * @throws InvalidCommandLineException on any command line error
    */
   public JavaLibraryBuildRequest(
-      List<String> args,
+      OptionsParser optionsParser,
       List<BlazeJavaCompilerPlugin> extraPlugins,
       DependencyModule.Builder depsBuilder)
       throws InvalidCommandLineException, IOException {
-    OptionsParser optionsParser = new OptionsParser(args);
-
     depsBuilder.addDirectMappings(optionsParser.getDirectMappings());
     depsBuilder.addIndirectMappings(optionsParser.getIndirectMappings());
     if (optionsParser.getStrictJavaDeps() != null) {
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index 7fa85ce647..443e0103f8 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -112,6 +112,13 @@ sh_test(
 )
 
 sh_test(
+    name = "bazel_java_test",
+    size = "large",
+    srcs = ["bazel_java_test.sh"],
+    data = [":test-deps"],
+)
+
+sh_test(
     name = "bazel_rules_test",
     size = "large",
     srcs = ["bazel_rules_test.sh"],
diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
new file mode 100755
index 0000000000..8bc7bef8fd
--- /dev/null
+++ b/src/test/shell/bazel/bazel_java_test.sh
@@ -0,0 +1,110 @@
+#!/bin/bash
+#
+# Copyright 2016 The Bazel Authors. 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.
+#
+# Tests the examples provided in Bazel
+#
+
+# Load test environment
+source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-setup.sh \
+  || { echo "test-setup.sh not found!" >&2; exit 1; }
+
+function write_hello_library_files() {
+  mkdir -p java/main
+  cat >java/main/BUILD <<EOF
+java_binary(name = 'main',
+    deps = ['//java/hello_library'],
+    srcs = ['Main.java'],
+    main_class = 'main.Main')
+EOF
+
+  cat >java/main/Main.java <<EOF
+package main;
+import hello_library.HelloLibrary;
+public class Main {
+  public static void main(String[] args) {
+    HelloLibrary.funcHelloLibrary();
+    System.out.println("Hello, World!");
+  }
+}
+EOF
+
+  mkdir -p java/hello_library
+  cat >java/hello_library/BUILD <<EOF
+package(default_visibility=['//visibility:public'])
+java_library(name = 'hello_library',
+             srcs = ['HelloLibrary.java']);
+EOF
+
+  cat >java/hello_library/HelloLibrary.java <<EOF
+package hello_library;
+public class HelloLibrary {
+  public static void funcHelloLibrary() {
+    System.out.print("Hello, Library!;");
+  }
+}
+EOF
+}
+
+function test_build_hello_world() {
+  write_hello_library_files
+
+  bazel build //java/main:main &> $TEST_log || fail "build failed"
+}
+
+function test_errorprone_error_fails_build_by_default() {
+  write_hello_library_files
+  # Trigger an error-prone error by comparing two arrays via #equals().
+  cat >java/hello_library/HelloLibrary.java <<EOF
+package hello_library;
+public class HelloLibrary {
+  public static boolean funcHelloLibrary() {
+    int[] arr1 = {1, 2, 3};
+    int[] arr2 = {1, 2, 3};
+    return arr1.equals(arr2);
+  }
+}
+EOF
+
+  bazel build //java/main:main &> $TEST_log && fail "build should have failed" || true
+  expect_log "error: \[ArrayEquals\] Reference equality used to compare arrays"
+}
+
+function test_extrachecks_off_disables_errorprone() {
+  write_hello_library_files
+  # Trigger an error-prone error by comparing two arrays via #equals().
+  cat >java/hello_library/HelloLibrary.java <<EOF
+package hello_library;
+public class HelloLibrary {
+  public static boolean funcHelloLibrary() {
+    int[] arr1 = {1, 2, 3};
+    int[] arr2 = {1, 2, 3};
+    return arr1.equals(arr2);
+  }
+}
+EOF
+  # Disable error-prone for this target, though.
+  cat >java/hello_library/BUILD <<EOF
+package(default_visibility=['//visibility:public'])
+java_library(name = 'hello_library',
+             srcs = ['HelloLibrary.java'],
+             javacopts = ['-extra_checks:off'],);
+EOF
+
+  bazel build //java/main:main &> $TEST_log || fail "build failed"
+  expect_not_log "error: \[ArrayEquals\] Reference equality used to compare arrays"
+}
+
+run_suite "Java integration tests"