From 0f183a19452d83ceae4f14e8754ea39578bc8e49 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Wed, 27 Jul 2016 15:00:41 +0000 Subject: [PATCH] JavaBuilder: Reintroduce the -extra_checks flag. Fixes #1570. -- MOS_MIGRATED_REVID=128585415 --- .../build/buildjar/BazelJavaBuilder.java | 42 ++++++- .../buildjar/JavaLibraryBuildRequest.java | 22 ++-- src/test/shell/bazel/BUILD | 7 ++ src/test/shell/bazel/bazel_java_test.sh | 110 ++++++++++++++++++ 4 files changed, 163 insertions(+), 18 deletions(-) create mode 100755 src/test/shell/bazel/bazel_java_test.sh 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 076088cd370ec0..c15f49a723ee99 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.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.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 static int processRequest(List args, PrintWriter err) { } } + private static boolean processAndRemoveExtraChecksOptions(List args) { + // error-prone is enabled by default for Bazel. + boolean errorProneEnabled = true; + + ListIterator 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 static int processRequest(List args, PrintWriter err) { @VisibleForTesting public static JavaLibraryBuildRequest parse(List args) throws IOException, InvalidCommandLineException { - ImmutableList plugins = - ImmutableList.of( - new ClassLoaderMaskingPlugin(), - new ErrorPronePlugin()); + OptionsParser optionsParser = new OptionsParser(args); + ImmutableList.Builder 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 b619e940a987b1..052b23fbe2d261 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.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 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 args, List extraPlugins) + public JavaLibraryBuildRequest( + OptionsParser optionsParser, List 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 args, + OptionsParser optionsParser, List 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 7e1f4e68cd2536..cd451ced319d7d 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -110,6 +110,13 @@ sh_test( shard_count = 3, ) +sh_test( + name = "bazel_java_test", + size = "large", + srcs = ["bazel_java_test.sh"], + data = [":test-deps"], +) + sh_test( name = "bazel_rules_test", size = "large", 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 00000000000000..8bc7bef8fdfa8e --- /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 <java/main/Main.java <java/hello_library/BUILD <java/hello_library/HelloLibrary.java < $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 < $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 <java/hello_library/BUILD < $TEST_log || fail "build failed" + expect_not_log "error: \[ArrayEquals\] Reference equality used to compare arrays" +} + +run_suite "Java integration tests"